Re: documentation fixes for partition pruning, round two

[Justin Pryzby]

On Thu, May 24, 2018 at 11:30:40AM +0900, Amit Langote wrote:
> Hi Justin.
> 
> Thanks for writing the patch.  I have a couple of comments.

Thanks for your review, find attached updated patch.

> +possible to show the difference between a plan whose partitions have been
> +pruned and one whose partitions haven't.  A typical unoptimized plan for
> +this type of table setup is:
> 
> "a plan whose partitions have been pruned" sounds a bit off; maybe, "a
> plan in which partitions have been pruned".

I wrote:

"[...] a plan for which partitions have been pruned and for which they have
not."

Cheers,
Justin
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2cd0b8a..bae2589 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2967,70 +2967,70 @@ VALUES ('Albany', NULL, NULL, 'NY');

 

 It is not possible to turn a regular table into a partitioned table or
 vice versa.  However, it is possible to add a regular or partitioned table
 containing data as a partition of a partitioned table, or remove a
 partition from a partitioned table turning it into a standalone table;
 see  to learn more about the
 ATTACH PARTITION and DETACH PARTITION
 sub-commands.

 

 Individual partitions are linked to the partitioned table with inheritance
-behind-the-scenes; however, it is not possible to use some of the
-inheritance features discussed in the previous section with partitioned
-tables and partitions.  For example, a partition cannot have any parents
+behind-the-scenes; however, it is not possible to use some of the generic
+features of inheritance (discussed below) with declaratively partitioned
+tables or their partitions For example, a partition cannot have any parents
 other than the partitioned table it is a partition of, nor can a regular
-table inherit from a partitioned table making the latter its parent.
-That means partitioned tables and partitions do not participate in
-inheritance with regular tables.  Since a partition hierarchy consisting
-of the partitioned table and its partitions is still an inheritance
-hierarchy, all the normal rules of inheritance apply as described in
+table inherit from a partitioned table making the latter its parent.  That
+means partitioned tables and partitions do not participate in inheritance
+with regular tables.  Since a partition hierarchy consisting of the
+partitioned table and its partitions is still an inheritance hierarchy, all
+the normal rules of inheritance apply as described in
  with some exceptions, most notably:
 
 
  
   
Both CHECK and NOT NULL
constraints of a partitioned table are always inherited by all its
partitions.  CHECK constraints that are marked
NO INHERIT are not allowed to be created on
partitioned tables.
   
  
 
  
   
Using ONLY to add or drop a constraint on only the
partitioned table is supported when there are no partitions.  Once
partitions exist, using ONLY will result in an error
as adding or dropping constraints on only the partitioned table, when
-   partitions exist, is not supported.  Instead, constraints can be added
-   or dropped, when they are not present in the parent table, directly on
-   the partitions.  As a partitioned table does not have any data
-   directly, attempts to use TRUNCATE
+   partitions exist, is not supported.  Instead, constraints on the
+   partitions themselves can be added and (if they are not present in the
+   parent table) dropped.  As a partitioned table does not
+   have any data directly, attempts to use TRUNCATE
ONLY on a partitioned table will always return an
error.
   
  
 
  
   
Partitions cannot have columns that are not present in the parent.  It
-   is neither possible to specify columns when creating partitions with
-   CREATE TABLE nor is it possible to add columns to
+   is not possible to specify columns when creating partitions with
+   CREATE TABLE, nor is it possible to add columns to
partitions after-the-fact using ALTER TABLE.  Tables 
may be
added as a partition with ALTER TABLE ... ATTACH 
PARTITION
only if their columns exactly match the parent, including any
oid column.
   
  
 
  
   
You cannot drop the NOT NULL constraint on a
partition's column if the constraint is present in the parent table.
   
  
 
@@ -3347,37 +3347,37 @@ ALTER TABLE measurement ATTACH PARTITION 
measurement_y2008m02
on individual partitions, not the partitioned table.
   
  
 
 
 

 

 Implementation Using Inheritance
 
  While the built-in declarative partitioning is suitable for most
  common use cases, there are some circumstances 

Re: PG11 jit failing on ppc64el

[Tom Lane]

Thomas Munro  writes:
> BTW It is working on arm64 too, starting with LLVM 6.  5 crashed the
> same way as it does on ppc.  See build farm member eelpout which is
> running Debian.

For entertainment's sake, I tried building --with-llvm on FreeBSD 12
arm64 (hey, gotta do something with this raspberry pi toy I got).
I used llvm-devel-7.0.d20180327 which seems to be the latest available in
FreeBSD's package system.  Builds cleanly, does not work at all.
SIGSEGV here:

#0  __clear_cache (start=0x4c055000, end=0x4c0566ec)
at /usr/src/contrib/compiler-rt/lib/builtins/clear_cache.c:168
#1  0x4bb78d8c in 
llvm::sys::Memory::protectMappedMemory(llvm::sys::MemoryBlock const&, unsigned 
int) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#2  0x4b68f020 in 
llvm::SectionMemoryManager::applyMemoryGroupPermissions(llvm::SectionMemoryManager::MemoryGroup&,
 unsigned int) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#3  0x4b68ef38 in 
llvm::SectionMemoryManager::finalizeMemory(std::__1::basic_string*) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#4  0x4b85d310 in llvm::RuntimeDyld::finalizeWithMemoryManagerLocking()
() from /home/tgl/installdir/lib/postgresql/llvmjit.so
#5  0x4ad22c38 in llvm::orc::RTDyldObjectLinkingLayer::ConcreteLinkedObj
ect::finalize() ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#6  0x4ad236ec in 
llvm::orc::RTDyldObjectLinkingLayer::ConcreteLinkedObject::getSymbolMaterializer(std::__1::basic_string)::{lambda()#1}::operator()() const ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#7  0x4ad22084 in llvm::JITSymbol::getAddress() ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#8  0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#9  0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#10 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#11 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#12 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#13 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#14 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#15 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#16 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#17 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#18 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#19 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#20 0x4ad1dbec in llvm::OrcCBindingsStack::findSymbolAddress(unsigned 
long&, std::__1::basic_string const&, bool) ()
   from /home/tgl/installdir/lib/postgresql/llvmjit.so
#21 0x4ad1dbec in 

Re: Simplify final sync in pg_rewind's target folder and add --no-sync

[Michael Paquier]

On Sun, Mar 25, 2018 at 09:26:07PM +0900, Michael Paquier wrote:
> Both things are implemented as attached.  I am of course not pushing for
> integrating that patch in v11 even if it is straight-forward, so I'll
> park it in the next future commit fest.

Patch has roted per the feedback of Mr Robot here:
http://cfbot.cputube.org/michael-paquier.html
So attached is a refreshed version.
--
Michael
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 520d843f0e..0f73a6a1ae 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -151,6 +151,22 @@ PostgreSQL documentation
   
  
 
+ 
+  -N
+  --no-sync
+  
+   
+By default, pg_rewind will wait for all files
+to be written safely to disk.  This option causes
+pg_rewind to return without waiting, which is
+faster, but means that a subsequent operating system crash can leave
+the synchronized data folder corrupt.  Generally, this option is
+useful for testing but should not be used when creating a production
+installation.
+   
+  
+ 
+
  
   -P
   --progress
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 52531bba7a..572290f14d 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -219,7 +219,8 @@ sub run_pg_rewind
 'pg_rewind',
 "--debug",
 "--source-pgdata=$standby_pgdata",
-"--target-pgdata=$master_pgdata"
+"--target-pgdata=$master_pgdata",
+"--no-sync"
 			],
 			'pg_rewind local');
 	}
@@ -231,7 +232,8 @@ sub run_pg_rewind
 			[
 'pg_rewind',   "--debug",
 "--source-server", $standby_connstr,
-"--target-pgdata=$master_pgdata"
+"--target-pgdata=$master_pgdata",
+"--no-sync"
 			],
 			'pg_rewind remote');
 	}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index b0fd3f66ac..7282cc6e7b 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -25,6 +25,7 @@
 #include "catalog/catversion.h"
 #include "catalog/pg_control.h"
 #include "common/file_perm.h"
+#include "common/file_utils.h"
 #include "common/restricted_token.h"
 #include "getopt_long.h"
 #include "storage/bufpage.h"
@@ -55,6 +56,7 @@ char	   *connstr_source = NULL;
 bool		debug = false;
 bool		showprogress = false;
 bool		dry_run = false;
+bool		do_sync = true;
 
 /* Target history */
 TimeLineHistoryEntry *targetHistory;
@@ -70,6 +72,7 @@ usage(const char *progname)
 	printf(_("  --source-pgdata=DIRECTORY  source data directory to synchronize with\n"));
 	printf(_("  --source-server=CONNSTRsource server to synchronize with\n"));
 	printf(_("  -n, --dry-run  stop before modifying anything\n"));
+	printf(_("  -N, --no-sync  do not wait for changes to be written safely to disk\n"));
 	printf(_("  -P, --progress write progress messages\n"));
 	printf(_("  --debugwrite a lot of debug messages\n"));
 	printf(_("  -V, --version  output version information, then exit\n"));
@@ -88,6 +91,7 @@ main(int argc, char **argv)
 		{"source-server", required_argument, NULL, 2},
 		{"version", no_argument, NULL, 'V'},
 		{"dry-run", no_argument, NULL, 'n'},
+		{"no-sync", no_argument, NULL, 'N'},
 		{"progress", no_argument, NULL, 'P'},
 		{"debug", no_argument, NULL, 3},
 		{NULL, 0, NULL, 0}
