Re: behave of --create-slot option

[Pavel Stehule]

2018-05-29 6:11 GMT+02:00 Craig Ringer :

> On 29 May 2018 at 11:51, Pavel Stehule  wrote:
>
>
>>
>> I understand so slot should be unique. But same result (unique rep slot)
>> can be done, if it does nothing when slot exists already. This behave is
>> not idempotent.
>>
>> Maybe I am search problem, where it is not. Just, when I see some "create
>> object" option, I expect any way, how I can enforce "--if-exists", because
>> it was necessary in major cases.
>>
>>
> If the slot already exists, don't you expect it to be in use for an
> existing replica?
>

the slot name is unique, so I don't expect it - when I use some name from
script


> I guess what you seem to want is to be able to delete the replica then
> re-clone it and use the same slot?
>

maybe. Now, it looks "asymmetric"


> Generally I'd expect you to delete the slot when you remove the replica.
> Really what this says to me is that we should have a way to delete the
> upstream slot when we promote or decommission a physical replica.
>

When I think about this option and designed behave, I am inclined to think
so it can be hard to use, and  better create slot outside, and outside drop
slot too.

I hope so I understand to motivation for this option - but I see issues:

1. pg_basebackup can fail from some other reasons, but there is not special
status for this issue.
2. when I use this option in any script, then I should to remove possibly
exists slot before, to minimize risk of fail of this command.

I understand, current behave can be wanted like protection against unwanted
running some deployment script. But can be problematic too, when
pg_basebackup or or some other fails from unexpected reasons.

Regards

Pavel


>
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

Re: behave of --create-slot option

[Craig Ringer]

On 29 May 2018 at 11:51, Pavel Stehule  wrote:


>
> I understand so slot should be unique. But same result (unique rep slot)
> can be done, if it does nothing when slot exists already. This behave is
> not idempotent.
>
> Maybe I am search problem, where it is not. Just, when I see some "create
> object" option, I expect any way, how I can enforce "--if-exists", because
> it was necessary in major cases.
>
>
If the slot already exists, don't you expect it to be in use for an
existing replica?

I guess what you seem to want is to be able to delete the replica then
re-clone it and use the same slot?

Generally I'd expect you to delete the slot when you remove the replica.
Really what this says to me is that we should have a way to delete the
upstream slot when we promote or decommission a physical replica.



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

Re: behave of --create-slot option

[Pavel Stehule]

2018-05-29 3:28 GMT+02:00 Craig Ringer :

> On 29 May 2018 at 03:41, Pavel Stehule  wrote:
>
>> Hi
>>
>> I am writing a article about PostgreSQL 11 features. Now I am looking on
>> new option --create-slot option of pg_basebackup command.
>>
>> I don't understand to use case for this option, because It fails when
>> requested slot already exists. I cannot to imagine use case for this. If I
>> write some scripts, then I prefer the behave like "create if doesn't exist
>> or do nothing".
>>
>> Any repeated running of script with this option should to fail. Is it
>> required? Why?
>>
>>
> The slot is intended to then be used by a replica that was created by
> pg_basebackup. I think it writes the slot name into recovery.conf; if it
> doesn't, it should.
>
> So you use a unique slot name for each replica.
>

I understand so slot should be unique. But same result (unique rep slot)
can be done, if it does nothing when slot exists already. This behave is
not idempotent.

Maybe I am search problem, where it is not. Just, when I see some "create
object" option, I expect any way, how I can enforce "--if-exists", because
it was necessary in major cases.

Regards

Pavel


> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

Re: Is a modern build system acceptable for older platforms

[Yuriy Zhuravlev]

First, I apologize if my words hurt someone. I didn't want this.
Second, I totally agree with Andrew.

> He's also right that the build system is among the
> least of our problems in making newcomers feel comfortable.
>

This what I wanted to say. Not big technical difference between build
systems for results. You can build executable and libraries for each needed
platforms use any of it.
The main difference in comfort for that and degree of comfort depending on
your development style, experience, and platform.
If you use Windows, or IDEs like XCode, or you don't know bash, m4, sed,
grep, perl, Makefile... working with Postgres as a developer will be
uncomfortable.
CMake can bring a similar experience of using for each platform. That's it.
(and maybe cost to support your build system)

PS I know all this technology and use usually Linux but CMake or Meson
still looks more comfortable for me.

Re: behave of --create-slot option

[Craig Ringer]

On 29 May 2018 at 03:41, Pavel Stehule  wrote:

> Hi
>
> I am writing a article about PostgreSQL 11 features. Now I am looking on
> new option --create-slot option of pg_basebackup command.
>
> I don't understand to use case for this option, because It fails when
> requested slot already exists. I cannot to imagine use case for this. If I
> write some scripts, then I prefer the behave like "create if doesn't exist
> or do nothing".
>
> Any repeated running of script with this option should to fail. Is it
> required? Why?
>
>
The slot is intended to then be used by a replica that was created by
pg_basebackup. I think it writes the slot name into recovery.conf; if it
doesn't, it should.

So you use a unique slot name for each replica.

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

Re: behave of --create-slot option

[Euler Taveira]

2018-05-28 16:41 GMT-03:00 Pavel Stehule :
> I am writing a article about PostgreSQL 11 features. Now I am looking on new
> option --create-slot option of pg_basebackup command.
>
> I don't understand to use case for this option, because It fails when
> requested slot already exists. I cannot to imagine use case for this. If I
> write some scripts, then I prefer the behave like "create if doesn't exist
> or do nothing".
>
In prior versions, you should create a slot (via SQL function or
streaming replication protocol) *before* run pg_basebackup. In 11, use
--create-slot option and you don't need an extra step (of course, you
should drop that slot after the end of backup -- if that is not a new
standby. It also does not make sense to use _persistent_ replication
slots for backup because you are probably archiving WAL). IMHO the use
case is new standbys that will use replication slots.

