Re: recovering from "found xmin ... from before relfrozenxid ..."

2020-09-12 Thread Tom Lane
Robert Haas  writes:
> I have committed this version.

This failure says that the test case is not entirely stable:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2020-09-12%2005%3A13%3A12

diff -U3 
/home/nm/farm/gcc64/HEAD/pgsql.build/contrib/pg_surgery/expected/heap_surgery.out
 
/home/nm/farm/gcc64/HEAD/pgsql.build/contrib/pg_surgery/results/heap_surgery.out
--- 
/home/nm/farm/gcc64/HEAD/pgsql.build/contrib/pg_surgery/expected/heap_surgery.out
   2020-09-11 06:31:36.0 +
+++ 
/home/nm/farm/gcc64/HEAD/pgsql.build/contrib/pg_surgery/results/heap_surgery.out
2020-09-12 11:40:26.0 +
@@ -116,7 +116,6 @@
  vacuum freeze htab2;
  -- unused TIDs should be skipped
  select heap_force_kill('htab2'::regclass, ARRAY['(0, 2)']::tid[]);
- NOTICE:  skipping tid (0, 2) for relation "htab2" because it is marked unused
   heap_force_kill 
  -
   

sungazer's first run after pg_surgery went in was successful, so it's
not a hard failure.  I'm guessing that it's timing dependent.

The most obvious theory for the cause is that what VACUUM does with
a tuple depends on whether the tuple's xmin is below global xmin,
and a concurrent autovacuum could very easily be holding back global
xmin.  While I can't easily get autovac to run at just the right
time, I did verify that a concurrent regular session holding back
global xmin produces the symptom seen above.  (To replicate, insert
"select pg_sleep(...)" in heap_surgery.sql before "-- now create an unused
line pointer"; run make installcheck; and use the delay to connect
to the database manually, start a serializable transaction, and do
any query to acquire a snapshot.)

I suggest that the easiest way to make this test reliable is to
make the test tables be temp tables (which allows dropping the
autovacuum_enabled = off property, too).  In the wake of commit
a7212be8b, that should guarantee that vacuum has stable tuple-level
behavior regardless of what is happening concurrently.

regards, tom lane




Re: Missing "Up" navigation link between parts and doc root?

2020-09-12 Thread Peter Eisentraut

On 2020-09-11 14:58, Fabien COELHO wrote:



On 2020-09-08 21:10, Bruce Momjian wrote:


I see this only applied to master.  Shouldn't this be backpatched?


I wasn't planning to.  It's not a bug fix.

Other thoughts?


Yep. ISTM nicer if all docs have the same navigation, especially as
googling often points to random versions. No big deal anyway, in six year
all supported versions will have a up link on the part level! :-)


Okay, backpatched to PG10.

--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: Parallel worker hangs while handling errors.

2020-09-12 Thread Tom Lane
Robert Haas  writes:
> On Fri, Sep 11, 2020 at 4:20 PM Tom Lane  wrote:
>> It's not clear to me whether we want to institute the "accepting SIGQUIT
>> is always okay" rule in processes that didn't already have code to change
>> BlockSig.

> I think a backend process that isn't timely handling SIGQUIT is by
> that very fact buggy. "pg_ctl stop -mi" isn't a friendly suggestion.
> So I favor trying to make it uniform.

Well, if we want to take a hard line about that, we should centralize
the setup of SIGQUIT.  The attached makes InitPostmasterChild do it,
and removes the duplicative code from elsewhere.

I also flipped autovacuum and walsender from using quickdie to using
SignalHandlerForCrashExit.  Whatever you think about the safety or
unsafety of quickdie, there seems no reason for autovacuum to be trying
to tell its nonexistent client about a shutdown.  I don't think it's
terribly useful for a walsender either, though maybe somebody has a
different opinion about that?

regards, tom lane

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 19ba26b914..2cef56f115 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -454,8 +454,8 @@ AutoVacLauncherMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, StatementCancelHandler);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
+	/* SIGQUIT handler was already set up by InitPostmasterChild */
 