@@ -124,7 +128,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	while ((c = getopt_long(argc, argv, "D:nP", long_options, _index)) != -1)
+	while ((c = getopt_long(argc, argv, "D:nNP", long_options, _index)) != -1)
 	{
 		switch (c)
 		{
@@ -140,6 +144,10 @@ main(int argc, char **argv)
 dry_run = true;
 break;
 
+			case 'N':
+do_sync = false;
+break;
+
 			case 3:
 debug = true;
 break;
@@ -701,50 +709,13 @@ updateControlFile(ControlFileData *ControlFile)
  *
  * We do this once, for the whole data directory, for performance reasons.  At
  * the end of pg_rewind's run, the kernel is likely to already have flushed
- * most dirty buffers to disk. Additionally initdb -S uses a two-pass approach
- * (only initiating writeback in the first pass), which often reduces the
- * overall amount of IO noticeably.
+ * most dirty buffers to disk.
  */
 static void
 syncTargetDirectory(const char *argv0)
 {
-	int			ret;
-#define MAXCMDLEN (2 * MAXPGPATH)
-	char		exec_path[MAXPGPATH];
-	char		cmd[MAXCMDLEN];
-
-	/* locate initdb binary */
-	if ((ret = find_other_exec(argv0, "initdb",
-			   "initdb (PostgreSQL) " PG_VERSION "\n",
-			   exec_path)) < 0)
-	{
-		char		full_path[MAXPGPATH];
-
-		if (find_my_exec(argv0, full_path) < 0)
-			strlcpy(full_path, progname, sizeof(full_path));
-
-		if (ret == -1)
-			pg_fatal("The program \"initdb\" is needed by %s but was\n"
-	 "not found in the same directory as \"%s\".\n"
-	 "Check your installation.\n", progname, full_path);
-		else

Re: documentation fixes for partition pruning, round two

[Amit Langote]

Hi Justin.

Thanks for writing the patch.  I have a couple of comments.

On 2018/05/24 8:31, Justin Pryzby wrote:
> On Thu, May 24, 2018 at 10:46:38AM +1200, David Rowley wrote:
>> On 24 May 2018 at 09:35, Justin Pryzby  wrote:
>>> On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
 I reread this and have some more comments.
 https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
>>>
 Let me know if it's useful to provide a patch.
>>>
>>> I propose this.
>>
>> Thanks for working on this.
>>
>> Can you just attach the patch?
> 
> Attached.

-behind-the-scenes; however, it is not possible to use some of the
-inheritance features discussed in the previous section with partitioned
-tables and partitions.  For example, a partition cannot have any parents
+behind-the-scenes; however, it is not possible to use some of the generic
+features of inheritance (discussed below) with declaratively partitioned
+tables or their partitions For example, a partition cannot have any
parents

As I recall, I had written the "previous section" in the original text to
mean 5.9 Inheritance

https://www.postgresql.org/docs/devel/static/ddl-inherit.html

Although, we do list some inheritance features that cannot be used with
declarative partitioned tables on the same page in 5.10.3, so what you
have here may be fine.

+possible to show the difference between a plan whose partitions have been
+pruned and one whose partitions haven't.  A typical unoptimized plan for
+this type of table setup is:

"a plan whose partitions have been pruned" sounds a bit off; maybe, "a
plan in which partitions have been pruned".

+ controlled ruled by the enable_partition_pruning

controlled ruled by -> still controlled by

-pruning uses the table's partitioning constraint, which exists only in
-the case of declarative partitioning.
...
+pruning uses the table's partitioning bounds, which exists only in
+the case of declarative partitioning.

Maybe say "partition bounds" here if change it at all.

Thanks,
Amit

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 22, 2018 at 11:31:08PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Fri, May 18, 2018 at 10:28:12AM -0400, Tom Lane wrote:
> >> While we're all griping about omissions from the release notes ...
> >> I think you should have documented that we fixed plpgsql to cope
> >> (or cope better, at least) with intrasession changes in the rowtypes
> >> of composite-type variables.
> 
> > It was fixed in this commit, right?
> > commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496
> 
> That was the main one, anyway.

OK, added.  Applied patch attached.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +
diff --git a/doc/src/sgml/release-11.sgml b/doc/src/sgml/release-11.sgml
new file mode 100644
index 05fd23f..7599c6f
*** a/doc/src/sgml/release-11.sgml
--- b/doc/src/sgml/release-11.sgml
*** same commits as above
*** 1982,1987 
--- 1982,2004 
  

  
+ 
+
+ Allow PL/pgSQL to handle changes to composite types (e.g. record,
+ row) that happen between the first and later function executions
+ in the same session (Tom Lane)
+
+ 
+
+ Previously such circumstances generated errors.
+
+   
+ 
+   
+ 
  

Re: Possible bug in logical replication.

[Kyotaro HORIGUCHI]

Hello.

At Wed, 23 May 2018 15:56:22 +0900, Masahiko Sawada  
wrote in 
> > So directly set ctx->reader->EndRecPtr by startlsn fixes the
> > problem, but I found another problem here.
> 
> I confirmed that the attached patch fixes this problem as well as the
> same problem reported on another thread.
> 
> I'm not sure it's a good approach to change the state of xlogreader
> directly in the replication slot codes because it also means that we
> have to care about this code as well when xlogreader code is changed.

XLogReadRecrod checks whether state->ReadRecPtr is invalid or not
in the case and works as the same to the explicit LSN case if
so. That is suggesting the usage. (I found no actual use case,
though.) It seems somewhat uneasy also to me, though..

> Another possible way might be to make XLogFindNextRecord valid in
> backend code and move startlsn to the first valid record with an lsn
> >= startlsn by using that function. Please find attached patch.

The another reason for the code is the fact that confirmed_lsn is
storing EndRecPtr after the last XLogReadRecord call. That is,
from the definition, confirmed_lsn must be on the start of a
record or page boundary and error out if not. For that reason,
calling XLogFindNextRecord would not be the right thing to do
here. We should just skip a header if we are on a boundary but it
already done in XLogReadRecord.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

RE: [HACKERS] Transactions involving multiple postgres foreign servers

[Tsunakawa, Takayuki]

From: Masahiko Sawada [mailto:sawada.m...@gmail.com]
> > I'm for the latter.  That is, COMMIT or PREPARE TRANSACTION statement
> issued from an application reports an error.
> 
> I'm not sure that we should end up with an error in such case, but if
> we want then we can raise an error when the transaction tries to
> modify 2PC-non-capable server after modified 2PC-capable server.

Such early reporting would be better, but I wonder if we can handle the 
opposite order: update data on a 2PC-capable server after a 2PC-non-capable 
server.  If it's not easy or efficient, I think it's enough to report the error 
at COMMIT and PREPARE TRANSACTION, just like we report "ERROR:  cannot PREPARE 
a transaction that has operated on temporary tables" at PREPARE TRANSACTION.


> Honestly I'm not sure we should use atomic commit by default at this
> point. Because it also means to change default behavior though the
> existing users use them without 2PC. But I think control of global
> transaction atomicity by GUC parameter would be a good idea. For
> example, synchronous_commit = 'global' makes backends wait for
> transaction to be resolved globally before returning to the user.

Regarding the incompatibility of default behavior, Postgres has the history to 
pursue correctness and less confusion, such as the handling of backslash 
characters in strings and automatic type casts below.

Non-character data types are no longer automatically cast to TEXT (Peter, Tom)
Previously, if a non-character value was supplied to an operator or function 
that requires text input, it was automatically cast to text, for most (though 
not all) built-in data types. This no longer happens: an explicit cast to text 
is now required for all non-character-string types. ... The reason for the 
change is that these automatic casts too often caused surprising behavior.


> I might not understand your comment correctly but the current patch is
> implemented in such way. The patch introduces new FDW APIs:
> PrepareForeignTransaction, EndForeignTransaction,
> ResolvePreparedForeignTransaction and GetPreapreId. The postgres core
> calls each APIs at appropriate timings while managing each foreign
> transactions. FDWs that don't support 2PC set the function pointers of
> them to NULL.

Ouch, you are right.


> Also, regarding the current API design it might not fit to other
> databases than PostgreSQL. For example, in MySQL we have to start xa
> transaction explicitly using by XA START whereas PostgreSQL can
> prepare the transaction that is started by BEGIN TRANSACTION. So in
> MySQL global transaction id is required at beginning of xa
> transaction. And we have to execute XA END is required before we
> prepare or commit it at one phase. So it would be better to define
> APIs according to X/Open XA in order to make it more general.

I thought of:

* Put the functions that implement xa_prepare(), xa_commit() and xa_rollback() 
in libxa.so or libpq.so.
* PrepareForeignTransaction and EndForeignTransaction for postgres_fdw call 
them.

I meant just code reuse for Postgres.  But this is my simple intuition, so 
don't mind.

Regards
Takayuki Tsunakawa

RE: Update backend/lib/README

[Ideriha, Takeshi]

>Seems reasonable. Pushed, thanks!
>
>- Heikki
>
Thanks for the quick work!

Ideriha, Takeshi 

Re: SCRAM with channel binding downgrade attack

[Michael Paquier]

On Wed, May 23, 2018 at 06:41:16PM -0400, Bruce Momjian wrote:
> I can imagine someone wanting both checks so merging them into a single
> options seems unwise, as Magnus mentioned.

FWIW, definitely agreed.
--
Michael


signature.asc
Description: PGP signature

Re: -D option of pg_resetwal is only works with absolute path

[Michael Paquier]

On Wed, May 23, 2018 at 11:40:48AM -0400, David Steele wrote:
> On 5/23/18 11:35 AM, Tom Lane wrote:
>> I pushed a patch already, although certainly an additional pair of eyes
>> on the issue would be good.
> 
> Looks good to me.  Thanks!

The TAP tests cannot catch that as all paths are absolute in
PostgresNode.pm.  We could easily have tests using chdir though.
Thoughts?
--
Michael


signature.asc
Description: PGP signature

Re: [PATCH v14] GSSAPI encryption support

[Michael Paquier]

On Wed, May 23, 2018 at 04:03:12PM -0400, Stephen Frost wrote:
>> Finally, I've submitted this as a single patch because it was requested
>> previously.  I'm happy to break it apart into many commits instead, if
>> that's helpful.
> 
> Please be sure to register this on the commitfest app.  I'm definitely
> interested and likely will have some time to take a look at this before
> then, but we should get it registered there in any case.

This patch got a mention in one of my slides for next week actually,
glad to see this come back.  You will need to wait a it for reviews
though, as the actual focus is to stabilize v11.
--
Michael


signature.asc
Description: PGP signature

Re: [PATCH v14] GSSAPI encryption support

[Thomas Munro]

On Thu, May 24, 2018 at 8:00 AM, Robbie Harwood  wrote:
> Zombie patch is back from the dead.

Hi Robbie,

Robots[1] vs zombies:

+ $postgres->RemoveFile('src/backennd/libpq/be-gssapi-common.c');

Typo, breaks on Windows.

runtime.sgml:2032: parser error : Opening and ending tag mismatch:
para line 2026 and sect1
 
 ^

Docs malformed.

[1] http://cfbot.cputube.org/robbie-harwood.html

-- 
Thomas Munro
http://www.enterprisedb.com

Re: documentation fixes for partition pruning, round two

[Justin Pryzby]

On Thu, May 24, 2018 at 10:46:38AM +1200, David Rowley wrote:
> On 24 May 2018 at 09:35, Justin Pryzby  wrote:
> > On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
> >> I reread this and have some more comments.
> >> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
> >
> >> Let me know if it's useful to provide a patch.
> >
> > I propose this.
> 
> Thanks for working on this.
> 
> Can you just attach the patch?

Attached.

Justin
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2cd0b8a..31f3438 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2967,70 +2967,70 @@ VALUES ('Albany', NULL, NULL, 'NY');

 

 It is not possible to turn a regular table into a partitioned table or
 vice versa.  However, it is possible to add a regular or partitioned table
 containing data as a partition of a partitioned table, or remove a
 partition from a partitioned table turning it into a standalone table;
 see  to learn more about the
 ATTACH PARTITION and DETACH PARTITION
 sub-commands.

 

 Individual partitions are linked to the partitioned table with inheritance
-behind-the-scenes; however, it is not possible to use some of the
-inheritance features discussed in the previous section with partitioned
-tables and partitions.  For example, a partition cannot have any parents
+behind-the-scenes; however, it is not possible to use some of the generic
+features of inheritance (discussed below) with declaratively partitioned
+tables or their partitions For example, a partition cannot have any parents
 other than the partitioned table it is a partition of, nor can a regular
-table inherit from a partitioned table making the latter its parent.
-That means partitioned tables and partitions do not participate in
-inheritance with regular tables.  Since a partition hierarchy consisting
-of the partitioned table and its partitions is still an inheritance
-hierarchy, all the normal rules of inheritance apply as described in
+table inherit from a partitioned table making the latter its parent.  That
+means partitioned tables and partitions do not participate in inheritance
+with regular tables.  Since a partition hierarchy consisting of the
+partitioned table and its partitions is still an inheritance hierarchy, all
+the normal rules of inheritance apply as described in
  with some exceptions, most notably:
 
 
  
   
Both CHECK and NOT NULL
constraints of a partitioned table are always inherited by all its
partitions.  CHECK constraints that are marked
NO INHERIT are not allowed to be created on
partitioned tables.
   
  
 
  
   
Using ONLY to add or drop a constraint on only the
partitioned table is supported when there are no partitions.  Once
partitions exist, using ONLY will result in an error
as adding or dropping constraints on only the partitioned table, when
-   partitions exist, is not supported.  Instead, constraints can be added
-   or dropped, when they are not present in the parent table, directly on
-   the partitions.  As a partitioned table does not have any data
-   directly, attempts to use TRUNCATE
+   partitions exist, is not supported.  Instead, constraints on the
+   partitions themselves can be added and (if they are not present in the
+   parent table) dropped.  As a partitioned table does not
+   have any data directly, attempts to use TRUNCATE
ONLY on a partitioned table will always return an
error.
   
  
 
  
   
Partitions cannot have columns that are not present in the parent.  It
-   is neither possible to specify columns when creating partitions with
-   CREATE TABLE nor is it possible to add columns to
+   is not possible to specify columns when creating partitions with
+   CREATE TABLE, nor is it possible to add columns to
partitions after-the-fact using ALTER TABLE.  Tables 
may be
added as a partition with ALTER TABLE ... ATTACH 
PARTITION
only if their columns exactly match the parent, including any
oid column.
   
  
 
  
   
You cannot drop the NOT NULL constraint on a
partition's column if the constraint is present in the parent table.
   
  
 
@@ -3347,37 +3347,37 @@ ALTER TABLE measurement ATTACH PARTITION 
measurement_y2008m02
on individual partitions, not the partitioned table.
   
  
 
 
 

 

 Implementation Using Inheritance
 
  While the built-in declarative partitioning is suitable for most
  common use cases, there are some circumstances where a more flexible
  approach may be useful.  Partitioning can be implemented using table
- inheritance, which allows for 

Re: [PATCH] (Windows) psql echoes password when reading from pipe

[Tom Lane]

Matthew Stickney  writes:
> The attached is a patch that uses the _fileno/_get_osfhandle approach. 
> This doesn't address the stdin fallback, or error handling if opening 
> termin fails; however, it should be no worse than what's there now, and 
> it fixes the immediate problem.

LGTM; pushed with some cosmetic changes.

> I'm still thinking about the fallback in terms of a mintty-type 
> environment, but I wanted to get this patch out there in the meantime.

Yeah.  I'm not absolutely sure that we need to do anything there,
given the lack of field complaints; but if we can think of a simple
way to make it more bulletproof, all the better.

regards, tom lane

Re: documentation fixes for partition pruning, round two

[David Rowley]

On 24 May 2018 at 09:35, Justin Pryzby  wrote:
> On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
>> I reread this and have some more comments.
>> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
>
>> Let me know if it's useful to provide a patch.
>
> I propose this.

Thanks for working on this.

Can you just attach the patch?

Personally, for me, it's much easier to review when applied rather
than looking at an email.

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: Subplan result caching

[David Rowley]

On 24 May 2018 at 04:25, Tom Lane  wrote:
> That's doable no doubt, but I wonder whether that leaves you in a place
> that's any better than the plan-time-decorrelation approach you proposed
> in the earlier thread.  I liked that better TBH; this one seems like
> a very ad-hoc reinvention of a hash join.  I don't especially like the
> unpredictable number of executions of the subquery that it results in,
> either.

Decorrelation is not always going to be the answer. There's going to
be plenty of cases where that makes the plan worse.

Consider:

SELECT * FROM sometable s WHERE rarelytrue AND y = (SELECT MAX(x) FROM
bigtable b WHERE b.z = s.z);

If the planner went and re-wrote that to execute as the following would;

SELECT * FROM sometable s LEFT JOIN (SELECT z,MAX(x) max FROM bigtable
GROUP BY z) b ON b.z = s.z
WHERE rarelytrue AND y = b.max;

then we've probably gone and built most of the groups for nothing.

The planner would have do this based on estimated costs.  Having the
ability to apply either of these optimisations would be useful,
providing the planner applied them correctly. However, I don't think
Heikki should be touching the decorrelation as part of this effort.

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: SCRAM with channel binding downgrade attack

[Bruce Momjian]

On Wed, May 23, 2018 at 11:15:28AM +0200, Magnus Hagander wrote:
> On Wed, May 23, 2018 at 11:08 AM, Heikki Linnakangas  wrote:
> SCRAM, even without channel binding, does prove that you're talking to the
> correct server. Or to be precise, it proves to the client, that the server
> also knows the password, so assuming that you're using strong passwords 
> and
> not sharing them across servers, you know that you're talking to the
> correct server.
> 
> Right. It provides a very different guarantee from what ssl certs provide. 
> They
> are not replaceable, or mutually exclusive. Trying to force those into a 
> single
> configuration parameter doesn't make a lot of sense IMO.

True.  sslmode is checking the the SSL endpoint with which you have a
shared secret has access to the private key of a server certificate that
is signed by a trusted CA, and perhaps the certificate's subject name
also matches the hostname.

With channel binding, you are proving that the SSL endpoint with which
you have a shared secret has access to the user password hash.

I can imagine someone wanting both checks so merging them into a single
options seems unwise, as Magnus mentioned.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: printf format selection vs. reality

[Thomas Munro]

On Thu, May 24, 2018 at 8:28 AM, Tom Lane  wrote:
> The problem is to get a compiler that thinks that %z is a violation
> of *any* archetype.  gaur's compiler does think that, but it has no
> archetype that does accept %z, so that's little help (I've had it
> building with -Wno-format since we added %z).

>From the pie-in-the-sky department:  I think it would be cool to
develop a Clang semantic checker plugin[1] that performs arbitrary
checks to our taste, as an external project.  Custom format string
checkers might not be terribly interesting but would make an easy
starting point (maybe start by stealing some code from
lib/Sema/SemaChecking.cpp?), but there are plenty of less localised
programming rules in our project that are easy to break (stuff about
node types etc).  I've seen this sort of thing done to impose house
rules on mountains of C++ code with good effect.  You can either
invent your own attributes or (to avoid having to change the target
source tree at all) just hard code your checker to recognise stuff.
It's considerably easier to do this with the full AST etc than with
(say) checker scripts operating on the source.  I'm not working on
this myself but I thought I'd mention it in case it interests someone
out there...

[1] https://clang.llvm.org/docs/ClangPlugins.html

-- 
Thomas Munro
http://www.enterprisedb.com

Re: printf format selection vs. reality

[Tom Lane]

Sigh, I'm an idiot.  I forgot that USE_REPL_SNPRINTF doesn't just
replace snprintf, it replaces the entire *printf family; see
port.h lines 137ff.  So actually we're OK as far as these %z and
argument-reordering concerns go.  Maybe the comments in configure
could use a bit of work though.

There's maybe also an argument for reverting b929614f5, because
actually that code did do something useful, ie allow us to work on
platforms without %ll.  But I'm inclined to leave that alone;
it's an extra configure test to detect a case that probably no longer
occurs in the wild.  Moreover, since %ll and %z are both C99-isms,
and the former had considerable currency even before C99 (evidence:
gaur/pademelon) it's pretty hard to credit that a platform's *printf
would fail the %ll test yet pass the %z test.  So I think we're
likely OK without it, even on dinosaur platforms.

regards, tom lane

documentation fixes for partition pruning, round two

[Justin Pryzby]

On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
> I reread this and have some more comments.
> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html

> Let me know if it's useful to provide a patch.

I propose this.

There's two other, wider changes to consider:

 - should "5.10.4. Partition Pruning" be moved after "5.10.2. Declarative
   Partitioning", rather than after "5.10.3. Implementation Using Inheritance" ?
 - should we find a unified term for "inheritence-based partitioning" and avoid
   using the word "partitioning" in that context?  For example: "Partitioning
   can be implemented using table inheritance[...]".  One possible phrase
   currently begin used is: "legacy inheritance method".

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2cd0b8a..6e1ade9 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2967,70 +2967,70 @@ VALUES ('Albany', NULL, NULL, 'NY');

 

 It is not possible to turn a regular table into a partitioned table or
 vice versa.  However, it is possible to add a regular or partitioned table
 containing data as a partition of a partitioned table, or remove a
 partition from a partitioned table turning it into a standalone table;
 see  to learn more about the
 ATTACH PARTITION and DETACH PARTITION
 sub-commands.

 

 Individual partitions are linked to the partitioned table with inheritance
-behind-the-scenes; however, it is not possible to use some of the
-inheritance features discussed in the previous section with partitioned
-tables and partitions.  For example, a partition cannot have any parents
+behind-the-scenes; however, it is not possible to use some of the generic
+features of inheritance (discussed below) with declaratively partitioned
+tables or their partitions For example, a partition cannot have any parents
 other than the partitioned table it is a partition of, nor can a regular
-table inherit from a partitioned table making the latter its parent.
-That means partitioned tables and partitions do not participate in
-inheritance with regular tables.  Since a partition hierarchy consisting
-of the partitioned table and its partitions is still an inheritance
-hierarchy, all the normal rules of inheritance apply as described in
+table inherit from a partitioned table making the latter its parent.  That
+means partitioned tables and partitions do not participate in inheritance
+with regular tables.  Since a partition hierarchy consisting of the
+partitioned table and its partitions is still an inheritance hierarchy, all
+the normal rules of inheritance apply as described in
  with some exceptions, most notably:
 
 
  
   
Both CHECK and NOT NULL
constraints of a partitioned table are always inherited by all its
partitions.  CHECK constraints that are marked
NO INHERIT are not allowed to be created on
partitioned tables.
   
  
 
  
   
Using ONLY to add or drop a constraint on only the
partitioned table is supported when there are no partitions.  Once
partitions exist, using ONLY will result in an error
as adding or dropping constraints on only the partitioned table, when
-   partitions exist, is not supported.  Instead, constraints can be added
-   or dropped, when they are not present in the parent table, directly on
-   the partitions.  As a partitioned table does not have any data
-   directly, attempts to use TRUNCATE
+   partitions exist, is not supported.  Instead, constraints on the
+   partitions themselves can be added and (if they are not present in the
+   parent table) dropped.  As a partitioned table does not
+   have any data directly, attempts to use TRUNCATE
ONLY on a partitioned table will always return an
error.
   
  
 
  
   
Partitions cannot have columns that are not present in the parent.  It
-   is neither possible to specify columns when creating partitions with
-   CREATE TABLE nor is it possible to add columns to
+   is not possible to specify columns when creating partitions with
+   CREATE TABLE, nor is it possible to add columns to
partitions after-the-fact using ALTER TABLE.  Tables 
may be
added as a partition with ALTER TABLE ... ATTACH 
PARTITION
only if their columns exactly match the parent, including any
oid column.
   
  
 
  
   
You cannot drop the NOT NULL constraint on a
partition's column if the constraint is present in the parent table.
   
  
 
@@ -3347,37 +3347,37 @@ ALTER TABLE measurement ATTACH PARTITION 
measurement_y2008m02
on individual partitions, not the partitioned table.
   
  
 
 
 

 

 Implementation Using Inheritance
 
  While 

Re: PG11 jit failing on ppc64el

[Thomas Munro]

On Thu, May 24, 2018 at 9:00 AM, Christoph Berg  wrote:
> Re: Andres Freund 2018-05-23 
> <20180523205521.mdzwldqabriup...@alap3.anarazel.de>
>> What I meant was that I'd conditionally enable it for the other archs
>> when the version is >= 7.
>
> Good idea, but unfortunately there's a bunch of architectures on
> ports.debian.org that llvm hasn't been ported to yet :(, so the
> architecture qualification on the dependencies is still necessary.

BTW It is working on arm64 too, starting with LLVM 6.  5 crashed the
same way as it does on ppc.  See build farm member eelpout which is
running Debian.

-- 
Thomas Munro
http://www.enterprisedb.com

Re: Time to put context diffs in the grave

[Andrew Dunstan]

On Wed, May 23, 2018 at 5:01 PM, Tom Lane  wrote:
> Andrew Dunstan  writes:
>> OK, that's done. Now I think we can get rid of git-external-diff.
>
> I for one rely on that.  I won't tell anyone else what kind of diff
> they have to read, but if you try to tell me what kind of diff I have
> to read, I'm going to complain.
>

OK, then given I have just removed all reference to it in the "Working
with Git" wiki page, maybe it needs  some comments on how to use it
:-)


>> While we're about it, does anyone use make_diff any more? It seems
>> rather ancient and crufty.
>
> Not me, but judging from the README, possibly Bruce still uses it.
> In any case, is it hurting anything?
>

No, just being anally retentive. But then we saw recently how that can
be a good thing :-)

cheers

andrew

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

Re: plperl fails with perl 5.28

[Tom Lane]

Christoph Berg  writes:
> Re: Tom Lane 2018-05-23 <23260.1527026...@sss.pgh.pa.us>
>> but TBH I think someone oughta file a bug report first.

> https://rt.perl.org/Public/Bug/Display.html?id=133220

Thanks.  Looking at this again, it seems like the core of the problem
is that S_run_body's fell-off-the-end behavior is equivalent to an
explicit "exit(0)".  Perhaps it should not be.  I can see the point of
treating "exit(0)" as an unusual quasi-error case, but fell-off-the-end
probably shouldn't be.

However, then somebody would have to look around and see if there are
any other uses of my_exit(0) that need to be rethought ...

regards, tom lane

Re: Time to put context diffs in the grave

[Tom Lane]

Andrew Dunstan  writes:
> OK, that's done. Now I think we can get rid of git-external-diff.

I for one rely on that.  I won't tell anyone else what kind of diff
they have to read, but if you try to tell me what kind of diff I have
to read, I'm going to complain.

> While we're about it, does anyone use make_diff any more? It seems
> rather ancient and crufty.

Not me, but judging from the README, possibly Bruce still uses it.
In any case, is it hurting anything?

regards, tom lane

Re: PG11 jit failing on ppc64el

[Christoph Berg]

Re: Andres Freund 2018-05-23 <20180523205521.mdzwldqabriup...@alap3.anarazel.de>
> What I meant was that I'd conditionally enable it for the other archs
> when the version is >= 7.

Good idea, but unfortunately there's a bunch of architectures on
ports.debian.org that llvm hasn't been ported to yet :(, so the
architecture qualification on the dependencies is still necessary.

Christoph

Re: PG11 jit failing on ppc64el

[Andres Freund]

On 2018-05-23 22:45:26 +0200, Christoph Berg wrote:
> Re: Andres Freund 2018-05-23 
> <38f42310-62ac-48b1-8a83-639b97e5f...@anarazel.de>
> > >I've disabled --with-llvm on all platforms except amd64 i386 now. Will
> > >try talking to the llvm maintainers in Debian to see if we can get
> > >this fixed and have more coverage.
> > 
> > How about making that dependant on the llvm version being < 7?
> 
> It does work on x86 for <7, so the architecture would still need to be
> coded into debian/control and/or debian/rules. Also, we can't depend
> on "llvm (>= 7 if it exists)"...

What I meant was that I'd conditionally enable it for the other archs
when the version is >= 7.

Greetings,

Andres Freund

Re: Time to put context diffs in the grave

[Andrew Dunstan]

On Mon, May 21, 2018 at 10:52 PM, Andres Freund  wrote:
> Hi,
>
> On 2018-05-21 21:51:11 -0400, Andrew Dunstan wrote:
>> We haven't insisted on context diffs in years now, and one of my
>> interlocutors has just turned handsprings trying to follow the advice at
>>  to produce his first
>> patch.
>>
>>
>> Unless someone objects really violently I'm going to rip all that stuff out
>> and let sanity prevail.
>
> Yes. Please.
>


OK, that's done. Now I think we can get rid of git-external-diff.
While we're about it, does anyone use make_diff any more? It seems
rather ancient and crufty.

cheers

andrew

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

Re: plperl fails with perl 5.28

[Christoph Berg]

Re: Tom Lane 2018-05-23 <23260.1527026...@sss.pgh.pa.us>
> but TBH I think someone oughta file a bug report first.

https://rt.perl.org/Public/Bug/Display.html?id=133220

Christoph

Re: PG11 jit failing on ppc64el

[Christoph Berg]

Re: Andres Freund 2018-05-23 <38f42310-62ac-48b1-8a83-639b97e5f...@anarazel.de>
> >I've disabled --with-llvm on all platforms except amd64 i386 now. Will
> >try talking to the llvm maintainers in Debian to see if we can get
> >this fixed and have more coverage.
> 
> How about making that dependant on the llvm version being < 7?

It does work on x86 for <7, so the architecture would still need to be
coded into debian/control and/or debian/rules. Also, we can't depend
on "llvm (>= 7 if it exists)"...

Christoph

Re: printf format selection vs. reality

[Tom Lane]

Alvaro Herrera  writes:
> On 2018-May-23, Tom Lane wrote:
>> The practical alternatives seem to be to avoid %z in frontend code,
>> or to invent a macro SIZE_T_MODIFIER and use it like INT64_MODIFIER.
>> Either one will be extremely error-prone, I'm afraid, unless we can
>> find a way to get compiler warnings for violations.

> Usage of %z outside safe-known seems really limited.  It would be sad to
> force SIZE_T_MODIFIER for elog calls (where it is prevalent) just for
> the benefit of usage outside of elog on fringe platforms -- you're right
> that we do have a few cases of %z under fprintf() already.  The good
> news is that AFAICS those strings are not translatable today, so
> changing only those to SIZE_T_MODIFIER (and leaving alone those using
> elog) is maybe not such a big deal.  I think those are dshash.c, dsa.c,
> slab.c and aset.c only.

Yeah, I just went through things myself, and concluded that right now
the only hazards are in debug code such as dsa_dump().  So I think that
(a) we don't have a problem we have to fix right now, and (b) going
over to SIZE_T_MODIFIER seems like more trouble than it'd be worth.
Still, this seems like something that will certainly bite us eventually
if we don't install some kind of check.

> (I assume without checking that with the stringinfo API it's OK to use %z).

It is, that goes to snprintf.

> Can't we raise warnings on such usages with an archetype change?  (Hm,
> is it possible to change archetype for fprintf?)

The problem is to get a compiler that thinks that %z is a violation
of *any* archetype.  gaur's compiler does think that, but it has no
archetype that does accept %z, so that's little help (I've had it
building with -Wno-format since we added %z).