> Any repeated running of script with this option should to fail. Is it
> required? Why?
>
As I said, you should have an extra step to drop that slot (even
before 11). There is no "create replication slots if not exists". If
you are repeatedly running a script, why don't you let pg_basebackup
use temporary replication slots?


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

adding tab completions

[Justin Pryzby]

Find attached tab completion for the following:

"... Also, recursively perform VACUUM and ANALYZE on partitions when the
command is applied to a partitioned table."
3c3bb99330aa9b4c2f6258bfa0265d806bf365c3

Add parenthesized options syntax for ANALYZE.
854dd8cff523bc17972d34772b0e39ad3d6d46a4

Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies.
ede62e56fbe809baa1a7bc3873d82f12ffe7540b

Allow multiple tables to be specified in one VACUUM or ANALYZE command.
11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7bb47ea..2d7b264 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -717,6 +717,26 @@ static const SchemaQuery Query_for_list_of_tmf = {
NULL
 };
 
+static const SchemaQuery Query_for_list_of_tpmf = {
+   /* min_server_version */
+   0,
+   /* catname */
+   "pg_catalog.pg_class c",
+   /* selcondition */
+   "c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
+   CppAsString2(RELKIND_MATVIEW) ", "
+   CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
+   CppAsString2(RELKIND_FOREIGN_TABLE) ")",
+   /* viscondition */
+   "pg_catalog.pg_table_is_visible(c.oid)",
+   /* namespace */
+   "c.relnamespace",
+   /* result */
+   "pg_catalog.quote_ident(c.relname)",
+   /* qualresult */
+   NULL
+};
+
 static const SchemaQuery Query_for_list_of_tpm = {
/* min_server_version */
0,
@@ -3563,33 +3583,41 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_CONST("OPTIONS");
 
 /*
- * VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
- * VACUUM [ FULL | FREEZE ] [ VERBOSE ] ANALYZE [ table [ (column [, ...] ) ] ]
+ * VACUUM [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
+ * VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns [, 
...] ]
  */
else if (Matches1("VACUUM"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
   " UNION 
SELECT 'FULL'"
   " UNION 
SELECT 'FREEZE'"
   " UNION 
SELECT 'ANALYZE'"
+  " UNION 
SELECT '('"
   " UNION 
SELECT 'VERBOSE'");
-   else if (Matches2("VACUUM", "FULL|FREEZE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   else if (Matches2("VACUUM", "FULL"))
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
+  " UNION 
SELECT 'FREEZE'"
   " UNION 
SELECT 'ANALYZE'"
   " UNION 
SELECT 'VERBOSE'");
-   else if (Matches3("VACUUM", "FULL|FREEZE", "ANALYZE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   else if (Matches3("VACUUM", "FULL|FREEZE|FULL FREEZE", "ANALYZE"))
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
   " UNION 
SELECT 'VERBOSE'");
-   else if (Matches3("VACUUM", "FULL|FREEZE", "VERBOSE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   else if (Matches3("VACUUM", "FULL|FREEZE|FULL FREEZE", "VERBOSE"))
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
   " UNION 
SELECT 'ANALYZE'");
else if (Matches2("VACUUM", "VERBOSE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
   " UNION 
SELECT 'ANALYZE'");
else if (Matches2("VACUUM", "ANALYZE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
   " UNION 
SELECT 'VERBOSE'");
+   else if (HeadMatches2("VACUUM", "(") && !ends_with(prev_wd, ',') && 
!ends_with(prev_wd, '('))
+   COMPLETE_WITH_LIST2(",", ")");
+   else if (HeadMatches2("VACUUM", "(") && !ends_with(prev_wd, ')'))
+   COMPLETE_WITH_LIST5("FULL", "FREEZE", "ANALYZE", 
"VERBOSE", "DISABLE_PAGE_SKIPPING" );
+   else if (HeadMatches1("VACUUM") && !ends_with(prev_wd, ',') && 
!ends_with(prev_wd, ')'))
+   COMPLETE_WITH_CONST(",");
else if (HeadMatches1("VACUUM"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf, 

Re: Is a modern build system acceptable for older platforms

[Andrew Dunstan]

On Mon, May 28, 2018 at 4:23 PM, Bruce Momjian  wrote:
> On Mon, May 28, 2018 at 11:16:08AM +0200, Magnus Hagander wrote:
>> I would argue the exact opposite - mail is a lot more flexible than using
>> github issues and that's one of the most important reasons I prefer it.
>>
>> (and there are of course many ways to tag and categorize your email, many 
>> more
>> so than with issues. Specifically bookmarks will of course depend on your 
>> mail
>> program)
>>
>> There are definitely advantages with issues for tracking, such as getting a
>> more structured central repository of them (but that requires very strict 
>> rules
>> for how to apply them and huge amounts of maintenance effort), but as a
>> communications tool I'd say email is vastly superior, particularly thanks to
>> the flexibility.
>
> It might be that occasional uses would find github easier, and more
> invested users would find email easier.
>


How do people get to be invested developers, though? Everybody starts
as a more occasional user. I started out with one smallish patch for
7.4 and never intended at that stage to do much more. (So much for
prescience.)

The older I get the more I am prepared to admit that my preferred way
to do things might not suit those younger than me. Craig is right, our
processes do not make newcomers, especially younger newcomers, feel
very comfortable. He's also right that the build system is among the
least of our problems in making newcomers feel comfortable.

cheers

andrew

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

Re: perl checking

[Andrew Dunstan]

On Sun, May 27, 2018 at 11:42 AM, Andrew Dunstan
 wrote:
>
>
> On 05/18/2018 02:02 PM, Andrew Dunstan wrote:
>>
>>
>> These two small patches allow us to run "perl -cw" cleanly on all our perl
>> code.
>>
>>
>> One patch silences a warning from convutils.pl about the unportability of
>> the literal 0x1. We've run for many years without this giving us a
>> problem, so I think we can turn the warning off pretty safely.
>>
>>
>> The other patch provides a dummy library that emulates just enough of the
>> Win32 perl infrastructure to allow us to run these checks. That means that
>> Unix-based developers who might want to make changes in the msvc code can
>> actually run a check against their code without having to put it on a
>> Windows machine. The invocation goes like this (to check Mkvcbuild.pl for
>> example):
>>
>>
>>PERL5LIB=src/tools/msvc/dummylib perl -cw src/tools/Mkvcbuild.pm
>>
>>
>> This also allows us to check src/tools/win32tzlist.pl.
>>
>>
>> In due course I'll submit a script to automate this syntax checking.
>>
>>
>>
>
>
> Here is the latest version of the second patch, this time with warnings
> about redefinition of some subroutines suppressed. These mostly occur
> because in a few cases we have multiple packages in a single file.
>
> This allows the following command to pass cleanly:
>
>{ find . -type f -a -name '*.p[lm]' -print; find . -type f -perm
>-100 -exec file {} \; -print | egrep -i ':.*perl[0-9]*\>' |cut -d:
>-f1 ; } | sort -u |
>
> PERL5LIB=src/test/perl:src/test/ssl:src/bin/pg_rewind:src/backend/catalog:src/backend/utils/mb/Unicode:src/tools/msvc/dummylib:src/tools/msvc
>xargs -L 1 perl -cw
>
>


Attached version actually does what's advertised above.


cheers

andrew

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


perl-warnings-fix-msvc-3.patch
Description: Binary data

Re: found xmin from before relfrozenxid on pg_catalog.pg_authid

[Andres Freund]

Hi,

On 2018-05-27 13:00:06 -0700, Andres Freund wrote:
> I've a patch that seems to work, that mostly needs some comment
> polishing.

Attached is what I currently have. Still needs some more work, but I
think it's more than good enough to review the approach.  Basically the
approach consists out of two changes:

1) Send init file removals for shared nailed relations as well.

   This fixes that the shared init file contains arbitrarily outdated
   information for relfrozenxid etc. Leading to e.g. the pg_authid
   errors we've seen in some recent threads.  Only applies to
   new connections.

2) Reread RelationData->rd_rel for nailed relations when invalidated.

   This ensures that already built relcache entries for nailed relations
   are updated. Currently they never are.  This currently doesn't cause
   *that* frequently an issue for !shared entries, because for those the
   init file gets zapped regularly, and autovacuum workers usually don't
   live that long.  But it's still a significant correctness issue for
   both shared an non shared relations.

FWIW, I wonder if this isn't critical enough to make us consider having
a point release earlier..

Greetings,

Andres Freund
>From c7121eec1e76b6f9b2cfa5d7d7f5f4b7a4f8be96 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 28 May 2018 12:38:22 -0700
Subject: [PATCH] WIP: Ensure relcache entries for nailed relations are
 accurate.

Author: Andres Freund
Reviewed-By:
Discussion:
https://postgr.es/m/20180525203736.crkbg36muzxrj...@alap3.anarazel.de
https://postgr.es/m/CAMa1XUhKSJd98JW4o9StWPrfS=11bpgg+_gdmxe25tvuy4s...@mail.gmail.com
https://postgr.es/m/cakmfjucqbuodrfxpdx39wha3vjyxwerg_zdvxzncr6+5wog...@mail.gmail.com
https://postgr.es/m/cagewt-ujgpmlq09gxcufmzazsgjc98vxhefbf-tppb0fb13...@mail.gmail.com
Backpatch:
---
 src/backend/utils/cache/inval.c|  31 +++--
 src/backend/utils/cache/relcache.c | 198 -
 src/include/storage/standbydefs.h  |   2 +-
 3 files changed, 156 insertions(+), 75 deletions(-)

diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index 955f5c7fdc5..0d6100fb082 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -521,12 +521,11 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
 	(void) GetCurrentCommandId(true);
 
 	/*
-	 * If the relation being invalidated is one of those cached in the local
-	 * relcache init file, mark that we need to zap that file at commit. Same
-	 * is true when we are invalidating whole relcache.
+	 * If the relation being invalidated is one of those cached in a relcache
+	 * init file, mark that we need to zap that file at commit. Same is true
+	 * when we are invalidating whole relcache.
 	 */
-	if (OidIsValid(dbId) &&
-		(RelationIdIsInInitFile(relId) || relId == InvalidOid))
+	if (relId == InvalidOid || RelationIdIsInInitFile(relId))
 		transInvalInfo->RelcacheInitFileInval = true;
 }
 
@@ -881,18 +880,26 @@ ProcessCommittedInvalidationMessages(SharedInvalidationMessage *msgs,
 
 	if (RelcacheInitFileInval)
 	{
+		elog(trace_recovery(DEBUG4), "removing relcache init files for database %u",
+			 dbid);
+
 		/*
-		 * RelationCacheInitFilePreInvalidate requires DatabasePath to be set,
-		 * but we should not use SetDatabasePath during recovery, since it is
+		 * RelationCacheInitFilePreInvalidate, when the invalidation message
+		 * is for a specific database, requires DatabasePath to be set, but we
+		 * should not use SetDatabasePath during recovery, since it is
 		 * intended to be used only once by normal backends.  Hence, a quick
 		 * hack: set DatabasePath directly then unset after use.
 		 */
-		DatabasePath = GetDatabasePath(dbid, tsid);
-		elog(trace_recovery(DEBUG4), "removing relcache init file in \"%s\"",
-			 DatabasePath);
+		if (OidIsValid(dbid))
+			DatabasePath = GetDatabasePath(dbid, tsid);
+
 		RelationCacheInitFilePreInvalidate();
-		pfree(DatabasePath);
-		DatabasePath = NULL;
+
+		if (OidIsValid(dbid))
+		{
+			pfree(DatabasePath);
+			DatabasePath = NULL;
+		}
 	}
 
 	SendSharedInvalidMessages(msgs, nmsgs);
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 3dfb1b8fbe2..aa4427724d5 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -250,6 +250,7 @@ static void RelationDestroyRelation(Relation relation, bool remember_tupdesc);
 static void RelationClearRelation(Relation relation, bool rebuild);
 
 static void RelationReloadIndexInfo(Relation relation);
+static void RelationReloadNailed(Relation relation);
 static void RelationFlushRelation(Relation relation);
 static void RememberToFreeTupleDescAtEOX(TupleDesc td);
 static void AtEOXact_cleanup(Relation relation, bool isCommit);
@@ -286,7 +287,7 @@ static void IndexSupportInitialize(oidvector *indclass,
 static OpClassCacheEnt *LookupOpclassInfo(Oid operatorClassOid,
   StrategyNumber numSupport);
 static void 

behave of --create-slot option

[Pavel Stehule]

Hi

I am writing a article about PostgreSQL 11 features. Now I am looking on
new option --create-slot option of pg_basebackup command.

I don't understand to use case for this option, because It fails when
requested slot already exists. I cannot to imagine use case for this. If I
write some scripts, then I prefer the behave like "create if doesn't exist
or do nothing".

Any repeated running of script with this option should to fail. Is it
required? Why?

Regards

Pavel

Re: Is a modern build system acceptable for older platforms

[Bruce Momjian]

On Mon, May 28, 2018 at 11:16:08AM +0200, Magnus Hagander wrote:
> I would argue the exact opposite - mail is a lot more flexible than using
> github issues and that's one of the most important reasons I prefer it. 
> 
> (and there are of course many ways to tag and categorize your email, many more
> so than with issues. Specifically bookmarks will of course depend on your mail
> program) 
> 
> There are definitely advantages with issues for tracking, such as getting a
> more structured central repository of them (but that requires very strict 
> rules
> for how to apply them and huge amounts of maintenance effort), but as a
> communications tool I'd say email is vastly superior, particularly thanks to
> the flexibility. 

It might be that occasional uses would find github easier, and more
invested users would find email easier.

-- 
  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: PGXS fails to byte-compile pgsql-ogr-fdw

[Andres Freund]

Hi,

On 2018-05-28 09:44:32 +0200, Christoph Berg wrote:
> pgsql-ogr-fdw fails to build against PG 11beta1 with JIT enabled. I
> just reported this as https://github.com/pramsey/pgsql-ogr-fdw/issues/153,
> but I think the problem might actually be in the PGXS Makefile - it
> assumes that all objects have a .c file to build the .bc from.

Not really see an alternative to that? What are you proposing?


> Another bug might be that PGXS doesn't try to inherit any preprocessor
> flags from the C compilation - the "-D USE_PG_MEM" part will be lost.

How could it? That seems to be a problem in the specific makefile, it
should adapt the proper variables rather than passing -D itself.

Greetings,

Andres Freund

Re: PGXS fails to byte-compile pgsql-ogr-fdw

[Laurenz Albe]

Christoph Berg wrote:
> pgsql-ogr-fdw fails to build against PG 11beta1 with JIT enabled. I
> just reported this as https://github.com/pramsey/pgsql-ogr-fdw/issues/153,
> but I think the problem might actually be in the PGXS Makefile - it
> assumes that all objects have a .c file to build the .bc from.

I have been bitten by that when building PostGIS.

A simple patch could fix it, but I agree that it would be better
to not build them.

Yours,
Laurenz Albe

Re: Problem while updating a foreign table pointing to a partitioned table on foreign server

[Alvaro Herrera]

Hello

I don't think this thread has reached a consensus on a design for a fix,
has it?  Does anybody have a clear idea on a path forward?  Is anybody
working on a patch?

Thanks

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

Re: found xmin from before relfrozenxid on pg_catalog.pg_authid

[Nishant, Fnu]

Hi,
We were working on this issue and thinking if we could actually make 
pg_class(rd_rel) part of recache entry upgradable.
To achieve this we can allocate Form_pg_class structures (for shared relations… 
a small number) on shared memory.
We do not need global pg_internal_init file as new backend during boot up will 
be set to point at already stored Form_pg_class structure.

Thanks,
Nishant

On 5/27/18, 1:01 PM, "Andres Freund"  wrote:

Hi,

On 2018-05-27 13:22:21 -0400, Tom Lane wrote:
> But I don't think there's any need for special magic here: we just
> have to accept the fact that there's a need to flush that cache
> sometimes.  In normal use it shouldn't happen often enough to be a
> performance problem.

Yea, it's not that problematic. We already remove the local init
file. I started out trying to write a version of invalidation that also
WAL logs shared inval, but that turns out to be hard to do without
breaking compatibilty.  So I think we should just always unlink the
shared one - that seems to work well.


> FWIW, I'm not on board with "memcpy the whole row".  I think the right
> thing is more like what we do in RelationReloadIndexInfo, ie copy over
> the specific fields that we expect to be mutable.

But that's what RelationReloadIndexInfo() etc do?
relp = (Form_pg_class) GETSTRUCT(pg_class_tuple);
memcpy(relation->rd_rel, relp, CLASS_TUPLE_SIZE);
I don't think we need to modify anything outside of rd_rel at this
point?

I've a patch that seems to work, that mostly needs some comment
polishing.

Greetings,

Andres Freund

Re: [JDBC] [HACKERS] PGSERVICEFILE as a connection string parameter

[Vladimir Sitnikov]

David>I'm guessing that the Java port wouldn't be too
complicated.  It's already well defined.

Is encoding defined somewhere for the "service file"?
I don't like the idea of using "a default" very much.

Vladimir

Re: Undo logs

[Simon Riggs]

On 24 May 2018 at 23:22, Thomas Munro  wrote:

> As announced elsewhere[1][2][3], at EnterpriseDB we are working on a
> proposal to add in-place updates with undo logs to PostgreSQL.  The
> goal is to improve performance and resource usage by recycling space
> better.

Cool

> The lowest level piece of this work is a physical undo log manager,

> 1.  Efficient appending of new undo data from many concurrent
> backends.  Like logs.
> 2.  Efficient discarding of old undo data that isn't needed anymore.
> Like queues.
> 3.  Efficient buffered random reading of undo data.  Like relations.

Like an SLRU?

> [4] 
> https://github.com/EnterpriseDB/zheap/tree/undo-log-storage/src/backend/access/undo
> [5] 
> https://github.com/EnterpriseDB/zheap/tree/undo-log-storage/src/backend/storage/smgr

I think there are quite a few design decisions there that need to be
discussed, so lets crack on and discuss them please.

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

Re: Is a modern build system acceptable for older platforms

[Yuriy Zhuravlev]

>
> It is correct that Gmail is incapable of this in the web browser. Many
> other email systems can though, and Gmail still speaks imap so you can use
> those if you prefer.
>

Mail programs outside web browser not popular anymore and this standalone
programs became very slow to grow (for example Thunderbird).
I am really don't want to install anything if GMail covers 99% of my usage.
We back to personal habits again...
Also, github have API and you can make your own App with a
custom interface.

(and there are of course many ways to tag and categorize your email, many
> more so than with issues. Specifically bookmarks will of course depend on
> your mail program)
>
but you should do it by yourself, it`s too many works. On github moderator
can put tags from the pre-prepared list, I think it's better.

but as a communications tool I'd say email is vastly superior, particularly
> thanks to the flexibility.
>

I agree, you right but I told about another type of flexible. From your
type of flexible, we have some sort of problems.
For example, complicated to make a good search over the archives. It's a
tradeoff and by my impression GitHub like way better but it's again
personal impression.

PS on github you can edit your message and you have highlighted for source
code...

Re: Is a modern build system acceptable for older platforms

[Yuriy Zhuravlev]

>
> It's more than just a bunch of conservative dinosaurs not wanting to
> change how they do anything,
>

I didn't talk that.

It's that a change needs to offer really compelling benefits
>

Because of this benefits depend on your development style and your habits.
For me for example, simple CMake syntax and possible to make projects files
for IDEs on many platform really compelling benefits.
If you love Perl's like syntax, work in Vim/Emacs under Linux you will have
an only extra problem with moderns build systems. (it's not bad it's just
how it work)

Re: SCRAM with channel binding downgrade attack

[Heikki Linnakangas]

On 28/05/18 12:20, Michael Paquier wrote:

On Mon, May 28, 2018 at 12:00:33PM +0300, Heikki Linnakangas wrote:

That's not a new problem, but it makes the MITM protection fairly pointless,
if a fake server can acquire the user's password by simply asking for it.
The client will report a failed connection, but with the user's password,
Mallory won't need to act as a MITM anymore.


Yeah, I know..  Do you think that it would be better to add an extra
switch/case at the beginning of pg_fe_sendauth which filters and checks
per message types then?


Sounds good.

- Heikki

Re: SCRAM with channel binding downgrade attack

[Michael Paquier]

On Mon, May 28, 2018 at 12:00:33PM +0300, Heikki Linnakangas wrote:
> That's not a new problem, but it makes the MITM protection fairly pointless,
> if a fake server can acquire the user's password by simply asking for it.
> The client will report a failed connection, but with the user's password,
> Mallory won't need to act as a MITM anymore.

Yeah, I know..  Do you think that it would be better to add an extra
switch/case at the beginning of pg_fe_sendauth which filters and checks
per message types then?
--
Michael


signature.asc
Description: PGP signature

Re: Is a modern build system acceptable for older platforms

[Magnus Hagander]

On Mon, May 28, 2018, 10:03 Yuriy Zhuravlev  wrote:

> пн, 28 мая 2018 г. в 16:42, Pierre Ducroquet :
>
>> On Monday, May 28, 2018 4:37:06 AM CEST Yuriy Zhuravlev wrote:
>> > > Can't see getting rid of those entirely. None of the github style
>> > > platforms copes with reasonable complex discussions.
>> >
>> > I disagree. A good example of complex discussions on github is Rust
>> > language tracker for RFCs:
>> > https://github.com/rust-lang/rfcs/issues
>> > and one concrete example: https://github.com/rust-lang/rfcs/issues/2327
>> > I have no any problem with complex discussions on github.
>>
>> It is indeed hard to follow on github, and would be even worse with
>> bigger
>> threads.
>> Email readers show threads in a hierarchical way, we can see who answered
>> to
>> who, discussions can fork to completely different aspects of an issue
>> without
>> being mixed together.
>>
> Anyway I have no this feature on GMail web interface. But yes, sometimes
> it's usefull.
>

It is correct that Gmail is incapable of this in the web browser. Many
other email systems can though, and Gmail still speaks imap so you can use
those if you prefer.

Which outlines a huge advantage of email as the communications medium. This
allows each and every person to pick a tool and interface that suits them.
Some prefer Gmail web, others mutt, others gnus. And they all work.

With something like github issues everybody is forced to use the same, more
limited, interface,with no choice in the matter.



>> > Anyway, it's much better than tons of emails in your mailbox without
>> tags
>> > and status of discussion.
>>
>> A github thread does not show what I read / what I have to read, does it
>> now ?
>>
> On github you have notifications about new messages in subsribed issues,
> and if you follow links from https://github.com/notifications these links
> disappear.
>

This works similar to unread threads in most mail programs, including
Gmail, doesn't it? And the subscribed issue functionality you can easily
get in Gmail by using starred threads for example?

And the read/unread can be handled both on a thread basis and individual
message basis depending on preference, but with issues it's only on thread
level.

Also, don't forget about browser bookmarks and other plugins for that, web
> much more flexible than emails.
>

I would argue the exact opposite - mail is a lot more flexible than using
github issues and that's one of the most important reasons I prefer it.

(and there are of course many ways to tag and categorize your email, many
more so than with issues. Specifically bookmarks will of course depend on
your mail program)

There are definitely advantages with issues for tracking, such as getting a
more structured central repository of them (but that requires very strict
rules for how to apply them and huge amounts of maintenance effort), but as
a communications tool I'd say email is vastly superior, particularly thanks
to the flexibility.

/Magnus

Re: pg_replication_slot_advance to return NULL instead of 0/0 if slot not advanced

[Michael Paquier]

On Mon, May 28, 2018 at 05:57:47PM +0900, Michael Paquier wrote:
> Found one.  All the things I have spotted are in the patch attached.

Oops, forgot a ReplicationSlotRelease call.
--
Michael
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 056628fe8e..79d7a57d67 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1016,7 +1016,9 @@ ReplicationSlotReserveWal(void)
 			XLogRecPtr	flushptr;
 
 			/* start at current insert position */
+			SpinLockAcquire(>mutex);
 			slot->data.restart_lsn = GetXLogInsertRecPtr();
+			SpinLockRelease(>mutex);
 
 			/* make sure we have enough information to start */
 			flushptr = LogStandbySnapshot();
@@ -1026,7 +1028,9 @@ ReplicationSlotReserveWal(void)
 		}
 		else
 		{
+			SpinLockAcquire(>mutex);
 			slot->data.restart_lsn = GetRedoRecPtr();
+			SpinLockRelease(>mutex);
 		}
 
 		/* prevent WAL removal as fast as possible */
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index d9e10263bb..1b8fd951d5 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -413,7 +413,9 @@ pg_logical_replication_slot_advance(XLogRecPtr startlsn, XLogRecPtr moveto)
 			ReplicationSlotMarkDirty();
 		}
 
+		SpinLockAcquire(>mutex);
 		retlsn = MyReplicationSlot->data.confirmed_flush;
+		SpinLockRelease(>mutex);
 
 		/* free context, call shutdown callback */
 		FreeDecodingContext(ctx);
@@ -472,7 +474,13 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
 	/* Acquire the slot so we "own" it */
 	ReplicationSlotAcquire(NameStr(*slotname), true);
 
-	startlsn = MyReplicationSlot->data.confirmed_flush;
+	SpinLockAcquire(>mutex);
+	if (OidIsValid(MyReplicationSlot->data.database))
+		startlsn = MyReplicationSlot->data.confirmed_flush;
+	else
+		startlsn = MyReplicationSlot->data.restart_lsn;
+	SpinLockRelease(>mutex);
+
 	if (moveto < startlsn)
 	{
 		ReplicationSlotRelease();
@@ -488,6 +496,13 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
 	else
 		endlsn = pg_physical_replication_slot_advance(startlsn, moveto);
 
+	if (XLogRecPtrIsInvalid(endlsn))
+	{
+		ReplicationSlotRelease();
+		ereport(ERROR,
+(errmsg("slot not moved forward as position is already reached")));
+	}
+
 	values[0] = NameGetDatum(>data.name);
 	nulls[0] = false;
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e47ddca6bc..0b1f1ba3c1 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1040,7 +1040,9 @@ DropReplicationSlot(DropReplicationSlotCmd *cmd)
 static void
 StartLogicalReplication(StartReplicationCmd *cmd)
 {
-	StringInfoData buf;
+	StringInfoData	buf;
+	XLogRecPtr		restart_lsn;
+	XLogRecPtr		confirmed_lsn;
 
 	/* make sure that our requirements are still fulfilled */
 	CheckLogicalDecodingRequirements();
@@ -1081,14 +1083,20 @@ StartLogicalReplication(StartReplicationCmd *cmd)
  WalSndWriteData,
  WalSndUpdateProgress);
 
+	/* Fetch all needed values from the slot */
+	SpinLockAcquire(>mutex);
+	restart_lsn = MyReplicationSlot->data.restart_lsn;
+	confirmed_lsn = MyReplicationSlot->data.confirmed_flush;
+	SpinLockRelease(>mutex);
+
 	/* Start reading WAL from the oldest required WAL. */
-	logical_startptr = MyReplicationSlot->data.restart_lsn;
+	logical_startptr = restart_lsn;
 
 	/*
 	 * Report the location after which we'll send out further commits as the
 	 * current sentPtr.
 	 */
-	sentPtr = MyReplicationSlot->data.confirmed_flush;
+	sentPtr = confirmed_lsn;
 
 	/* Also update the sent position status in shared memory */
 	SpinLockAcquire(>mutex);


signature.asc
Description: PGP signature

Re: SCRAM with channel binding downgrade attack

[Heikki Linnakangas]

On 28/05/18 04:23, Michael Paquier wrote:

On Sat, May 26, 2018 at 11:42:38PM +0900, Michael Paquier wrote:

On Sat, May 26, 2018 at 09:08:50AM -0400, Bruce Momjian wrote:

On Sat, May 26, 2018 at 08:32:20AM +0900, Michael Paquier wrote:


OK, I can live with that as well.  So we'll go in the direction of two
parameters then:
- scram_channel_binding, which can use "prefer" (default), "require" or
"disable".
- scram_channel_binding_name, developer option to choose the type of
channel binding, with "tls-unique" (default) and "tls-server-end-point".
We could also remove the prefix "scram_".  Ideas of names are welcome.


scram_channel_binding_method?


Or scram_channel_binding_type.  The first sentence of RFC 5929 uses this
term.


I just went with scram_channel_binding_mode (require, disable and
prefer) and scram_channel_binding_type as parameter names, in the shape
of the attached patch.


Thanks! Getting better.

There's one pretty fatal bug in this patch: If you use 
"scram_channel_binding=require", we only fail the connection after going 
through the motions of authenticating with whatever authentication the 
server asked for. That's bad, because it means that the client will 
merrily respond to a AUTH_REQ_PASSWORD request from the server, by 
sending the password in cleartext.


That's not a new problem, but it makes the MITM protection fairly 
pointless, if a fake server can acquire the user's password by simply 
asking for it. The client will report a failed connection, but with the 
user's password, Mallory won't need to act as a MITM anymore.


- Heikki

Re: pg_replication_slot_advance to return NULL instead of 0/0 if slot not advanced

[Michael Paquier]

On Fri, May 25, 2018 at 02:12:32PM +0200, Magnus Hagander wrote:
> I agree that returning 0/0 on this is wrong.
> 
> However, can this actually occour for any case other than exactly the case
> of "moving the position to where the position already is"? If I look at the
> physical slot path at least that seems to be the only case, and in that
> case I think the correct thing to return would be the new position, and not
> NULL. If we actually *fail* to move the position, we give an error.

Yes, this only returns InvalidXLogRecPtr if the location could not be
moved forward.  Still, there is more going on here.  For a physical
slot, confirmed_lsn is always 0/0, hence the backward check is never
applied for it.  What I think should be used for value assigned to
startlsn is restart_lsn instead.  Then what do we do if the position
cannot be moved: should we raise an error, as what my patch attached
does, or just report the existing position the slot is at?

A second error that I can see is in pg_logical_replication_slot_advance,
which does not take the mutex lock of MyReplicationSlot, so concurrent
callers like those of ReplicationSlotsComputeRequiredLSN (applies to
physical slot actually) and pg_get_replication_slots() may read false
data.

On top of that, it seems to me that StartLogicalReplication is reading a
couple of fields from a slot without taking a lock, so at least
pg_get_replication_slots() may read incorrect data.
ReplicationSlotReserveWal also is missing a lock..  Those are older than
v11 though.

> Actually, isn't there also a race there? That is, if we try to move it, we
> check that we're not trying to move it backwards, and throw an error, but
> that's checked outside the lock. Then later we actually move it, and check
> *again* if we try to move it backwards, but if that one fails we return
> InvalidXLogRecPtr (which can happen in the case of concurrent activity on
> the slot, I think)? In this case, maybe we should just re-check that and
> raise an error appropriately?

Er, isn't the take here that ReplicationSlotAcquire is used to lock any
replication slot update to happen from other backends?  It seems to me
that what counts at the end if if a backend PID is associated to a slot
in memory.  If you look at the code paths updating a logical or physical
slot then those imply that the slot is owned, still a spin lock needs to
be taken for concurrent readers.

> (I haven't looked at the logical slot path, but I assume it would have
> something similar in it)

Found one.  All the things I have spotted are in the patch attached.
--
Michael
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 056628fe8e..79d7a57d67 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1016,7 +1016,9 @@ ReplicationSlotReserveWal(void)
 			XLogRecPtr	flushptr;
 
 			/* start at current insert position */
+			SpinLockAcquire(>mutex);
 			slot->data.restart_lsn = GetXLogInsertRecPtr();
+			SpinLockRelease(>mutex);
 
 			/* make sure we have enough information to start */
 			flushptr = LogStandbySnapshot();
@@ -1026,7 +1028,9 @@ ReplicationSlotReserveWal(void)
 		}
 		else
 		{
+			SpinLockAcquire(>mutex);
 			slot->data.restart_lsn = GetRedoRecPtr();
+			SpinLockRelease(>mutex);
 		}
 
 		/* prevent WAL removal as fast as possible */
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index d9e10263bb..4cf2aef95f 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -413,7 +413,9 @@ pg_logical_replication_slot_advance(XLogRecPtr startlsn, XLogRecPtr moveto)
 			ReplicationSlotMarkDirty();
 		}
 
+		SpinLockAcquire(>mutex);
 		retlsn = MyReplicationSlot->data.confirmed_flush;
+		SpinLockRelease(>mutex);
 
 		/* free context, call shutdown callback */
 		FreeDecodingContext(ctx);
@@ -472,7 +474,13 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
 	/* Acquire the slot so we "own" it */
 	ReplicationSlotAcquire(NameStr(*slotname), true);
 
-	startlsn = MyReplicationSlot->data.confirmed_flush;
+	SpinLockAcquire(>mutex);
+	if (OidIsValid(MyReplicationSlot->data.database))
+		startlsn = MyReplicationSlot->data.confirmed_flush;
+	else
+		startlsn = MyReplicationSlot->data.restart_lsn;
+	SpinLockRelease(>mutex);
+
 	if (moveto < startlsn)
 	{
 		ReplicationSlotRelease();
@@ -488,6 +496,10 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
 	else
 		endlsn = pg_physical_replication_slot_advance(startlsn, moveto);
 
+	if (XLogRecPtrIsInvalid(endlsn))
+		ereport(ERROR,
+(errmsg("slot not moved forward as position is already reached")));
+
 	values[0] = NameGetDatum(>data.name);
 	nulls[0] = false;
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e47ddca6bc..0b1f1ba3c1 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1040,7 +1040,9 @@ DropReplicationSlot(DropReplicationSlotCmd *cmd)
 static void
 

Few comments on commit 857f9c36 (skip full index scans )

[Amit Kapila]

Hi,

Review comments on commit 857f9c36:
1.
@@ -2049,6 +2055,10 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
metapg = BufferGetPage(metabuf);
metad = BTPageGetMeta(metapg);

+   /* upgrade metapage if needed */
+   if (metad->btm_version < BTREE_VERSION)
+   _bt_upgrademetapage(metapg);
+

The metapage upgrade should be performed under critical section.

2.
@@ -191,6 +304,10 @@ _bt_getroot(Relation rel, int access)
LockBuffer(metabuf, BUFFER_LOCK_UNLOCK);
LockBuffer(metabuf, BT_WRITE);

+   /* upgrade metapage if needed */
+   if (metad->btm_version < BTREE_VERSION)
+   _bt_upgrademetapage(metapg);
+

Same as above.

In other cases like _bt_insertonpg, the upgrade is done inside the
critical section.  It seems the above cases are missed.

3.
+   TransactionId btm_oldest_btpo_xact; /* oldest btpo_xact among of
+* deleted pages */

/among of/among all

Attached patch to fix the above comments.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


fix_review_skip_full_index_scan_v1.patch
Description: Binary data

Re: Keeping temporary tables in shared buffers

[Amit Kapila]

On Fri, May 25, 2018 at 6:33 AM, Asim Praveen  wrote:
> Hello
>
> We are evaluating the use of shared buffers for temporary tables.  The
> advantage being queries involving temporary tables can make use of parallel
> workers.
>

This is one way, but I think there are other choices as well.  We can
identify and flush all the dirty (local) buffers for the relation
being accessed parallelly.  Now, once the parallel operation is
started, we won't allow performing any write operation on them.  It
could be expensive if we have a lot of dirty local buffers for a
particular relation.  I think if we are worried about the cost of
writes, then we can try some different way to parallelize temporary
table scan.  At the beginning of the scan, leader backend will
remember the dirty blocks present in local buffers, it can then share
the list with parallel workers which will skip scanning those blocks
and in the end leader ensures that all those blocks will be scanned by
the leader.  This shouldn't incur a much additional cost as the
skipped blocks should be present in local buffers of backend.

I understand that none of these alternatives are straight-forward, but
I think it is worth considering whether we have any better way to
allow parallel temporary table scans.


> Challenges:
> 1. We lose the performance benefit of local buffers.

Yeah, I think cases, where we need to drop temp relations, will become
costlier as they have to traverse all the shared buffers instead of
just local buffers.

I think if we use shared buffers for temp relations, there will be
some overhead for other backends as well, especially for the cases
when backends need to evict buffers.  It is quite possible that if the
relation is in local buffers, we might not write it at all, but moving
it to shared buffers will increase its probability of being written to
disk.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: Is a modern build system acceptable for older platforms

[Yuriy Zhuravlev]

пн, 28 мая 2018 г. в 16:42, Pierre Ducroquet :

> On Monday, May 28, 2018 4:37:06 AM CEST Yuriy Zhuravlev wrote:
> > > Can't see getting rid of those entirely. None of the github style
> > > platforms copes with reasonable complex discussions.
> >
> > I disagree. A good example of complex discussions on github is Rust
> > language tracker for RFCs:
> > https://github.com/rust-lang/rfcs/issues
> > and one concrete example: https://github.com/rust-lang/rfcs/issues/2327
> > I have no any problem with complex discussions on github.
>
> It is indeed hard to follow on github, and would be even worse with bigger
> threads.
> Email readers show threads in a hierarchical way, we can see who answered
> to
> who, discussions can fork to completely different aspects of an issue
> without
> being mixed together.
>
Anyway I have no this feature on GMail web interface. But yes, sometimes
it's usefull.
On github you can make new issue and move some messages, it shoud be done
by moderator.


>
> > Anyway, it's much better than tons of emails in your mailbox without tags
> > and status of discussion.
>
> A github thread does not show what I read / what I have to read, does it
> now ?
>
On github you have notifications about new messages in subsribed issues,
and if you follow links from https://github.com/notifications these links
disappear.
Also, don't forget about browser bookmarks and other plugins for that, web
much more flexible than emails.

Re: Is a modern build system acceptable for older platforms

[Pierre Ducroquet]

On Monday, May 28, 2018 4:37:06 AM CEST Yuriy Zhuravlev wrote:
> > Can't see getting rid of those entirely. None of the github style
> > platforms copes with reasonable complex discussions.
> 
> I disagree. A good example of complex discussions on github is Rust
> language tracker for RFCs:
> https://github.com/rust-lang/rfcs/issues
> and one concrete example: https://github.com/rust-lang/rfcs/issues/2327
> I have no any problem with complex discussions on github.

It is indeed hard to follow on github, and would be even worse with bigger 
threads.
Email readers show threads in a hierarchical way, we can see who answered to 
who, discussions can fork to completely different aspects of an issue without 
being mixed together.

> Anyway, it's much better than tons of emails in your mailbox without tags
> and status of discussion.

A github thread does not show what I read / what I have to read, does it now ?

[PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

[Craig Ringer]

Hi

Per topic, the Pg makefiles install pg_regress (for use by extensions) and
htey install the isolationtester, but they don't install
pg_isolation_regress.

We should install it too.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 47857238a47653081e4d88e2ba14f8712349ebbd Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Mon, 28 May 2018 15:04:42 +0800
Subject: [PATCH] Install pg_isolation_regress

---
 src/test/regress/GNUmakefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 587de56fb9..c1fe375f46 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -48,6 +48,7 @@ $(top_builddir)/src/port/pg_config_paths.h: $(top_builddir)/src/Makefile.global
 
 install: all installdirs
 	$(INSTALL_PROGRAM) pg_regress$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_regress$(X)'
+	$(INSTALL_PROGRAM) pg_isolation_regress$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_isolation_regress$(X)'
 
 installdirs:
 	$(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
-- 
2.14.3