-	pqsignal(SIGQUIT, quickdie);
 	InitializeTimeouts();		/* establishes SIGALRM handler */
 
 	pqsignal(SIGPIPE, SIG_IGN);
@@ -498,9 +498,10 @@ AutoVacLauncherMain(int argc, char *argv[])
 	 *
 	 * Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
 	 * (to wit, BlockSig) will be restored when longjmp'ing to here.  Thus,
-	 * signals will be blocked until we complete error recovery.  It might
-	 * seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
-	 * it is not since InterruptPending might be set already.
+	 * signals other than SIGQUIT will be blocked until we complete error
+	 * recovery.  It might seem that this policy makes the HOLD_INTERRUPTS()
+	 * call redundant, but it is not since InterruptPending might be set
+	 * already.
 	 */
 	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
 	{
@@ -1531,7 +1532,8 @@ AutoVacWorkerMain(int argc, char *argv[])
 	 */
 	pqsignal(SIGINT, StatementCancelHandler);
 	pqsignal(SIGTERM, die);
-	pqsignal(SIGQUIT, quickdie);
+	/* SIGQUIT handler was already set up by InitPostmasterChild */
+
 	InitializeTimeouts();		/* establishes SIGALRM handler */
 
 	pqsignal(SIGPIPE, SIG_IGN);
@@ -1562,9 +1564,9 @@ AutoVacWorkerMain(int argc, char *argv[])
 	 *
 	 * Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
 	 * (to wit, BlockSig) will be restored when longjmp'ing to here.  Thus,
-	 * signals will be blocked until we exit.  It might seem that this policy
-	 * makes the HOLD_INTERRUPTS() call redundant, but it is not since
-	 * InterruptPending might be set already.
+	 * signals other than SIGQUIT will be blocked until we exit.  It might
+	 * seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
+	 * it is not since InterruptPending might be set already.
 	 */
 	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
 	{
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index d043ced686..5a9a0e3435 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -731,9 +731,9 @@ StartBackgroundWorker(void)
 		pqsignal(SIGFPE, SIG_IGN);
 	}
 	pqsignal(SIGTERM, bgworker_die);
+	/* SIGQUIT handler was already set up by InitPostmasterChild */
 	pqsignal(SIGHUP, SIG_IGN);
 
-	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	InitializeTimeouts();		/* establishes SIGALRM handler */
 
 	pqsignal(SIGPIPE, SIG_IGN);
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index c96568149f..a7afa758b6 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -104,7 +104,7 @@ BackgroundWriterMain(void)
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+	/* SIGQUIT handler was already set up by InitPostmasterChild */
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -115,10 +115,6 @@ BackgroundWriterMain(void)
 	 */
 	pqsignal(SIGCHLD, SIG_DFL);
 
-	/* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
-	sigdelset(, SIGQUIT);
-	PG_SETMASK();
-
 	/*
 	 * We just started, assume there has been either a shutdown or
 	 * end-of-recovery snapshot.
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 45f5deca72..3e7dcd4f76 100644
--- a/src/backend/postmaster/checkpointer.c
+++ 

Re: Function to execute a program

2020-09-12 Thread Andrew Dunstan


On 9/12/20 11:11 AM, Magnus Hagander wrote:
>
>
> On Sat, Sep 12, 2020 at 5:06 PM Tom Lane  > wrote:
>
> Magnus Hagander mailto:mag...@hagander.net>>
> writes:
> > Would it make sense to have a pg_execute_program() that
> corresponds to COPY
> > FROM PROGRAM? This would obviously have the same permissions
> restrictions
> > as COPY FROM PROGRAM.
> > The usecase would be to for example execute a command that
> returns json
> > format output, which could then be parsed and processed as part
> of a query.
> > Today, COPY FROM PROGRAM cannot do this, as we can't read a
> multiline json
> > value with COPY.
>
> copy ... from program 'random_json_producer | tr "\n\t" "  "';
>
> I don't necessarily object to providing such a function to make it
> easier, but it's not the case that you can't have the functionality
> today.
>
>
> "tr" is not typically available on Windows, for one :)
>
> But yes, assuming tr is available, it is true that you can. (And you
> can perhaps find something else to pipe it through on Windows). Of
> course, you still can't use it in a query, since COPY can only target
> a table :) Independently of something like this it would be nice to be
> able to target say a CTE with COPY, but that's kan entirely different
> topic.
>
>

A more robust recipe  would use "jq -c" as the filter. And it's
available on Windows via chocolatey.


I don't have a strong opinion on the suggested facility. Presumably you
can construct a function that runs COPY into a temp table and then
returns the results. But maybe that's more work than we should require
users to perform.


cheers


andrew


-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





Re: Function to execute a program

2020-09-12 Thread Magnus Hagander
On Sat, Sep 12, 2020 at 5:06 PM Tom Lane  wrote:

> Magnus Hagander  writes:
> > Would it make sense to have a pg_execute_program() that corresponds to
> COPY
> > FROM PROGRAM? This would obviously have the same permissions restrictions
> > as COPY FROM PROGRAM.
> > The usecase would be to for example execute a command that returns json
> > format output, which could then be parsed and processed as part of a
> query.
> > Today, COPY FROM PROGRAM cannot do this, as we can't read a multiline
> json
> > value with COPY.
>
> copy ... from program 'random_json_producer | tr "\n\t" "  "';
>
> I don't necessarily object to providing such a function to make it
> easier, but it's not the case that you can't have the functionality
> today.
>

"tr" is not typically available on Windows, for one :)

But yes, assuming tr is available, it is true that you can. (And you can
perhaps find something else to pipe it through on Windows). Of course, you
still can't use it in a query, since COPY can only target a table :)
Independently of something like this it would be nice to be able to target
say a CTE with COPY, but that's kan entirely different topic.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 


Re: Function to execute a program

2020-09-12 Thread Tom Lane
Magnus Hagander  writes:
> Would it make sense to have a pg_execute_program() that corresponds to COPY
> FROM PROGRAM? This would obviously have the same permissions restrictions
> as COPY FROM PROGRAM.
> The usecase would be to for example execute a command that returns json
> format output, which could then be parsed and processed as part of a query.
> Today, COPY FROM PROGRAM cannot do this, as we can't read a multiline json
> value with COPY.

copy ... from program 'random_json_producer | tr "\n\t" "  "';

I don't necessarily object to providing such a function to make it
easier, but it's not the case that you can't have the functionality
today.

regards, tom lane




Re: Logical Replication - detail message with names of missing columns

2020-09-12 Thread Bharath Rupireddy
On Fri, Sep 11, 2020 at 9:05 PM Alvaro Herrera  wrote:
>
> On 2020-Sep-11, Tom Lane wrote:
>
> > Check, but you could imagine that the column-list string is constructed
> > with code along the lines of
> >
> >   if (first)
> >   appendStringInfo(buf, _("\"%s\""), colname);
> >   else
> >   appendStringInfo(buf, _(", \"%s\""), colname);
> > thus allowing a translator to replace the quote marks.
>
> This works OK for my language at least.  I couldn't find any guidance on
> whether there's a problem doing things this way for RTL languages etc,
> but +1 for doing it this way.
>

Thanks for the comments. I changed the patch to use the string
preparation in below fashion. Attaching the v5 patch. Please let me
know if there are any further inputs.

+if (missingattcnt == 1)
+appendStringInfo(, _("\"%s\""),
+ remoterel->attnames[i]);
+else
+appendStringInfo(, _(", \"%s\""),
+ remoterel->attnames[i]);

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com


v5-0001-Detail-message-with-names-of-missing-columns-in-l.patch
Description: Binary data


Re: Inconsistency in determining the timestamp of the db statfile.

2020-09-12 Thread Magnus Hagander
On Fri, Sep 11, 2020 at 4:53 AM Amit Kapila  wrote:

> On Thu, Sep 10, 2020 at 6:42 PM Alvaro Herrera 
> wrote:
> >
> > On 2020-Sep-10, Amit Kapila wrote:
> >
> > > On Thu, Sep 10, 2020 at 1:03 PM Magnus Hagander 
> wrote:
> >
> > > The comments already say what you said in the second suggestion:"The
> > > caller must rely on timestamp stored in *ts iff the function returns
> > > true.". Read iff "as if and only if"
> >
> > I think "must" should be "may" there, if we're nitpicking.
> >
>
> Here, we want to say that "caller can rely on *ts only if the function
> returns true". If we replace 'must' with 'may' then it seems to me we
> are trying to say that caller can ignore the timestamp value, if so,
> why at first place caller has called this function.
>

This is true, but that should really be the decision of the caller, not of
the function.

But again, that's extremely nitpicky, so it doesn't really matter :)

And +1 on the push you did, and the decision not to backpatch it since
there haven't been any reports.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 


Function to execute a program

2020-09-12 Thread Magnus Hagander
Today we have pg_read_file() and pg_read_binary_file(), which mostly
correspond to COPY from a file, but reads a whole (or parts but based on
bytes) into a single value rather than rows.

Would it make sense to have a pg_execute_program() that corresponds to COPY
FROM PROGRAM? This would obviously have the same permissions restrictions
as COPY FROM PROGRAM.

The usecase would be to for example execute a command that returns json
format output, which could then be parsed and processed as part of a query.
Today, COPY FROM PROGRAM cannot do this, as we can't read a multiline json
value with COPY.

Thoughts?

//Magnus


Re: Inconsistency in determining the timestamp of the db statfile.

2020-09-12 Thread Amit Kapila
On Thu, Sep 10, 2020 at 1:13 PM Amit Kapila  wrote:
>
>
> BTW, do we want to backpatch this? There is no user reported bug and
> not sure if the user will encounter any problem. I think it is a minor
> improvement and more of code consistency. So, making HEAD only change
> should be okay.
>

Seeing no other opinions, pushed this in Head.

-- 
With Regards,
Amit Kapila.




Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

2020-09-12 Thread Amit Kapila
On Wed, Sep 9, 2020 at 2:26 PM Amit Kapila  wrote:
>
> On Wed, Sep 9, 2020 at 2:13 PM Tomas Vondra
>  wrote:
> >
> > Hi,
> >
> > while looking at the streaming code I noticed two minor issues:
> >
> > 1) logicalrep_read_stream_stop is never defined/called, so the prototype
> > in logicalproto.h is unnecessary
> >
> > 2) minor typo in one of the comments
> >
> > Patch attached.
> >
>
> LGTM.
>

Pushed.

-- 
With Regards,
Amit Kapila.




Re: [HACKERS] logical decoding of two-phase transactions

2020-09-12 Thread Amit Kapila
On Wed, Sep 9, 2020 at 3:33 PM Ajin Cherian  wrote:
>
> On Mon, Sep 7, 2020 at 11:17 PM Amit Kapila  wrote:
>>
>>
>> Nikhil has a test for the same
>> (0004-Teach-test_decoding-plugin-to-work-with-2PC.Jan4) in his last
>> email [1]. You might want to use it to test this behavior. I think you
>> can also keep the tests as a separate patch as Nikhil had.
>>
> Done. I've added the tests and also tweaked code to make sure that the aborts 
> during 2 phase commits are also handled.
>>

Okay, I'll look into your changes but before that today, I have gone
through this entire thread to check if there are any design problems
and found that there were two major issues in the original proposal,
(a) one was to handle concurrent aborts which I think we should be
able to deal in a way similar to what we have done for decoding of
in-progress transactions and (b) what if someone specifically locks
pg_class or pg_attribute in exclusive mode (say be Lock pg_attribute
...), it seems the deadlock can happen in that case [0]. AFAIU, people
seem to think if there is no realistic scenario where deadlock can
happen apart from user explicitly locking the system catalog then we
might be able to get away by just ignoring such xacts to be decoded at
prepare time or would block it in some other way as any way that will
block the entire system. I am not sure what is the right thing but
something has to be done to avoid any sort of deadlock for this.

Another thing, I noticed is that originally we have subscriber-side
support as well, see [1] (see *pgoutput* patch) but later dropped it
due to some reasons [2]. I think we should have pgoutput support as
well, so see what is required to get that incorporated.

I would also like to summarize my thinking on the usefulness of this
feature. One of the authors of this patch Stats wants this for a
conflict-free logical replication, see more details [3]. Craig seems
to suggest [3] that this will allow us to avoid conflicting schema
changes at different nodes though it is not clear to me if that is
possible without some external code support because we don't send
schema changes in logical replication, maybe Craig can shed some light
on this. Another use-case, I am thinking is if this can be used for
scaling-out reads as well. Because of 2PC, we can ensure that on
subscribers we have all the data committed on the master. Now, we can
design a system where different nodes are owners of some set of tables
and we can always get the data of those tables reliably from those
nodes, and then one can have some external process that will route the
reads accordingly. I know that the last idea is a bit of a hand-waving
but it seems to be possible after this feature.

[0] - 
https://www.postgresql.org/message-id/20170328012546.473psm6546bgsi2c%40alap3.anarazel.de
[1] - 
https://www.postgresql.org/message-id/CAMGcDxchx%3D0PeQBVLzrgYG2AQ49QSRxHj5DCp7yy0QrJR0S0nA%40mail.gmail.com
[2] - 
https://www.postgresql.org/message-id/CAMGcDxc-kuO9uq0zRCRwbHWBj_rePY9%3DraR7M9pZGWoj9EOGdg%40mail.gmail.com
[3] - 
https://www.postgresql.org/message-id/CAMsr%2BYHQzGxnR-peT4SbX2-xiG2uApJMTgZ4a3TiRBM6COyfqg%40mail.gmail.com

-- 
With Regards,
Amit Kapila.




Re: Avoid incorrect allocation in buildIndexArray

2020-09-12 Thread Julien Rouhaud
Le sam. 12 sept. 2020 à 11:14, Michael Paquier  a
écrit :

> On Fri, Sep 11, 2020 at 01:49:26PM +0200, Julien Rouhaud wrote:
> > On Fri, Sep 11, 2020 at 1:39 PM Daniel Gustafsson 
> wrote:
> >> Any reason not to bail early as per the attached?
> >
> > +1
>
> Makes sense to me.  This has also the advantage to cause a crash if
> there is an attempt to refer to those empty arrays in case of future
> refactoring, which is rather defensive.  By looking at
> findObjectByOid(), I can also see that we check for a negative number,
>

yes, I also checked that current code is already checking for that.

so I concur with Ranier's comment to check after that on top of 0.
> If there are no objections, I'll apply that on HEAD.


agreed.

>


Re: Avoid incorrect allocation in buildIndexArray

2020-09-12 Thread Michael Paquier
On Fri, Sep 11, 2020 at 01:49:26PM +0200, Julien Rouhaud wrote:
> On Fri, Sep 11, 2020 at 1:39 PM Daniel Gustafsson  wrote:
>> Any reason not to bail early as per the attached?
> 
> +1

Makes sense to me.  This has also the advantage to cause a crash if
there is an attempt to refer to those empty arrays in case of future
refactoring, which is rather defensive.  By looking at
findObjectByOid(), I can also see that we check for a negative number,
so I concur with Ranier's comment to check after that on top of 0.
If there are no objections, I'll apply that on HEAD.
--
Michael


signature.asc
Description: PGP signature


Re: proposal: possibility to read dumped table's name from file

2020-09-12 Thread Pavel Stehule
Hi

pá 11. 9. 2020 v 10:50 odesílatel Pavel Stehule 
napsal:

> Hi
>
> po 7. 9. 2020 v 14:14 odesílatel Surafel Temesgen 
> napsal:
>
>> Hi Pavel
>>
>> On Fri, Sep 4, 2020 at 6:22 AM Pavel Stehule 
>> wrote:
>>
>>>
>>> Here is updated patch for pg_dump
>>>
>>>
>> pg_dumpall also has –exclude-database=pattern and –no-comments option
>> doesn't that qualify it to benefits from this feature? And please add a
>> test case for this option
>>
>
> This patch is related to pg_dump (in this moment), so pg_dumpall options
> are out of scope.
>
> I am not sure if pg_dumpall needs this functionality - maybe, but I can
> use bash or some similar for implementation of this feature. There is no
> requirement to do it all necessary work under one transaction, one snapshot.
>
> For pg_dump can be used different format, because it uses different
> granularity.  Some like "{+/-} dbname"
>
> "--no-comments" is a global parameter without arguments. I don't
> understand how this parameter can be related to this feature?
>
> I am working on regress tests.
>

There is a updated version with regress tests

Regards

Pavel


> Regards
>
> Pavel
>
>
>> regards
>>
>> Surafel
>>
>>
>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 0b2e2de87b..c4f818f305 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -755,6 +755,56 @@ PostgreSQL documentation
   
  
 
+ 
+  --filter=filename
+  
+   
+Read objects filters from the specified file.
+If you use "-" as a filename, the filters are read from stdin.
+The lines of this file must have the following format:
+
+(+|-)[tnfd] objectname
+
+   
+
+   
+The first character specifies whether the object is to be included
+(+) or excluded (-), and the
+second character specifies the type of object to be filtered:
+t (table),
+n (schema),
+f (foreign server),
+d (table data).
+   
+
+   
+With the following filter file, the dump would include table
+mytable1 and data from foreign tables of
+some_foreign_server foreign server, but exclude data
+from table mytable2.
+
++t mytable1
++f some_foreign_server
+-d mytable2
+
+   
+
+   
+The lines starting with symbol # are ignored.
+Previous white chars (spaces, tabs) are not allowed. These
+lines can be used for comments, notes. Empty lines are ignored too.
+   
+
+   
+The --filter option works just like the other
+options to include or exclude tables, schemas, table data, or foreign
+tables, and both forms may be combined.  Note that there are no options
+to exclude a specific foreign table or to include a specific table's
+data.
+   
+  
+ 
+
  
   --if-exists
   
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 784bceaec3..94a8b2e187 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -53,9 +53,11 @@
 #include "catalog/pg_trigger_d.h"
 #include "catalog/pg_type_d.h"
 #include "common/connect.h"
+#include "common/string.h"
 #include "dumputils.h"
 #include "fe_utils/string_utils.h"
 #include "getopt_long.h"
+#include "lib/stringinfo.h"
 #include "libpq/libpq-fs.h"
 #include "parallel.h"
 #include "pg_backup_db.h"
@@ -291,6 +293,7 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
 static char *get_synchronized_snapshot(Archive *fout);
 static void setupDumpWorker(Archive *AHX);
 static TableInfo *getRootTableInfo(TableInfo *tbinfo);
+static void read_patterns_from_file(char *filename, DumpOptions *dopt);
 
 
 int
@@ -365,6 +368,7 @@ main(int argc, char **argv)
 		{"enable-row-security", no_argument, _row_security, 1},
 		{"exclude-table-data", required_argument, NULL, 4},
 		{"extra-float-digits", required_argument, NULL, 8},
+		{"filter", required_argument, NULL, 12},
 		{"if-exists", no_argument, _exists, 1},
 		{"inserts", no_argument, NULL, 9},
 		{"lock-wait-timeout", required_argument, NULL, 2},
@@ -604,6 +608,10 @@ main(int argc, char **argv)
 		  optarg);
 break;
 
+			case 12:			/* filter implementation */
+read_patterns_from_file(optarg, );
+break;
+
 			default:
 fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 exit_nicely(1);
@@ -1023,6 +1031,8 @@ help(const char *progname)
 			 "   access to)\n"));
 	printf(_("  --exclude-table-data=PATTERN do NOT dump data for the specified table(s)\n"));
 	printf(_("  --extra-float-digits=NUM override default setting for extra_float_digits\n"));
+	printf(_("  --filter=FILENAMEdump objects and data based on the filter expressions\n"
+			 "   from the filter file\n"));
 	printf(_("  --if-exists  use IF EXISTS when dropping objects\n"));
 	printf(_("