It might be possible for me to install a fractionally-newer compiler
on gaur's host and get usable warnings that way.  I think however
that a more practical approach is likely to be to depend on the
Windows/gcc buildfarm members, where (if gcc is correctly installed
and doing what it's supposed to) we should find that %z is accepted
by the gnu_printf archetype but not the plain printf one.  So I wish
somebody would try out the patch in <2975.1526862...@sss.pgh.pa.us>
on MinGW.  It would also be good to find out whether MSVC can be
persuaded to check printf strings.

regards, tom lane

PS: per the above, the patch in <2975.1526862...@sss.pgh.pa.us>
would need to be adjusted to use gnu_printf on the stringinfo
functions, if we don't want complaints about %z.  This opens
the question of whether we want to allow %m there ...

Re: printf format selection vs. reality

[Alvaro Herrera]

On 2018-May-23, Tom Lane wrote:

> The practical alternatives seem to be to avoid %z in frontend code,
> or to invent a macro SIZE_T_MODIFIER and use it like INT64_MODIFIER.
> Either one will be extremely error-prone, I'm afraid, unless we can
> find a way to get compiler warnings for violations.

Usage of %z outside safe-known seems really limited.  It would be sad to
force SIZE_T_MODIFIER for elog calls (where it is prevalent) just for
the benefit of usage outside of elog on fringe platforms -- you're right
that we do have a few cases of %z under fprintf() already.  The good
news is that AFAICS those strings are not translatable today, so
changing only those to SIZE_T_MODIFIER (and leaving alone those using
elog) is maybe not such a big deal.  I think those are dshash.c, dsa.c,
slab.c and aset.c only.

(I assume without checking that with the stringinfo API it's OK to use %z).

Can't we raise warnings on such usages with an archetype change?  (Hm,
is it possible to change archetype for fprintf?)

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [PATCH v14] GSSAPI encryption support

[Stephen Frost]

Greetings Robbie!

* Robbie Harwood (rharw...@redhat.com) wrote:
> Zombie patch is back from the dead.  It's been a bit more than two years
> since v12 (the last major revision) and almost three since it was
> originally submitted.  (I do have enough pride to point out that it did
> not actually /take/ anywhere close to two years to update it.)

Neat!

[...]

> Finally, I've submitted this as a single patch because it was requested
> previously.  I'm happy to break it apart into many commits instead, if
> that's helpful.

Please be sure to register this on the commitfest app.  I'm definitely
interested and likely will have some time to take a look at this before
then, but we should get it registered there in any case.

Thanks!

Stephen


signature.asc
Description: PGP signature

Re: [PATCH v14] GSSAPI encryption support

[Robbie Harwood]

Hello -hackers,

Zombie patch is back from the dead.  It's been a bit more than two years
since v12 (the last major revision) and almost three since it was
originally submitted.  (I do have enough pride to point out that it did
not actually /take/ anywhere close to two years to update it.)

CC'd are reviewers from before; I appreciate their input from before,
but there is of course no obligation for them to page all this back in,
especially if they don't want to.  A large chunk of this code is
unchanged from previous iterations of the patch, but this is a major
re-architect.  Various things have also been previously fixed as part of
the GSSAPI testing efforts, for which I am grateful.

So: this is GSSAPI encryption support for libpq.  Based on feedback on
previous versions, GSSAPI encryption has a separate negotiation step -
similar to SSL negotiation.  I've tried to incorporate all other
feedback I've received thus far, but very likely missed things (and
introduced new problems).

To actually see encryption, you'll first need to configure the server as
for GSSAPI authentication.  You'll also need to ensure the HBA
configuration has a rule that will permit it.  However, there should
hopefully be enough information to set this up in the corresponding docs
changes (and if there isn't, I should fix it).  The Kerberos/GSSAPI
implementation shouldn't matter, but I am testing using MIT krb5
(through freeIPA); I wrote a post a while back for my setup here:
https://mivehind.net/2015/06/11/kerberized-postgresql/

Finally, I've submitted this as a single patch because it was requested
previously.  I'm happy to break it apart into many commits instead, if
that's helpful.

Thanks,
--Robbie


signature.asc
Description: PGP signature
>From 45de59244e4b9ef887cf910a17cbe63c9043f17e Mon Sep 17 00:00:00 2001
From: Robbie Harwood 
Date: Thu, 10 May 2018 16:12:03 -0400
Subject: [PATCH] libpq GSSAPI encryption support

On both the frontend and backend, prepare for GSSAPI encryption
support by moving common code for error handling into a separate file.
Fix a TODO for handling multiple status messages in the process.
Eliminate the OIDs, which have not been needed for some time.

Add frontend and backend encryption support functions.  Keep the
context initiation for authentication-only separate on both the
frontend and backend in order to avoid concerns about changing the
requested flags to include encryption support.

In postmaster, pull GSSAPI authorization checking into a shared
function.  Also share the initiator name between the encryption and
non-encryption codepaths.

Modify pqsecure_write() to take a non-const pointer.  The pointer will
not be modified, but this change (or a const-discarding cast, or a
malloc()+memcpy()) is necessary for GSSAPI due to const/struct
interactions in C.

For HBA, add "hostgss" and "hostnogss" entries that behave similarly
to their SSL counterparts.  "hostgss" requires either "gss", "trust",
or "reject" for its authentication.

Simiarly, add a "gssmode" parameter to libpq.  Supported values are
"disable", "require", and "prefer".  Notably, negotiation will only be
attempted if credentials can be acquired.  Move credential acquisition
into its own function to support this behavior.

Finally, add documentation for everything new, and update existing
documentation on connection security.

Thanks to Michael Paquier for the Windows fixes.
---
 doc/src/sgml/client-auth.sgml   |  75 --
 doc/src/sgml/libpq.sgml |  60 -
 doc/src/sgml/runtime.sgml   |  80 +-
 src/backend/libpq/Makefile  |   4 +
 src/backend/libpq/auth.c| 103 +++
 src/backend/libpq/be-gssapi-common.c|  64 +
 src/backend/libpq/be-gssapi-common.h|  26 ++
 src/backend/libpq/be-secure-gssapi.c| 319 ++
 src/backend/libpq/be-secure.c   |  16 ++
 src/backend/libpq/hba.c |  51 +++-
 src/backend/postmaster/pgstat.c |   3 +
 src/backend/postmaster/postmaster.c |  64 -
 src/include/libpq/hba.h |   4 +-
 src/include/libpq/libpq-be.h|  11 +-
 src/include/libpq/libpq.h   |   3 +
 src/include/libpq/pqcomm.h  |   5 +-
 src/include/pgstat.h|   3 +-
 src/interfaces/libpq/Makefile   |   4 +
 src/interfaces/libpq/fe-auth.c  |  90 +--
 src/interfaces/libpq/fe-connect.c   | 232 +++-
 src/interfaces/libpq/fe-gssapi-common.c | 128 +
 src/interfaces/libpq/fe-gssapi-common.h |  23 ++
 src/interfaces/libpq/fe-secure-gssapi.c | 343 
 src/interfaces/libpq/fe-secure.c|  16 +-
 src/interfaces/libpq/libpq-fe.h |   3 +-
 src/interfaces/libpq/libpq-int.h|  30 ++-
 src/tools/msvc/Mkvcbuild.pm |  20 +-
 27 files changed, 1578 insertions(+), 202 deletions(-)
 create mode 100644 src/backend/libpq/be-gssapi-common.c
 create mode 

Re: Error on vacuum: xmin before relfrozenxid

[Paolo Crosato]

2018-05-23 20:32 GMT+02:00 Andres Freund :

> On 2018-05-22 16:39:58 -0700, Andres Freund wrote:
> > Hi,
> >
> > On 2018-05-23 00:04:26 +0200, Paolo Crosato wrote:
> > > I managed to recover the log of the first time we run into the issue,
> the
> > > error was the same but on template1:
> > >
> > > May  8 11:26:46 xxx postgres[32543]: [1154-1] user=,db=,client= ERROR:
> > > found xmin 2600758304 from before relfrozenxid 400011439
> > > May  8 11:26:46 xxx postgres[32543]: [1154-2] user=,db=,client=
> CONTEXT:
> > > automatic vacuum of table "template1.pg_catalog.pg_authid"
> >
> > pg_authid (along with a few other tables) is shared between
> > databases. So that's just hte same error.  At which rate are you
> > creating / updating database users / roles?
>
> Other questions:
> - did you ever use VACUUM FULL or CLUSTER on pg_authid (or just on all
>   tables)?
> - Did you have any failovers?
> - Do you use logical replication?
>

1) VACUUM FULL was issued after the first time the error occurred, and a
couple of times later. CLUSTER was never run.
2) Several failovers tests were perfomed before the cluster was moved to
production. However, before the move, the whole cluster was wiped,
including all the application and monitoring users. After the db was moved
to production, a couple of users were added without any problem.
3) No, even if the replication level is set to logical in postgresql.conf,
we only use streaming replication.

Best Regards,

Paolo Crosato

Re: [PATCH] (Windows) psql echoes password when reading from pipe

[Matthew Stickney]

The attached is a patch that uses the _fileno/_get_osfhandle approach. 
This doesn't address the stdin fallback, or error handling if opening 
termin fails; however, it should be no worse than what's there now, and 
it fixes the immediate problem.


I'm still thinking about the fallback in terms of a mintty-type 
environment, but I wanted to get this patch out there in the meantime.


-Matt Stickney
From 734b979e64947e1b820f5c1a40454bb35c03 Mon Sep 17 00:00:00 2001
From: Matthew Stickney 
Date: Mon, 21 May 2018 23:24:34 -0400
Subject: [PATCH] Disable input echo on the console, not stdin.

When data is piped to psql, a handle to stdin will no longer be a handle
to the console; SetConsoleMode will fail, and prompt input will be echoed
to the screen.

Instead, retrieve a handle to the console's input buffer fron termin, and
set the console mode there (this requires write permissions). The code is
now setting the console mode on the same handle it reads from, which should
always be consistent.
---
 src/port/sprompt.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/port/sprompt.c b/src/port/sprompt.c
index 70dfa69d7b..340232a7a2 100644
--- a/src/port/sprompt.c
+++ b/src/port/sprompt.c
@@ -66,8 +66,12 @@ simple_prompt(const char *prompt, char *destination, size_t 
destlen, bool echo)
 *
 * XXX fgets() still receives text in the console's input code page.  
This
 * makes non-ASCII credentials unportable.
+*
+* Note that termin must also be opened with write permissions in order 
for
+* SetConsoleMode() to succeed, even though it's only used for reading 
here.
+*
 */
-   termin = fopen("CONIN$", "r");
+   termin = fopen("CONIN$", "w+");
termout = fopen("CONOUT$", "w+");
 #else
 
@@ -112,7 +116,7 @@ simple_prompt(const char *prompt, char *destination, size_t 
destlen, bool echo)
if (!echo)
{
/* get a new handle to turn echo off */
-   t = GetStdHandle(STD_INPUT_HANDLE);
+   t = (HANDLE)_get_osfhandle(_fileno(termin));
 
/* save the old configuration first */
GetConsoleMode(t, _orig);
-- 
2.16.2.windows.1

Re: printf format selection vs. reality

[Tom Lane]

I wrote:
> I think we should abandon the pretense that we can work with libc
> printfs that accept anything but "l"/"ll", and rip out the excess
> complexity in configure, just setting INT64_MODIFIER to "l" or "ll"
> depending on whether "int64" is "long" or "long long".

I pushed that, but while working on the patch I noticed two other
overly-optimistic assumptions of the same kind:

* If the system's printf doesn't support the "z" length modifier,
we assume we can deal with that by using our own snprintf.  Just
as in the "ll" case, that only works to the extent that "z" is
used only with elog/ereport, not directly in printf or fprintf.
I've not dug for counterexamples, but it's hard to believe there
aren't some now, and even harder to believe we'd not introduce
them in future.

* If the system's printf doesn't support argument reordering (%$n),
we assume we can deal with that by using our own snprintf.  Again,
that's bunk because we apply translation to all sorts of strings
that are given directly to printf or fprintf.

I think that a reasonably painless solution to the second point
is just to reject --enable-nls if printf doesn't support argument
reordering.  This would not even break any buildfarm animals,
AFAICT; all the ones that are testing for the feature are finding
it present.  And, though the feature seems to postdate C99, it's
hard to believe that anybody who'd care about NLS would be using
a platform that hasn't got it.

The %z issue is a good deal stickier, as (a) we do have a surviving
Unix buildfarm animal (gaur/pademelon) that doesn't support %z,
and (b) so far as I can tell from available documentation, Windows
doesn't support it either, until very recently (like VS2017).
If it were just (a), that would probably mean it's time to put
gaur/pademelon out to pasture, but (b) means we have to deal with
this somehow.

The practical alternatives seem to be to avoid %z in frontend code,
or to invent a macro SIZE_T_MODIFIER and use it like INT64_MODIFIER.
Either one will be extremely error-prone, I'm afraid, unless we can
find a way to get compiler warnings for violations.

Also, I'm suspcious that we're going to have to back-patch something
for this.

Thoughts?

regards, tom lane

Re: Error on vacuum: xmin before relfrozenxid

[Andres Freund]

On 2018-05-22 16:39:58 -0700, Andres Freund wrote:
> Hi,
> 
> On 2018-05-23 00:04:26 +0200, Paolo Crosato wrote:
> > I managed to recover the log of the first time we run into the issue, the
> > error was the same but on template1:
> > 
> > May  8 11:26:46 xxx postgres[32543]: [1154-1] user=,db=,client= ERROR:
> > found xmin 2600758304 from before relfrozenxid 400011439
> > May  8 11:26:46 xxx postgres[32543]: [1154-2] user=,db=,client= CONTEXT:
> > automatic vacuum of table "template1.pg_catalog.pg_authid"
> 
> pg_authid (along with a few other tables) is shared between
> databases. So that's just hte same error.  At which rate are you
> creating / updating database users / roles?

Other questions:
- did you ever use VACUUM FULL or CLUSTER on pg_authid (or just on all
  tables)?
- Did you have any failovers?
- Do you use logical replication?

Greetings,

Andres Freund

Re: [PATCH] Clear up perlcritic 'missing return' warning

[Andrew Dunstan]

On Wed, May 23, 2018 at 1:45 PM, Alvaro Herrera
 wrote:
> On 2018-May-23, Andrew Dunstan wrote:
>
>> And yes, the idea is that if we do this then we adopt a perlcritic
>> policy that calls it out when we forget.
>
> If we can have a buildfarm animal that runs perlcritic that starts green
> (and turns yellow with any new critique), with a config that's also part
> of our tree, so we can easily change it as we see fit, it should be
> good.
>


Should be completely trivial to do. We already have the core
infrastructure - I added it a week or two ago.

cheers

andrew

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

Re: Subplan result caching

[Andres Freund]

On 2018-05-23 12:51:38 -0400, Tom Lane wrote:
> Heikki Linnakangas  writes:
> > On 23/05/18 19:25, Tom Lane wrote:
> >> To make this
> >> patch safe, I think you'd need to grovel through the subquery and make
> >> sure that the parameters are only used as inputs to operators that belong
> >> to the type's default btree or hash opfamily.  (Many other cases would
> >> work in practice, but we have no semantic knowledge that would let us be
> >> sure of that.)
> 
> > Hmm. First thing that comes to mind is to use the raw bytes as cache 
> > key, only treating Datums as equal if their binary representation is 
> > identical.
> 
> Ah.  That would work, though it'd make the number of subquery executions
> even less predictable (since some logically-equal values would compare
> as physically unequal).

As long as there's no volatile functions, would anybody care?

Greetings,

Andres Freund

Re: [PATCH] Clear up perlcritic 'missing return' warning

[Alvaro Herrera]

On 2018-May-23, Andrew Dunstan wrote:

> And yes, the idea is that if we do this then we adopt a perlcritic
> policy that calls it out when we forget.

If we can have a buildfarm animal that runs perlcritic that starts green
(and turns yellow with any new critique), with a config that's also part
of our tree, so we can easily change it as we see fit, it should be
good.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [PATCH] Clear up perlcritic 'missing return' warning

[Andrew Dunstan]

On Tue, May 22, 2018 at 4:35 PM, Mike Blackwell  wrote:
> On Tue, May 22, 2018 at 3:32 AM, Michael Paquier 
> wrote:
>>
>>
>>  And this
>> maps with any C code.
>
>
> The important differences here are:
>   *) Declaring a C function as void prevents returning a value.  The intent
> not to return a value is clear to any caller and is enforced by the
> compiler.  There is no equivalent protection in Perl.
>   *) Falling off the end of a C function that returns a type other than void
> has undefined results.  Perl will always return the value of the last
> statement executed.
>
> Because Perl does allow returning a value without explicitly using return,
> it's easy to write code that breaks if an unwary person adds a line to the
> end of the subroutine.  There's a common constructor incantation that has
> this problem.  It's a real gotcha for C programmers just starting to poke
> around in Perl code.
>
> This difference also allows users of .pm modules to abuse the API of a
> method intended to be "void", if the value returned falling off the end
> happens to seem useful, leading to breakage if the method's code changes in
> the future.
>
>>
>> This is most likely going to be forgotten.
>
>
> That's what perlcritic is for. :)
>
> Mike
>

I should also point out that Mike posted on this subject back on May
11, and nobody but me replied.

And yes, the idea is that if we do this then we adopt a perlcritic
policy that calls it out when we forget.

cheers

andrew

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

Re: Subplan result caching

[Tom Lane]

Heikki Linnakangas  writes:
> On 23/05/18 19:25, Tom Lane wrote:
>> To make this
>> patch safe, I think you'd need to grovel through the subquery and make
>> sure that the parameters are only used as inputs to operators that belong
>> to the type's default btree or hash opfamily.  (Many other cases would
>> work in practice, but we have no semantic knowledge that would let us be
>> sure of that.)

> Hmm. First thing that comes to mind is to use the raw bytes as cache 
> key, only treating Datums as equal if their binary representation is 
> identical.

Ah.  That would work, though it'd make the number of subquery executions
even less predictable (since some logically-equal values would compare
as physically unequal).

regards, tom lane

Re: Subplan result caching

[Heikki Linnakangas]

On 23/05/18 19:25, Tom Lane wrote:

Heikki Linnakangas  writes:

I've been working on a patch to add a little cache to SubPlans, to speed
up queries with correlated subqueries, where the same subquery is
currently executed multiple times with the same parameters. The idea is
to cache the result of the subplan, with the correlation vars as the
cache key.


I find this pretty bogus as it stands, because it assumes without proof
that the subquery will deliver identical results for any two parameter
values that are considered equal by the datatype's default equality
operator.  An easy counterexample is a subquery whose result depends on
the text conversion of a float8 parameter: zero and minus zero have
different text forms, but are equal according to float8eq.


Good point.


To make this
patch safe, I think you'd need to grovel through the subquery and make
sure that the parameters are only used as inputs to operators that belong
to the type's default btree or hash opfamily.  (Many other cases would
work in practice, but we have no semantic knowledge that would let us be
sure of that.)


Hmm. First thing that comes to mind is to use the raw bytes as cache 
key, only treating Datums as equal if their binary representation is 
identical.



That's doable no doubt, but I wonder whether that leaves you in a place
that's any better than the plan-time-decorrelation approach you proposed
in the earlier thread.  I liked that better TBH; this one seems like
a very ad-hoc reinvention of a hash join.  I don't especially like the
unpredictable number of executions of the subquery that it results in,
either.


I'd certainly prefer the plan-time-decorrelation, too. But I suspect 
that there's always going to be some cases that cannot be decorrelated, 
where a cache like this would help.


- Heikki

Re: PG11 jit failing on ppc64el

[Andres Freund]

On May 23, 2018 4:59:00 AM PDT, Christoph Berg  wrote:
>Re: Andres Freund 2018-05-22
><20180522151101.drsbh6p7ltxpm...@alap3.anarazel.de>
>> Hi,
>> 
>> On 2018-05-22 16:33:57 +0200, Christoph Berg wrote:
>> > PG 11 beta1 is failing on ppc64el. All Debian/Ubuntu releases
>except
>> > Jessie are affected. My guess is on problems with llvm/jit, because
>of
>> > the C++ style error message (and LLVM is disabled on Jessie).
>> 
>> It was bug in LLVM that's fixed now. I guess you can either disable
>jit
>> on arm or ask the LLVM maintainer to backport it...
>> 
>> r328687 - but the expanded tests created a few problems (windows
>mainly,
>> but somewhere else too), so I'd just backport the actual code change.
>
>Thanks also for the extra details on IRC.
>
>I've disabled --with-llvm on all platforms except amd64 i386 now. Will
>try talking to the llvm maintainers in Debian to see if we can get
>this fixed and have more coverage.

How about making that dependant on the llvm version being < 7?

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Subplan result caching

[Tom Lane]

Heikki Linnakangas  writes:
> I've been working on a patch to add a little cache to SubPlans, to speed 
> up queries with correlated subqueries, where the same subquery is 
> currently executed multiple times with the same parameters. The idea is 
> to cache the result of the subplan, with the correlation vars as the 
> cache key.

I find this pretty bogus as it stands, because it assumes without proof
that the subquery will deliver identical results for any two parameter
values that are considered equal by the datatype's default equality
operator.  An easy counterexample is a subquery whose result depends on
the text conversion of a float8 parameter: zero and minus zero have
different text forms, but are equal according to float8eq.  To make this
patch safe, I think you'd need to grovel through the subquery and make
sure that the parameters are only used as inputs to operators that belong
to the type's default btree or hash opfamily.  (Many other cases would
work in practice, but we have no semantic knowledge that would let us be
sure of that.)

That's doable no doubt, but I wonder whether that leaves you in a place
that's any better than the plan-time-decorrelation approach you proposed
in the earlier thread.  I liked that better TBH; this one seems like
a very ad-hoc reinvention of a hash join.  I don't especially like the
unpredictable number of executions of the subquery that it results in,
either.

regards, tom lane

Re: -D option of pg_resetwal is only works with absolute path

[David Steele]

On 5/23/18 11:35 AM, Tom Lane wrote:

David Steele  writes:

On 5/23/18 10:08 AM, Tom Lane wrote:

Seems to be caused by careless placement of new umask-changing code.
I wonder how many other places that patch broke similarly.



I'll look into this today.


I pushed a patch already, although certainly an additional pair of eyes
on the issue would be good.


Looks good to me.  Thanks!

--
-David
da...@pgmasters.net

Re: -D option of pg_resetwal is only works with absolute path

[Tom Lane]

David Steele  writes:
> On 5/23/18 10:08 AM, Tom Lane wrote:
>> Seems to be caused by careless placement of new umask-changing code.
>> I wonder how many other places that patch broke similarly.

> I'll look into this today.

I pushed a patch already, although certainly an additional pair of eyes
on the issue would be good.

regards, tom lane

Re: -D option of pg_resetwal is only works with absolute path

[David Steele]

On 5/23/18 10:08 AM, Tom Lane wrote:

tushar  writes:

In the  latest PG v11,  found that  -D option of pg_resetwal is only
works with absolute path .. not with relative path


Confirmed here.  This did work in previous releases, so I'd say it's
unquestionably a bug.

[ diffs v10 against head... ]

Seems to be caused by careless placement of new umask-changing code.
I wonder how many other places that patch broke similarly.


I'll look into this today.

--
-David
da...@pgmasters.net

Re: A Japanese-unfriendy error message contruction

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> At Tue, 22 May 2018 14:27:29 -0400, Tom Lane  wrote in 
> <13575.1527013...@sss.pgh.pa.us>
>> * Is it OK for the OCLASS_CLASS-with-subId case to assume that it can
>> tack on "column COLNAME" after the description of the relation containing
>> the column?  Perhaps this is tolerable but I'm not sure.  In English it'd
>> be at least as plausible to write "column COLNAME of ", and
>> maybe there are other languages where there's no good way to deal with
>> the current message structure.

> In Japanese it is written as " RELNAME 'NO'  COLNAME"
> the or just " RELNAME  COLNAME" is no problem.

After thinking about this some more, I'd like to propose that we change
the English output to be "column COLNAME of ", using code
similar to what you suggested for O_POLICY etc.  I know that I've been
momentarily confused more than once by looking at obj_description output
and thinking "what, the whole relation depends on this? ... oh, no, it's
just the one column".  It would be better if the head-word were "column".
If that leads to better translations in other languages, fine, but in
any case this'd be an improvement for English.

> I'll clean-up the two thinkgs and post the result later.

OK, I'll await your patch before doing more here.

regards, tom lane

Re: -D option of pg_resetwal is only works with absolute path

[Tom Lane]

tushar  writes:
> In the  latest PG v11,  found that  -D option of pg_resetwal is only 
> works with absolute path .. not with relative path

Confirmed here.  This did work in previous releases, so I'd say it's
unquestionably a bug.

[ diffs v10 against head... ]

Seems to be caused by careless placement of new umask-changing code.
I wonder how many other places that patch broke similarly.

regards, tom lane

Re: Subplan result caching

[David Rowley]

On 23 May 2018 at 21:31, Heikki Linnakangas  wrote:
> I've been working on a patch to add a little cache to SubPlans, to speed up
> queries with correlated subqueries, where the same subquery is currently
> executed multiple times with the same parameters. The idea is to cache the
> result of the subplan, with the correlation vars as the cache key.
...
> Thoughts?

G'day Sir,

I'm in favour of making improvements here. I had a think about this
and just glanced at the patch to check if you'd done it the way I'd
thought..

I'd thought this might be done with some sort of "LazyMaterialize"
node that could take params and store multiple rows per param set.
That node type could contain all the LRU logic to get rid of the
lesser used items when work_mem begin to fill. If you did it this way
then nodeSubplan.c would need to know nothing about this. The planner
would simply just inject one of these when it thinks some caching
would be wise, similar to how it does with Materialize.
LazyMaterialize would simply check the cache and return those rows, if
they exist, otherwise consult its only subplan to get the rows and
then cache them. If you did it this way, as a followup we could go
plug it into parameterised nested loops to speed up repeated lookups
of the inner side plan. The gains there are probably similar to what
you've mentioned.

What do you think?

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data

[Carter Thaxton]

>
> pg_dump.c:2323:2: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
>   char *filter_clause = NULL;
>   ^
>
> You need to declare this variable at the top of its scope.  If you're
> using GCC or Clang you might consider building with COPT=-Werror so
> that any compiler warnings will stop the build from succeeding.
>
> This doesn't build on Windows[1], probably for the same reason.
>

Done.  And thanks for the tip about COPT=-Werror



>  /*
>   * Is OID present in the list?
> + * Also return extra pointer-sized data by setting extra_data paramter
>   */
>  bool
> -simple_oid_list_member(SimpleOidList *list, Oid val)
> +simple_oid_list_member2(SimpleOidList *list, Oid val, void **extra_data)
>
> I feel like that isn't in the spirit of Lisp "member".  It's now a
> kind of association list.  I wonder if we are really constrained to
> use the cave-man facilities in fe_utils anyway.  Though I suppose this
> list is never going to be super large so maybe the data structure
> doesn't matter too much (famous last words).
>

Yeah, I'm just trying to fit into the surrounding code as much as
possible.  If you have a specific recommendation, I'm all ears.
SimpleOidList is only used by pg_dump, so if we want to rename or refactor
this data structure, it won't have much widespread impact.

And you're right that the list is not going to be particularly large.
Consider that it's already a simple linked-list, and not some more complex
hashtable, for the use cases that it already covers in pg_dump.  For all of
these uses, it will only be as large as the number of options provided on
the command-line.



> + char *where_clause = pg_malloc(strlen(filter_clause) + 8 + 1);
> + strcpy(where_clause, "WHERE (");
> + strcat(where_clause, filter_clause);
> + strcat(where_clause, ")");
>
> pg_dump.c seems to be allowed to use psprintf() which'd be less
> fragile than the above code.
>

Done.  Didn't realize psprintf() was available here.



> typo
>
And fixed typos.

Thanks for the review!


pgdump-include-table-data-where-v3.patch
Description: Binary data

Re: Shared PostgreSQL libraries and symbol versioning

[Pavel Raiskup]

Thanks Stephen,

On Wednesday, May 23, 2018 1:13:08 PM CEST Stephen Frost wrote:
> Greetings,
> 
> * Pavel Raiskup (prais...@redhat.com) wrote:
> > On Thursday, May 10, 2018 12:41:40 PM CEST Pavel Raiskup wrote:
> > > On Monday, April 9, 2018 11:04:33 PM CEST Tom Lane wrote:
> > > > Stephen Frost  writes:
> > > > > * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> > > > >> On 4/5/18 02:04, Pavel Raiskup wrote:
> > > > >>> As a followup thought; there are probably two major obstacles ATM
> > > > >>> - the DSOs' symbols are not yet versioned, and
> > > > >>> - the build-system doesn't seem to know how to -lpq link against
> > > > >>> external libpq.so
> > > > 
> > > > > I've not looked but neither of those strike me as terribly difficult 
> > > > > to
> > > > > overcome, assuming they need to be overcome.
> > > > 
> > > > I'm not excited about introducing YA cross-platform difference in our
> > > > library/linking behavior without fairly strong evidence that it's
> > > > necessary.  The available evidence points in the other direction.
> > > 
> > > Well, but I believe it is really needed (in our case at least) - it's so
> > > important that we think about doing the symbol versioning downstream-only.
> > 
> > Gently ping on this; sum-up is that we'll move libpq.so.5 into 'libpq'
> > package (with corresponding libpq-devel having libpq.so).  That package is
> > likely to receive PG major version updates during one version of
> > Fedora/CentOS/RHEL version; so because of that the plan is to start symbol
> > versioning like RHPG_10@@..., e.g. 'PQinitOpenSSL@@RHPG_10'.
> > 
> > I'd go with downstream change now, and I'd like to hear voices against this
> > change (if there is anything obvious to you).  I'd like to help having this
> > as upstream opt-in of course, too, so feel free to ping me any time in 
> > future.
> 
> The way to try and move this forward would be to specifically address
> Tom's concern about having more differences in our library/linking
> behavior across platforms.

It's not really that different.  Linking is done the same way, except that -
when available - symbol versions are used, too (we use --version-script GNU ld
option anyways).

> Implementing this change in a way that makes it very specific to one platform
> is surely going in the wrong direction.

Please elaborate.  It shouldn't really cause troubles IMO.

> Have you tried talking to the Debian and the maintainers of other platforms to
> get their input on such a change?  Are they supportive of it, would they like
> to see upstream do it, what would make sense here if upstream did it?

CCing Christoph.

> Are there still platforms we care about which wouldn't be able to support
> this?

I think so - if there's no such linker support, but it shouldn't really matter.

> Is there a way to make it work across all platforms?

No, seems like it's GNU feature, see GNU ld manual [1], plus the dsohowto
reference there.

> The other side of this is: why isn't the packaging system handling this?

It would (RPM), if there were the symbol versions.

> On Debian, at least, if you build a package with a newer version of a
> library, then that package will depend on that newer version to be
> installed; why isn't that enough for the RPM-based systems too?
> Older packages will still run against the newer libpq unless/until we
> make a backwards-incompatible change, which is rather rare.

Interesting, thanks.  How this is implemented?  What you mean by "newer
library" -- new soname, or just a newer package build (without any other
change) e.g.?

> > > As I said, we'd like to provide multiple major PG versions, but only one
> > > set of PG libraries.  But as the time will continue, supporting newer PG
> > > major versions will require updating the system's default 'libpq.so.5',
> > > potentially for the newly added symbols.  If we had in our RPMs versioned
> > > symbols, RPM would for free guard us against wrong package installations
> > > (IOW RPM would guarantee that users won't install packages depending on
> > > newer symbols unless also newer 'libpq.so.5' is installed).  Without
> > > lib symbol versions, there's no **easy** way around this.
> 
> I'm failing to see why symbol versioning is the way to prevent this from
> happening.

That's the RPM automation; more concretely:

1. if libpq.so.5 library provides symbol versions, then the RPM package
   that provides the library also has some info for depsolver (so called
   "Provides") about what symbol versions that particular RPM provides

2. if you build-time link some package against ^^^ that library, then the
   RPM will provide the info for depsolver (so called "Requires") what
   symbol versions that package _requires_.  Note that it's somewhat
   "optimal", because if the package only requires symbols provided by
   v9.6 then -- even though you build against v10 -- the v10 dependancy
   isn't added and your built package can be installed against v9.6

> > 

Re: Compiler warnings with --enable-dtrace

[Christoph Berg]

Re: Peter Geoghegan 2018-05-12 

Re: PG11 jit failing on ppc64el

[Christoph Berg]

Re: Andres Freund 2018-05-22 <20180522151101.drsbh6p7ltxpm...@alap3.anarazel.de>
> Hi,
> 
> On 2018-05-22 16:33:57 +0200, Christoph Berg wrote:
> > PG 11 beta1 is failing on ppc64el. All Debian/Ubuntu releases except
> > Jessie are affected. My guess is on problems with llvm/jit, because of
> > the C++ style error message (and LLVM is disabled on Jessie).
> 
> It was bug in LLVM that's fixed now. I guess you can either disable jit
> on arm or ask the LLVM maintainer to backport it...
> 
> r328687 - but the expanded tests created a few problems (windows mainly,
> but somewhere else too), so I'd just backport the actual code change.

Thanks also for the extra details on IRC.

I've disabled --with-llvm on all platforms except amd64 i386 now. Will
try talking to the llvm maintainers in Debian to see if we can get
this fixed and have more coverage.

Christoph

Re: Subplan result caching

[Vladimir Sitnikov]

>I like the idea.

+1

>Also, there probably should be a GUC for this, defaulting to "off".

I think the feature could be on by default provided it can properly
identify "volatile" functions/tables hidden behind views.


Vladimir

Re: Subplan result caching

[Laurenz Albe]

Heikki Linnakangas wrote:
> I've been working on a patch to add a little cache to SubPlans, to speed 
> up queries with correlated subqueries, where the same subquery is 
> currently executed multiple times with the same parameters. The idea is 
> to cache the result of the subplan, with the correlation vars as the 
> cache key.

I like the idea.

I think the cache should be limited in size, perhaps by work_mem.
Also, there probably should be a GUC for this, defaulting to "off".

Yours,
Laurenz Albe

Subplan result caching

[Heikki Linnakangas]

Hi,

I've been working on a patch to add a little cache to SubPlans, to speed 
up queries with correlated subqueries, where the same subquery is 
currently executed multiple times with the same parameters. The idea is 
to cache the result of the subplan, with the correlation vars as the 
cache key.


That helps a lot, if you happen to have that kind of a query. I bumped 
into this while looking at TPC-DS query 6:


select a.ca_state state, count(*) cnt
 from customer_address a
 ,customer c
 ,store_sales s
 ,date_dim d
 ,item i
 where   a.ca_address_sk = c.c_current_addr_sk
and c.c_customer_sk = s.ss_customer_sk
and s.ss_sold_date_sk = d.d_date_sk
and s.ss_item_sk = i.i_item_sk
and d.d_month_seq =
 (select distinct (d_month_seq)
  from date_dim
   where d_year = 2000
and d_moy = 5 )
and i.i_current_price > 1.2 *
 (select avg(j.i_current_price)
 from item j
 where j.i_category = i.i_category)
 group by a.ca_state
 having count(*) >= 10
 order by cnt

The first subquery is uncorrelated, and is already handled efficiently 
as an InitPlan. This patch helps with the second subquery. There are 
only 11 different categories, but we currently re-execute it for every 
row of the outer query, over 26000 times. (I think I have about 1 GB of 
data in my little database I've been testing with, I'm not sure how this 
would scale with the amount of data.) With this patch, it's only 
executed 11 times, the cache avoids the rest of the executions. That 
brings the runtime, on my laptop, from about 30 s to 120 ms.


For this particular query, I actually wish we could pull up that 
subquery, instead. I did some investigation into that last summer, 
https://www.postgresql.org/message-id/67e353e8-c20e-7980-a282-538779edf25b%40iki.fi, 
but that's a much bigger project. In any case, even if the planner was 
able to pull up subqueries in more cases, a cache like this would still 
be helpful for those cases where pulling up was still not possible.


Thoughts?

- Heikki
>From d3a738ea6427df908b61a5fdd3ee14a7b78b724a Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Tue, 22 May 2018 08:59:33 +0300
Subject: [PATCH 1/3] Add comment to explain that SubPlan can return its
 results in two ways.

It took me a while to understand this, I hope this helps the next person
reading the code to grok it faster.
---
 src/backend/executor/nodeSubplan.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 44f551bcf1..aab1c7942a 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -10,6 +10,12 @@
  * direct correlation variables from the parent plan level), and "regular"
  * subplans, which are re-evaluated every time their result is required.
  *
+ * There are two ways a SubPlan node can return the result.  The first is
+ * that ExecSubPlan() runs the subquery, and returns its result like any
+ * other expression.  The second way is to return the result in executor
+ * Params.  In this method, the execution happens when ExecSetParamPlan() is
+ * called, and the subquery's results are set as the current values of
+ * executor Params, as indicated by SubPlan->setParam.
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
-- 
2.11.0

>From fcf040b589ffc318cbbc060deec296577b3ff24b Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Tue, 22 May 2018 09:05:38 +0300
Subject: [PATCH 2/3] Remove some unused code.

---
 src/backend/executor/nodeSubplan.c | 10 --
 src/include/nodes/execnodes.h  |  1 -
 2 files changed, 11 deletions(-)

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index aab1c7942a..909f77905c 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -802,7 +802,6 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 	sstate->keyColIdx = NULL;
 	sstate->tab_eq_funcoids = NULL;
 	sstate->tab_hash_funcs = NULL;
-	sstate->tab_eq_funcs = NULL;
 	sstate->lhs_hash_funcs = NULL;
 	sstate->cur_eq_funcs = NULL;
 
@@ -900,7 +899,6 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 		lefttlist = righttlist = NIL;
 		sstate->tab_eq_funcoids = (Oid *) palloc(ncols * sizeof(Oid));
 		sstate->tab_hash_funcs = (FmgrInfo *) palloc(ncols * sizeof(FmgrInfo));
-		sstate->tab_eq_funcs = (FmgrInfo *) palloc(ncols * sizeof(FmgrInfo));
 		sstate->lhs_hash_funcs = (FmgrInfo *) palloc(ncols * sizeof(FmgrInfo));
 		sstate->cur_eq_funcs = (FmgrInfo *) palloc(ncols * sizeof(FmgrInfo));
 		i = 1;
@@ -909,7 +907,6 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 			OpExpr	   *opexpr = lfirst_node(OpExpr, l);
 			Expr	   *expr;
 			TargetEntry *tle;

Re: SCRAM with channel binding downgrade attack

[Magnus Hagander]

On Wed, May 23, 2018 at 11:08 AM, Heikki Linnakangas 
wrote:

> On 23/05/18 09:59, Magnus Hagander wrote:
>
>> With that, a connection would be allowed, if either the server's SSL
>>> certificate is verified as with "sslmode=verify-full", *or* SCRAM
>>> authentication with channel binding was used. Or perhaps cram it into
>>> sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a
>>> nicer name. (We can do that after v11 though, I think.)
>>>
>>
>> sslmode=verify-full is very different from SCRAM with channel binding,
>> isn't it? As in, SCRAM with channel binding at no point proves which
>> server
>> you're talking to -- only that you are talking to the SSL endpoint? It
>> could be a rogue SSL endpoint unless you do certificate validation.
>>
>
> SCRAM, even without channel binding, does prove that you're talking to the
> correct server. Or to be precise, it proves to the client, that the server
> also knows the password, so assuming that you're using strong passwords and
> not sharing them across servers, you know that you're talking to the
> correct server.
>

Right. It provides a very different guarantee from what ssl certs provide.
They are not replaceable, or mutually exclusive. Trying to force those into
a single configuration parameter doesn't make a lot of sense IMO.


Channel binding adds the guarantee that the SSL endpoint belongs to the
> same server you're authenticating with, i.e. there is no man in the middle.


 Yeah, it does protect you against things like pgbouncer (a real one or a
rogue one- the rogue one being the mitm attacker). But again, only if you
never share a password, which would be a nice world to live in :)

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

Re: SCRAM with channel binding downgrade attack

[Michael Paquier]

On Wed, May 23, 2018 at 05:56:19PM +0900, Michael Paquier wrote:
> OK, being able to introduce a new default if necessary is a good point.
> Let's introduce a "require" mode then, which uses tls-unique
> underground, while "tls-unique" and "tls-server-end-point" are
> documented as developer-oriented.

By the way, if somebody could review the latest version of the patch
before I write a new version and agrees with the concept introduced
would be nice..  Adding one option is simple enough, making sure that we
agree that the patch is on good tracks is harder.
--
Michael


signature.asc
Description: PGP signature

Re: SCRAM with channel binding downgrade attack

[Heikki Linnakangas]

On 23/05/18 09:59, Magnus Hagander wrote:

With that, a connection would be allowed, if either the server's SSL
certificate is verified as with "sslmode=verify-full", *or* SCRAM
authentication with channel binding was used. Or perhaps cram it into
sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a
nicer name. (We can do that after v11 though, I think.)


sslmode=verify-full is very different from SCRAM with channel binding,
isn't it? As in, SCRAM with channel binding at no point proves which server
you're talking to -- only that you are talking to the SSL endpoint? It
could be a rogue SSL endpoint unless you do certificate validation.


SCRAM, even without channel binding, does prove that you're talking to 
the correct server. Or to be precise, it proves to the client, that the 
server also knows the password, so assuming that you're using strong 
passwords and not sharing them across servers, you know that you're 
talking to the correct server.


Channel binding adds the guarantee that the SSL endpoint belongs to the 
same server you're authenticating with, i.e. there is no man in the middle.


- Heikki

Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data

[Thomas Munro]

On Wed, May 23, 2018 at 5:18 PM, Carter Thaxton
 wrote:
> Ah yes, thanks.  I did in fact have colors enabled.
> I've attached a new patch generated by `git format-patch`.  Hopefully that's
> correct.

pg_dump.c:2323:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
  char *filter_clause = NULL;
  ^

You need to declare this variable at the top of its scope.  If you're
using GCC or Clang you might consider building with COPT=-Werror so
that any compiler warnings will stop the build from succeeding.

This doesn't build on Windows[1], probably for the same reason.

 /*
  * Is OID present in the list?
+ * Also return extra pointer-sized data by setting extra_data paramter
  */
 bool
-simple_oid_list_member(SimpleOidList *list, Oid val)
+simple_oid_list_member2(SimpleOidList *list, Oid val, void **extra_data)

I feel like that isn't in the spirit of Lisp "member".  It's now a
kind of association list.  I wonder if we are really constrained to
use the cave-man facilities in fe_utils anyway.  Though I suppose this
list is never going to be super large so maybe the data structure
doesn't matter too much (famous last words).

+ char *where_clause = pg_malloc(strlen(filter_clause) + 8 + 1);
+ strcpy(where_clause, "WHERE (");
+ strcat(where_clause, filter_clause);
+ strcat(where_clause, ")");

pg_dump.c seems to be allowed to use psprintf() which'd be less
fragile than the above code.

+ /* When match_data is set, split the pattern on the ':' chararcter,

typo

+ * Also return extra pointer-sized data by setting extra_data paramter

typo

[1] https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.311

-- 
Thomas Munro
http://www.enterprisedb.com

Re: SCRAM with channel binding downgrade attack

[Magnus Hagander]

On Wed, May 23, 2018 at 10:56 AM, Michael Paquier 
wrote:

> On Wed, May 23, 2018 at 08:59:43AM +0200, Magnus Hagander wrote:
> > On Wed, May 23, 2018 at 8:46 AM, Heikki Linnakangas 
> wrote:
> >> "tls-unique" and "tls-server-end-point" are overly technical to users.
> >> They don't care which one is used, there's no difference in security.
> >> Furthermore, if we add another channel binding type in the future,
> perhaps
> >> because someone finds a vulnerability in those types and a new one is
> added
> >> to address it, users would surely like to be automatically switched
> over to
> >> the new binding type. If they have "tls-unique" hardcoded in connection
> >> strings, that won't happen.
> >>
> >> So I think the options should be "prefer", "disable", and "require".
> >> That's also symmetrical with sslmode, which is nice.
>
> OK, being able to introduce a new default if necessary is a good point.
> Let's introduce a "require" mode then, which uses tls-unique
> underground, while "tls-unique" and "tls-server-end-point" are
> documented as developer-oriented.


> > As a general point, I still don't like being symmetrical with
> > sslmode=prefer, because that's a silly setting for sslmode :) It
> basically
> > says "I don't care about the security guarantees, I just want the
> overhead
> > please". Now, granted, the over head for SCRAM channel-binding is
> certainly
> > a lot less than it is for TLS. But if we just want to go with symmetry,
> it
> > would perhaps make more sense to implement the "allow" mode which makes
> > more sense on the TLS side as well.
>
> Something that you may be forgetting here is that we still want to be
> able to connect to a v10 backend with default options even with a
> post-v11 libpq.  Or we will get complaints.
>

Right. Having a mode called "allow" and having that be default would work
fine in this case, wouldn't it?

(That is, my suggestion is to implement "allow", "disable" and "require",
and to make "allow" the default)


>> It would be nice to have a libpq setting like
> >> "authenticate_server=require", which would mean "I want
> man-in-the-middle
> >> protection".
> >
> > That seems like a bad name for such a thing. Shouldn't it be
> > "authenticate_server=no_man_in_the_middle" (not those words but you get
> the
> > idea). Specifically, protecting against man in the middle attack does not
> > equal authenticating server -- you can still be connected to the wrong
> > server just with no second attacker between you and the first
> > attacker.
>
> Still that's not something we want for v11, right?
>

Agreed.


>> With that, a connection would be allowed, if either the server's SSL
> >> certificate is verified as with "sslmode=verify-full", *or* SCRAM
> >> authentication with channel binding was used. Or perhaps cram it into
> >> sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a
> >> nicer name. (We can do that after v11 though, I think.)
> >
> > sslmode=verify-full is very different from SCRAM with channel binding,
> > isn't it? As in, SCRAM with channel binding at no point proves which
> server
> > you're talking to -- only that you are talking to the SSL endpoint? It
> > could be a rogue SSL endpoint unless you do certificate validation.
>
> And the reverse is true as well, say the CA is compromised..
>

Well, sure. But scram channel binding doesn't protect you there, so you're
screwed either way if that happens.


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

Re: SCRAM with channel binding downgrade attack

[Michael Paquier]

On Wed, May 23, 2018 at 08:59:43AM +0200, Magnus Hagander wrote:
> On Wed, May 23, 2018 at 8:46 AM, Heikki Linnakangas  wrote:
>> "tls-unique" and "tls-server-end-point" are overly technical to users.
>> They don't care which one is used, there's no difference in security.
>> Furthermore, if we add another channel binding type in the future, perhaps
>> because someone finds a vulnerability in those types and a new one is added
>> to address it, users would surely like to be automatically switched over to
>> the new binding type. If they have "tls-unique" hardcoded in connection
>> strings, that won't happen.
>>
>> So I think the options should be "prefer", "disable", and "require".
>> That's also symmetrical with sslmode, which is nice.

OK, being able to introduce a new default if necessary is a good point.
Let's introduce a "require" mode then, which uses tls-unique
underground, while "tls-unique" and "tls-server-end-point" are
documented as developer-oriented.

> As a general point, I still don't like being symmetrical with
> sslmode=prefer, because that's a silly setting for sslmode :) It basically
> says "I don't care about the security guarantees, I just want the overhead
> please". Now, granted, the over head for SCRAM channel-binding is certainly
> a lot less than it is for TLS. But if we just want to go with symmetry, it
> would perhaps make more sense to implement the "allow" mode which makes
> more sense on the TLS side as well.

Something that you may be forgetting here is that we still want to be
able to connect to a v10 backend with default options even with a
post-v11 libpq.  Or we will get complaints.

>> It would be nice to have a libpq setting like
>> "authenticate_server=require", which would mean "I want man-in-the-middle
>> protection".
> 
> That seems like a bad name for such a thing. Shouldn't it be
> "authenticate_server=no_man_in_the_middle" (not those words but you get the
> idea). Specifically, protecting against man in the middle attack does not
> equal authenticating server -- you can still be connected to the wrong
> server just with no second attacker between you and the first
> attacker.

Still that's not something we want for v11, right?

>> With that, a connection would be allowed, if either the server's SSL
>> certificate is verified as with "sslmode=verify-full", *or* SCRAM
>> authentication with channel binding was used. Or perhaps cram it into
>> sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a
>> nicer name. (We can do that after v11 though, I think.)
> 
> sslmode=verify-full is very different from SCRAM with channel binding,
> isn't it? As in, SCRAM with channel binding at no point proves which server
> you're talking to -- only that you are talking to the SSL endpoint? It
> could be a rogue SSL endpoint unless you do certificate validation.

And the reverse is true as well, say the CA is compromised..
--
Michael


signature.asc
Description: PGP signature

Re: Shared PostgreSQL libraries and symbol versioning

[Pavel Raiskup]

On Thursday, May 10, 2018 12:41:40 PM CEST Pavel Raiskup wrote:
> On Monday, April 9, 2018 11:04:33 PM CEST Tom Lane wrote:
> > Stephen Frost  writes:
> > > * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> > >> On 4/5/18 02:04, Pavel Raiskup wrote:
> > >>> As a followup thought; there are probably two major obstacles ATM
> > >>> - the DSOs' symbols are not yet versioned, and
> > >>> - the build-system doesn't seem to know how to -lpq link against
> > >>> external libpq.so
> > 
> > > I've not looked but neither of those strike me as terribly difficult to
> > > overcome, assuming they need to be overcome.
> > 
> > I'm not excited about introducing YA cross-platform difference in our
> > library/linking behavior without fairly strong evidence that it's
> > necessary.  The available evidence points in the other direction.
> 
> Well, but I believe it is really needed (in our case at least) - it's so
> important that we think about doing the symbol versioning downstream-only.

Gently ping on this; sum-up is that we'll move libpq.so.5 into 'libpq'
package (with corresponding libpq-devel having libpq.so).  That package is
likely to receive PG major version updates during one version of
Fedora/CentOS/RHEL version; so because of that the plan is to start symbol
versioning like RHPG_10@@..., e.g. 'PQinitOpenSSL@@RHPG_10'.

I'd go with downstream change now, and I'd like to hear voices against this
change (if there is anything obvious to you).  I'd like to help having this
as upstream opt-in of course, too, so feel free to ping me any time in future.

Pavel

> I wouldn't bother you much with this, but it's worth keeping you at least
> well informed since - if we moved that direction - there would soon exist
> binaries in the wild linked against versioned PG shared libraries, and
> those would raise ugly runtime linking warnings if used on systems where
> are non-versioned PG libs (it's no problem vice-versa).
> 
> As I said, we'd like to provide multiple major PG versions, but only one
> set of PG libraries.  But as the time will continue, supporting newer PG
> major versions will require updating the system's default 'libpq.so.5',
> potentially for the newly added symbols.  If we had in our RPMs versioned
> symbols, RPM would for free guard us against wrong package installations
> (IOW RPM would guarantee that users won't install packages depending on
> newer symbols unless also newer 'libpq.so.5' is installed).  Without
> lib symbol versions, there's no **easy** way around this.
> 
> Debian folks claim they don't have a problem with symbol versioning even
> though they have the same installation pattern since ever, but IMO that's
> because (as far as I understand how all of this is done on Debian):
> 
> - Debian doesn't have that long support life cycle, so new symbols are
>   to occur less likely
> 
> - Debian actually provides officially only one version of PG stack
>   (including libraries), the rest is provided through postgresql.org
>   repositories (one could say "third party", even though those are
>   probably the same maintainers).  So for Debian, it's really unlikely to
>   build system package against updated 'libpq.so.5' which comes from
>   the "third party" repo.
> 
> I mean, worthless saying that Debian packaging would benefit from
> versioned symbols too (== it would be safe to update system-wide package),
> but it would be really awesome to have consistent (upstream blessed) way
> to do the versioning.
> 
> As for how it would be done downstream-only:  Have a look at the
> 'symbol-versioning' patch attached.  Sorry for me being verbose here and
> there..  It's pretty narrow patch luckily; because the buildsystem is
> already GNU ld friendly.  I implemented the new 'exports.txt' parser in
> relatively portable /bin/sh, but I can (probably less portably) implement
> that in awk too.  Or anything, please tell.
> 
> > As for linking against an external .so, commit dddfc4cb2 just went to
> > some lengths to make sure that that *wouldn't* happen.  But as long
> > as all the builds expect libpq.so to end up in the same place after
> > installation, I doubt it matters much what happens at build time.
> > You just need to control which build actually installs it.
> 
> Agreed, but we can build-time link against system's libpq.so pretty easily
> too.  E.g. by something like the attached 'no-libs' patch.  Could we have
> something like this as upstream ./configure opt-in?
> 
> ---
> Overall, what are your feelings?  I'd be really glad to go through more
> formal patch submission, those attachments are here just to have something 
> more
> concrete in hand for the discussion purposes.
> 
> Pavel
> 
> > regards, tom lane
> > 
> 
> 

Re: pgAdmin4 Docker behind load balancer

[Dave Page]

On Tue, May 22, 2018 at 8:09 PM, Daniel Gustafsson  wrote:

> > On 22 May 2018, at 18:07, Lenain  wrote:
> >
> > Hello hackers,
> >
> > We are currently using the dpage/pgadmin4 image to run a pgAdmin4 web
> interface behind an AWS application load balancer.
> > The load balancer is configured to check the health of containers by
> querying the /login URI and checking if it answers with a 200 HTTP code.
> >
> > However the app always send a new cookie for this page, storing it into
> the mounted docker volume.
> > It is understandable that it is wanted to generate a new session on
> login, but as load balancers check numerous times a day this URI, it
> quickly fill and use all of the inodes of the volume as it generate session
> tokens, and consequently saturate also the inodes of the underlying system.
> >
> > We are therefore looking for another URI to do our healthcheck that
> won't generate a new session item.
> > However it seems that even on statics assets or redirects, the app set
> the pga4_session cookie.
> >
> > Is there another way available to do these checks ? Am I missing
> something ?
>
> This is the mailinglist for the core postgres database server.  While there
> certainly are lots of people skilled in pgadmin here, you will probably
> have a
> better chance of getting help on the pgadmin-support mailinglist:
>
> https://www.postgresql.org/list/pgadmin-support/


+1

Though to save on traffic, /misc/ping should give you what you need.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Error on vacuum: xmin before relfrozenxid

[Paolo Crosato]

2018-05-23 1:39 GMT+02:00 Andres Freund :

> Hi,
>
> On 2018-05-23 00:04:26 +0200, Paolo Crosato wrote:
> > I managed to recover the log of the first time we run into the issue, the
> > error was the same but on template1:
> >
> > May  8 11:26:46 xxx postgres[32543]: [1154-1] user=,db=,client= ERROR:
> > found xmin 2600758304 from before relfrozenxid 400011439
> > May  8 11:26:46 xxx postgres[32543]: [1154-2] user=,db=,client= CONTEXT:
> > automatic vacuum of table "template1.pg_catalog.pg_authid"
>
> pg_authid (along with a few other tables) is shared between
> databases. So that's just hte same error.  At which rate are you
> creating / updating database users / roles?
>

Once or twice a month. Yesterday I updated some rows on the table with a

update pg_auth_id set rolname=rolname where rolname=...

for some users in the hope to work around the issue, but it didn't work.
That's why there are recent xmin on some rows.

Best Regards,

Paolo Crosato

Re: A Japanese-unfriendy error message contruction

[Kyotaro HORIGUCHI]

Thank you for the suggestion.

Is there anyone using another language who is having difficulties
in translating some messages?

At Tue, 22 May 2018 14:27:29 -0400, Tom Lane  wrote in 
<13575.1527013...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > The "policy %s" and " %s" are transposed in Japaese
> > syntax.  Specifically " %s NO  %s" is the
> > natural translation of "policy %s on  %s". But currently
> > we cannot get the natural error message in Japanese.
> 
> > For the reason, I'd like to propose to refactor
> > getObjectDescription:OPCLASS_POLICY as the attached patch. The
> > same structure is seen for OPCLASS_AMOP.
> 
> Taking a quick look through objectaddress.c, I see quite a lot of
> deficiencies:
> 
> * Is it OK for the OCLASS_CLASS-with-subId case to assume that it can
> tack on "column COLNAME" after the description of the relation containing
> the column?  Perhaps this is tolerable but I'm not sure.  In English it'd
> be at least as plausible to write "column COLNAME of ", and
> maybe there are other languages where there's no good way to deal with
> the current message structure.

In Japanese it is written as " RELNAME 'NO'  COLNAME"
the or just " RELNAME  COLNAME" is no problem.

> * Column default values are written as "default for %s" where %s is what
> you get from the above.  This seems likely to be even more awkward; do we
> need to flatten that into one translatable string instead of recursing?
> (At the very least I wish it was "default value for %s".)

I see this is constructed in the following structure.

| (default for ((table t) (column a))) depends on (sequence seq1)

In Japanese, a difference in this sentense is the order of the
outermost blocks so there's no problem here.

The topmost structure is "%s depends on %s" so it doesn't seem
modifiable into plnainer shape without adding duplications..

I agree that "values" make the sentense neater.

> * Collations have namespaces, but the OCLASS_COLLATION code doesn't
> account for that.  Likewise for conversions, extended statistics
> objects, and all four types of text search objects.

I'm not sure it is necessarily required. getObjectDescription
cares only for OPCLASS (and DEFACL), but we could do so for all
possible places with no difficulties.

> * OCLASS_PUBLICATION_REL likewise neglects schema-qualification of the
> relation name.  I wonder why it's not using getRelationDescription, too.

The function takes ObjectAddresss which we don't have there.
It makes the code as the following, the format string looks
somewhat confusing..

| reladdr.classId = RelationRelationId;
| reladdr.objectId = prform->prrelid;
| reladdr.objectSubId = 0;
| appendStringInfo(, _("publication %s in publication %s"),
| getObjectDescription(), pubname)

> * Both OCLASS_AMOP and OCLASS_AMPROC have the problem that they plug
> the translation of "operator family %s for access method %s" into
> " of %s".  I'm not sure how big a problem this is, or whether
> it's worth the code-duplication needed to expose the whole thing as
> one translatable message.  Opfamily members are pretty advanced things,
> so maybe we shouldn't expend too much work on them.  (But if we do,
> we should also take a look at the no-such-member error strings in
> get_object_address_opf_member, which do the same thing.)

Regarding Japanese, it is not a problem. It is specifically the
following

| (operator %d of (operator family %s for access method %s))

It is translated into Japanese as:

| (( %s 

Re: Memory unit GUC range checks

[Heikki Linnakangas]

On 17/05/18 00:56, Alexander Korotkov wrote:

On Wed, May 16, 2018 at 10:41 PM, Andres Freund  wrote:


Generally ok, two minor points:


diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7cd2d2d80e..93402030f7 100644
   {"TB", GUC_UNIT_BLOCKS, (1024 * 1024 * 1024) / (BLCKSZ / 1024)},
   {"GB", GUC_UNIT_BLOCKS, (1024 * 1024) / (BLCKSZ / 1024)},
   {"MB", GUC_UNIT_BLOCKS, 1024 / (BLCKSZ / 1024)},
   {"kB", GUC_UNIT_BLOCKS, -(BLCKSZ / 1024)},
+ {"B", GUC_UNIT_BLOCKS, -(BLCKSZ / (1024 * 1024))},


Isn't this 0 in the common case of 8k pages?


   {"TB", GUC_UNIT_XBLOCKS, (1024 * 1024 * 1024) / (XLOG_BLCKSZ /

1024)},

   {"GB", GUC_UNIT_XBLOCKS, (1024 * 1024) / (XLOG_BLCKSZ / 1024)},
   {"MB", GUC_UNIT_XBLOCKS, 1024 / (XLOG_BLCKSZ / 1024)},
   {"kB", GUC_UNIT_XBLOCKS, -(XLOG_BLCKSZ / 1024)},
+ {"B", GUC_UNIT_XBLOCKS, -(XLOG_BLCKSZ / (1024 * 1024))},


Same?


As I understand, in these cases multiplier should be just -BLCKSZ and
-XLOG_BLCKSZ correspondingly.


Yep, quite right. Fixed and committed, thanks!

- Heikki

Re: SCRAM with channel binding downgrade attack

[Magnus Hagander]

On Wed, May 23, 2018 at 8:46 AM, Heikki Linnakangas  wrote:

> On 22/05/18 11:22, Michael Paquier wrote:
>
>> On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:
>>
>>> The previous patch has actually problems with servers using "trust",
>>> "password" and any protocol which can send directly AUTH_REQ_OK as those
>>> could still enforce a downgrade to not use channel binding, so we
>>> actually need to make sure that AUTH_REQ_SASL_FIN has been received when
>>> channel binding is required when looking at a AUTH_REQ_OK message.
>>>
>>
>> Okay, so I have digested the previous comments with the attached.
>> scram_channel_binding is modified as follows (from commit message):
>> - "prefer", is the default and behaves so as channel binding is used if
>> available.  If the cluster does not support it then it is not used. This
>> does not protect from downgrade attacks.
>> - "disable", which is the equivalent of the empty value previously,
>> disables channel binding.
>> - "tls-unique" and "tls-server-end-point" make channel binding a
>> requirement and use the channel binding of the same name for
>> connection.  Note that in this case, if the connection is attempted
>> without SSL or if server does not support channel binding then libpq
>> refuses the connection as well.
>>
>
> "tls-unique" and "tls-server-end-point" are overly technical to users.
> They don't care which one is used, there's no difference in security.
> Furthermore, if we add another channel binding type in the future, perhaps
> because someone finds a vulnerability in those types and a new one is added
> to address it, users would surely like to be automatically switched over to
> the new binding type. If they have "tls-unique" hardcoded in connection
> strings, that won't happen.
>
> So I think the options should be "prefer", "disable", and "require".
> That's also symmetrical with sslmode, which is nice.
>

As a general point, I still don't like being symmetrical with
sslmode=prefer, because that's a silly setting for sslmode :) It basically
says "I don't care about the security guarantees, I just want the overhead
please". Now, granted, the over head for SCRAM channel-binding is certainly
a lot less than it is for TLS. But if we just want to go with symmetry, it
would perhaps make more sense to implement the "allow" mode which makes
more sense on the TLS side as well.


We could provide "tls-unique" and "tls-server-end-point" in addition to
> those, but I'd consider those to be developer only settings, useful only
> for testing the protocol.
>
> It would be nice to have a libpq setting like
> "authenticate_server=require", which would mean "I want man-in-the-middle
> protection".


That seems like a bad name for such a thing. Shouldn't it be
"authenticate_server=no_man_in_the_middle" (not those words but you get the
idea). Specifically, protecting against man in the middle attack does not
equal authenticating server -- you can still be connected to the wrong
server just with no second attacker between you and the first attacker.



> With that, a connection would be allowed, if either the server's SSL
> certificate is verified as with "sslmode=verify-full", *or* SCRAM
> authentication with channel binding was used. Or perhaps cram it into
> sslmode, "sslmode=verify-full-or-scram-channel-binding", just with a
> nicer name. (We can do that after v11 though, I think.)


sslmode=verify-full is very different from SCRAM with channel binding,
isn't it? As in, SCRAM with channel binding at no point proves which server
you're talking to -- only that you are talking to the SSL endpoint? It
could be a rogue SSL endpoint unless you do certificate validation.

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

Re: Possible bug in logical replication.

[Masahiko Sawada]

On Fri, May 18, 2018 at 2:37 PM, Kyotaro HORIGUCHI
 wrote:
> At Thu, 17 May 2018 13:54:07 +0300, Arseny Sher  wrote 
> in <87in7md034.fsf@ars-thinkpad>
>>
>> Konstantin Knizhnik  writes:
>>
>> > I think that using restart_lsn instead of confirmed_flush is not right 
>> > approach.
>> > If restart_lsn is not available and confirmed_flush is pointing to page
>> > boundary, then in any case we should somehow handle this case and adjust
>> > startlsn to point on the valid record position (by jjust adding page header
>> > size?).
>>
>> Well, restart_lsn is always available on live slot: it is initially set
>> in ReplicationSlotReserveWal during slot creation.
>
> restart_lsn stays at the beginning of a transaction until the
> transaction ends so just using restart_lsn allows repeated
> decoding of a transaction, in short, rewinding occurs. The
> function works only for inactive slot so the current code works
> fine on this point. Addition to that restart_lsn also can be on a
> page bounary.
>
>
> We can see the problem easily.
>
> 1. Just create a logical replication slot with setting current LSN.
>
>   select pg_create_logical_replication_slot('s1', 'pgoutput');
>
> 2. Advance LSN by two or three pages by doing anyting.
>
> 3. Advance the slot to a page bounadry.
>
>   e.g. select pg_replication_slot_advance('s1', '0/9624000');
>
> 4. advance the slot further, then crash.
>
> So directly set ctx->reader->EndRecPtr by startlsn fixes the
> problem, but I found another problem here.

I confirmed that the attached patch fixes this problem as well as the
same problem reported on another thread.

I'm not sure it's a good approach to change the state of xlogreader
directly in the replication slot codes because it also means that we
have to care about this code as well when xlogreader code is changed.
Another possible way might be to make XLogFindNextRecord valid in
backend code and move startlsn to the first valid record with an lsn
>= startlsn by using that function. Please find attached patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


find_next_valid_record.patch
Description: Binary data

Re: [PATCH] Clear up perlcritic 'missing return' warning

[Mike Blackwell]

On Tue, May 22, 2018 at 3:32 AM, Michael Paquier 
wrote:

>
>  And this
> maps with any C code.
>

The important differences here are:
  *) Declaring a C function as void prevents returning a value.  The intent
not to return a value is clear to any caller and is enforced by the
compiler.  There is no equivalent protection in Perl.
  *) Falling off the end of a C function that returns a type other than
void has undefined results.  Perl will always return the value of the last
statement executed.

Because Perl does allow returning a value without explicitly using return,
it's easy to write code that breaks if an unwary person adds a line to the
end of the subroutine.  There's a common constructor incantation that has
this problem.  It's a real gotcha for C programmers just starting to poke
around in Perl code.

This difference also allows users of .pm modules to abuse the API of a
method intended to be "void", if the value returned falling off the end
happens to seem useful, leading to breakage if the method's code changes in
the future.


> This is most likely going to be forgotten.
>

That's what perlcritic is for. :)

Mike

Re: SCRAM with channel binding downgrade attack

[Heikki Linnakangas]

On 22/05/18 11:22, Michael Paquier wrote:

On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:

The previous patch has actually problems with servers using "trust",
"password" and any protocol which can send directly AUTH_REQ_OK as those
could still enforce a downgrade to not use channel binding, so we
actually need to make sure that AUTH_REQ_SASL_FIN has been received when
channel binding is required when looking at a AUTH_REQ_OK message.


Okay, so I have digested the previous comments with the attached.
scram_channel_binding is modified as follows (from commit message):
- "prefer", is the default and behaves so as channel binding is used if
available.  If the cluster does not support it then it is not used. This
does not protect from downgrade attacks.
- "disable", which is the equivalent of the empty value previously,
disables channel binding.
- "tls-unique" and "tls-server-end-point" make channel binding a
requirement and use the channel binding of the same name for
connection.  Note that in this case, if the connection is attempted
without SSL or if server does not support channel binding then libpq
refuses the connection as well.


"tls-unique" and "tls-server-end-point" are overly technical to users. 
They don't care which one is used, there's no difference in security. 
Furthermore, if we add another channel binding type in the future, 
perhaps because someone finds a vulnerability in those types and a new 
one is added to address it, users would surely like to be automatically 
switched over to the new binding type. If they have "tls-unique" 
hardcoded in connection strings, that won't happen.


So I think the options should be "prefer", "disable", and "require". 
That's also symmetrical with sslmode, which is nice.


We could provide "tls-unique" and "tls-server-end-point" in addition to 
those, but I'd consider those to be developer only settings, useful only 
for testing the protocol.


It would be nice to have a libpq setting like 
"authenticate_server=require", which would mean "I want 
man-in-the-middle protection". With that, a connection would be allowed, 
if either the server's SSL certificate is verified as with 
"sslmode=verify-full", *or* SCRAM authentication with channel binding 
was used. Or perhaps cram it into sslmode, 
"sslmode=verify-full-or-scram-channel-binding", just with a nicer name. 
(We can do that after v11 though, I think.)



In order to make sure that a client is not tricked by a "trust"
connection downgrade which sends back directly AUTH_REQ_OK, one way I
have thought about is to check if the client has achieved with a server
a full SASL exchange when receiving this message type, which is
something that we know about as the exchange state is saved in
PGconn->sasl_state.


It'd be nice if the client could tell the server that it requires 
authentication, so that the server would go through the SCRAM 
authentication even with "trust". With channel binding, SCRAM not only 
authenticates the client to the server, but also the server to the 
client. But that's not urgent, I think we can live without it for v11.


- Heikki

Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data

[Carter Thaxton]

Ah yes, thanks.  I did in fact have colors enabled.
I've attached a new patch generated by `git format-patch`.  Hopefully
that's correct.


On Mon, May 21, 2018 at 4:00 PM, Thomas Munro  wrote:

> On Tue, May 22, 2018 at 4:05 AM, Stephen Frost  wrote:
> > * Carter Thaxton (carter.thax...@gmail.com) wrote:
> >>   pg_dump --include-table-data-where=largetable:"created_at >=
> >> '2018-05-01'" database_name
> >
> > I've wanted something similar to this in the past as well, and, as
> > you've seen, we have some support for this kind of thing in pg_dump
> > already and what you're doing is exposing that.
>
> +1
>
> >> I've prepared a proposed patch for this, which is attached.  The code
> >> changes are rather straightforward.  I did have to add the ability to
> carry
> >> around an extra pointer-sized object to the simple_list implementation,
> in
> >> order to allow the filter clause to be associated to the matching oids
> of
> >> the table pattern.  It seemed the best way to augment the existing
> >> simple_list implementation, but change as little as possible elsewhere
> in
> >> the codebase.  (Note that SimpleOidList is actually only used by
> pg_dump).
> >>
> >> Feel free to review and propose any amendments.
> >
> > I've only taken a quick look but I don't see any regression tests, for
> > starters, and it's not clear if this can be passed multiple times for
> > one pg_dump run (I'd certainly hope that it could be...).
> >
> > Also, if you haven't already, this should be registered on the
> > commitfest app, so we don't lose track of it.
>
> Thanks for doing that.  Unfortunately the patch seems to be corrupted
> in some way, maybe ANSI control characters or something... perhaps you
> set colour.ui = always in your git config, instead of auto?  You might
> also consider using git format-patch so you can include a brief commit
> message that explains the feature.
>
> --
> Thomas Munro
> http://www.enterprisedb.com
>


pgdump-include-table-data-where-v2.patch
Description: Binary data

Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data

[Carter Thaxton]

> How would you handle foreign keys? It seems easier to produce a dump
> that won't restore.
>

This proposal will not attempt to be smart about foreign keys or anything
like that.  I don't believe that would even be expected.

> We have the existing options:
> >   --include-table=table(and its -t synonym)
> >   --exclude-table=table
> >   --exclude-table-data=table
> >
> > I propose a new option:
> >   --include-table-data-where=table:filter_clause
> >
> I remembered an old thread [1]. At that time pg_dump was not so
> decoupled from the backend. We are far from being decoupled in a way
> that someone can write his own pg_dump using only calls from a
> library. I'm not sure pg_dump is the right place to add another ETL
> parameter. We already have too much parameters that could break a
> restore (flexibility is always welcome but too much is not so good).
>

In general, I agree with your sentiment that we don't want too much
flexibility in this tool.  However, this just seems like a very obvious
missing feature to me.  I was frankly surprised that pg_dump didn't already
have it.

I've designed this feature so that it behaves like a more flexible version
between --exclude-table-data and --include-table.  Instead of dumping the
schema and zero rows, or the schema and all of the rows, it dumps the
schema and some specific rows.

Providing "--include-table-data-where=table:false" behaves exactly like
--exclude-table-data, and "--include-table-data-where=table:true" behaves
exactly like --include-table.
It does no more or less to prevent a restore.  Given that
--exclude-table-data already exists, this seems to introduce no new issues
with restore.



> >   pg_dump --include-table-data-where=largetable:"created_at >=
> '2018-05-01'"
> > database_name
> >
> How would you check that that expression is correct?


The patch as already provided produces an error message and appropriate
exit code during the dump process, presenting the invalid SQL that is
produced as part of the WHERE clause.
I could see some value in refactoring it to provide error messages earlier
in the process, but it's actually not bad as is.


Every parameter could quote its value. It means that your parameter have to
> escape the
> quote in '2018-05-01'.


I don't understand.  The double quotes in my example are bash shell
quotes.  There is no special quote parsing in this patch.  The single
quotes are part of the WHERE clause.
Note that pg_dump already uses getopt_long, so it's not required to use the
= symbol to separate option from its associated value.  So, it would also
be fine to call as follows:

  pg_dump --include-table-data-where "largetable:created_at >=
'2018-05-01'" database_name


Another problem is that your spec does not show
> us how you would handle tables like Foo.Bar or "foo:bar" (colon have
> to be escaped)?
>

Using a dot to separate the schema works just fine.  My proposal uses the
same mechanism as --include-table, --exclude-table, and
--exclude-table-data.  In fact, it even supports wildcards in those
patterns.

Your point about a colon in the table name is interesting.  In all my years
of working with PostgreSQL and other databases, I've never encountered a
table name that contained a colon.  Perhaps an escape character, like \:
could work.  Is there another separator character you would suggest, which
is illegal in table names, but also intuitive as a separator?  Maybe a
comma?



> > The filter_clause is used as the contents of a WHERE clause when querying
> > the data to generate the COPY statement produced by pg_dump.
> >
> You are forgetting about --inserts parameter. Could I use
> --include-table-data-where and --inserts?


Yes, the --inserts parameter works just fine.  Perhaps I should have said
"the COPY statement or INSERT statements".

-D option of pg_resetwal is only works with absolute path

[tushar]

Hi,

In the  latest PG v11,  found that  -D option of pg_resetwal is only 
works with absolute path .. not with relative path


e.g

Not Working -

[centos@localhost bin]$ ./pg_resetwal -D data
pg_resetwal: could not read permissions of directory "data": No such 
file or directory


Working -

[centos@localhost bin]$ ./pg_resetwal -D 
/home/centos/pg11_22may/postgresql/edbpsql/bin/data

Write-ahead log reset

Is this something expected  in v11 ?

--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company