pgsql: Refactor XLogReadRecord(), adding XLogBeginRead() function.

2020-01-26 Thread Heikki Linnakangas
Refactor XLogReadRecord(), adding XLogBeginRead() function.

The signature of XLogReadRecord() required the caller to pass the starting
WAL position as argument, or InvalidXLogRecPtr to continue reading at the
end of previous record. That's slightly awkward to the callers, as most
of them don't want to randomly jump around in the WAL stream, but start
reading at one position and then read everything from that point onwards.
Remove the 'RecPtr' argument and add a new function XLogBeginRead() to
specify the starting position instead. That's more convenient for the
callers. Also, xlogreader holds state that is reset when you change the
starting position, so having a separate function for doing that feels like
a more natural fit.

This changes XLogFindNextRecord() function so that it doesn't reset the
xlogreader's state to what it was before the call anymore. Instead, it
positions the xlogreader to the found record, like XLogBeginRead().

Reviewed-by: Kyotaro Horiguchi, Alvaro Herrera
Discussion: 
https://www.postgresql.org/message-id/5382a7a3-debe-be31-c860-cb810c08f366%40iki.fi

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/38a957316d7e46d4b00de40f43966984a463d80a

Modified Files
--
src/backend/access/transam/twophase.c  |  3 +-
src/backend/access/transam/xlog.c  | 36 ++--
src/backend/access/transam/xlogreader.c| 79 --
src/backend/replication/logical/logical.c  |  7 +--
src/backend/replication/logical/logicalfuncs.c | 14 +
src/backend/replication/slotfuncs.c| 13 +
src/backend/replication/walsender.c|  8 +--
src/bin/pg_rewind/parsexlog.c  | 15 +++--
src/bin/pg_waldump/pg_waldump.c|  5 +-
src/include/access/xlogreader.h| 19 ---
10 files changed, 102 insertions(+), 97 deletions(-)



pgsql: In postgres_fdw, don't try to ship MULTIEXPR updates to remote s

2020-01-26 Thread Tom Lane
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.

In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
we'd conclude that the statement could be directly executed remotely,
because the sub-SELECT is in a resjunk tlist item that's not examined
for shippability.  Currently that ends up crashing if the sub-SELECT
contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
Params to be unshippable.

This is a bit of a brute-force solution, since if the sub-SELECT
*doesn't* contain any remote Vars, the current execution technology
would work; but that's not a terribly common use-case for this syntax,
I think.  In any case, we generally don't try to ship sub-SELECTs, so
it won't surprise anybody that this doesn't end up as a remote direct
update.  I'd be inclined to see if that general limitation can be fixed
before worrying about this case further.

Per report from Lukáš Sobotka.

Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
remote direct updates then, so the case didn't arise anyway.

Discussion: 
https://postgr.es/m/cajif3k+ia_ekbb5zw2hdbae1wtiqa4lh4_juxrrmgwtrh0j...@mail.gmail.com

Branch
--
REL_12_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/7294f99a0b2043cf9b34a9a59ecb3dfbfae94f85

Modified Files
--
contrib/postgres_fdw/deparse.c | 16 +
contrib/postgres_fdw/expected/postgres_fdw.out | 31 ++
contrib/postgres_fdw/sql/postgres_fdw.sql  | 20 +
3 files changed, 67 insertions(+)



pgsql: In postgres_fdw, don't try to ship MULTIEXPR updates to remote s

2020-01-26 Thread Tom Lane
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.

In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
we'd conclude that the statement could be directly executed remotely,
because the sub-SELECT is in a resjunk tlist item that's not examined
for shippability.  Currently that ends up crashing if the sub-SELECT
contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
Params to be unshippable.

This is a bit of a brute-force solution, since if the sub-SELECT
*doesn't* contain any remote Vars, the current execution technology
would work; but that's not a terribly common use-case for this syntax,
I think.  In any case, we generally don't try to ship sub-SELECTs, so
it won't surprise anybody that this doesn't end up as a remote direct
update.  I'd be inclined to see if that general limitation can be fixed
before worrying about this case further.

Per report from Lukáš Sobotka.

Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
remote direct updates then, so the case didn't arise anyway.

Discussion: 
https://postgr.es/m/cajif3k+ia_ekbb5zw2hdbae1wtiqa4lh4_juxrrmgwtrh0j...@mail.gmail.com

Branch
--
REL_10_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/603e03b4c9e8f853ab15da45e6e72df718ec335c

Modified Files
--
contrib/postgres_fdw/deparse.c | 16 +
contrib/postgres_fdw/expected/postgres_fdw.out | 31 ++
contrib/postgres_fdw/sql/postgres_fdw.sql  | 20 +
3 files changed, 67 insertions(+)



pgsql: In postgres_fdw, don't try to ship MULTIEXPR updates to remote s

2020-01-26 Thread Tom Lane
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.

In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
we'd conclude that the statement could be directly executed remotely,
because the sub-SELECT is in a resjunk tlist item that's not examined
for shippability.  Currently that ends up crashing if the sub-SELECT
contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
Params to be unshippable.

This is a bit of a brute-force solution, since if the sub-SELECT
*doesn't* contain any remote Vars, the current execution technology
would work; but that's not a terribly common use-case for this syntax,
I think.  In any case, we generally don't try to ship sub-SELECTs, so
it won't surprise anybody that this doesn't end up as a remote direct
update.  I'd be inclined to see if that general limitation can be fixed
before worrying about this case further.

Per report from Lukáš Sobotka.

Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
remote direct updates then, so the case didn't arise anyway.

Discussion: 
https://postgr.es/m/cajif3k+ia_ekbb5zw2hdbae1wtiqa4lh4_juxrrmgwtrh0j...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/215824f9188a2b19f870e6a707c5a81e1ac3f1fc

Modified Files
--
contrib/postgres_fdw/deparse.c | 16 +
contrib/postgres_fdw/expected/postgres_fdw.out | 31 ++
contrib/postgres_fdw/sql/postgres_fdw.sql  | 20 +
3 files changed, 67 insertions(+)



pgsql: In postgres_fdw, don't try to ship MULTIEXPR updates to remote s

2020-01-26 Thread Tom Lane
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.

In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
we'd conclude that the statement could be directly executed remotely,
because the sub-SELECT is in a resjunk tlist item that's not examined
for shippability.  Currently that ends up crashing if the sub-SELECT
contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
Params to be unshippable.

This is a bit of a brute-force solution, since if the sub-SELECT
*doesn't* contain any remote Vars, the current execution technology
would work; but that's not a terribly common use-case for this syntax,
I think.  In any case, we generally don't try to ship sub-SELECTs, so
it won't surprise anybody that this doesn't end up as a remote direct
update.  I'd be inclined to see if that general limitation can be fixed
before worrying about this case further.

Per report from Lukáš Sobotka.

Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
remote direct updates then, so the case didn't arise anyway.

Discussion: 
https://postgr.es/m/cajif3k+ia_ekbb5zw2hdbae1wtiqa4lh4_juxrrmgwtrh0j...@mail.gmail.com

Branch
--
REL9_6_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/43a648f57ce3d45fc77b0d18b0829884d8449151

Modified Files
--
contrib/postgres_fdw/deparse.c | 16 +
contrib/postgres_fdw/expected/postgres_fdw.out | 31 ++
contrib/postgres_fdw/sql/postgres_fdw.sql  | 20 +
3 files changed, 67 insertions(+)



pgsql: In postgres_fdw, don't try to ship MULTIEXPR updates to remote s

2020-01-26 Thread Tom Lane
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.

In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
we'd conclude that the statement could be directly executed remotely,
because the sub-SELECT is in a resjunk tlist item that's not examined
for shippability.  Currently that ends up crashing if the sub-SELECT
contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
Params to be unshippable.

This is a bit of a brute-force solution, since if the sub-SELECT
*doesn't* contain any remote Vars, the current execution technology
would work; but that's not a terribly common use-case for this syntax,
I think.  In any case, we generally don't try to ship sub-SELECTs, so
it won't surprise anybody that this doesn't end up as a remote direct
update.  I'd be inclined to see if that general limitation can be fixed
before worrying about this case further.

Per report from Lukáš Sobotka.

Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
remote direct updates then, so the case didn't arise anyway.

Discussion: 
https://postgr.es/m/cajif3k+ia_ekbb5zw2hdbae1wtiqa4lh4_juxrrmgwtrh0j...@mail.gmail.com

Branch
--
REL_11_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/5220ced0de5de1b88437c00fffabfac223ee9866

Modified Files
--
contrib/postgres_fdw/deparse.c | 16 +
contrib/postgres_fdw/expected/postgres_fdw.out | 31 ++
contrib/postgres_fdw/sql/postgres_fdw.sql  | 20 +
3 files changed, 67 insertions(+)



pgsql: Refactor confusing code in _mdfd_openseg().

2020-01-26 Thread Thomas Munro
Refactor confusing code in _mdfd_openseg().

As reported independently by a couple of people, _mdfd_openseg() is coded in a
way that seems to imply that the segments could be opened in an order that
isn't strictly sequential.  Even if that were true, it's also using the wrong
comparison.  It's not an active bug, since the condition is always true anyway,
but it's confusing, so replace it with an assertion.

Author: Thomas Munro
Reviewed-by: Andres Freund, Kyotaro Horiguchi, Noah Misch
Discussion: 
https://postgr.es/m/CA%2BhUKG%2BNBw%2BuSzxF1os-SO6gUuw%3DcqO5DAybk6KnHKzgGvxhxA%40mail.gmail.com
Discussion: https://postgr.es/m/20191222091930.GA1280238%40rfd.leadboat.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/f37ff03478aefb5e01d748b85ad86e6213624992

Modified Files
--
src/backend/storage/smgr/md.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)



pgsql: Fix EXPLAIN (SETTINGS) to follow policy about when to print empt

2020-01-26 Thread Tom Lane
Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.

In non-TEXT output formats, the "Settings" field should appear when
requested, even if it would be empty.

Also, get rid of the premature optimization of counting all the
GUC_EXPLAIN variables at startup.  Since there was no provision for
adjusting that count later, all it'd take would be some extension marking
a parameter as GUC_EXPLAIN to risk an assertion failure or memory stomp.
We could make get_explain_guc_options() count those variables on-the-fly,
or dynamically resize its array ... but TBH I do not think that making a
transient array of pointers a bit smaller is worth any extra complication,
especially when you consider all the other transient space EXPLAIN eats.
So just allocate that array at the max possible size.

In HEAD, also add some regression test coverage for this feature.

Because of the memory-stomp hazard, back-patch to v12 where this
feature was added.

Discussion: https://postgr.es/m/[email protected]

Branch
--
REL_12_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/bad494380210a76071485c5d8624334c4389dd88

Modified Files
--
src/backend/commands/explain.c | 18 ++
src/backend/utils/misc/guc.c   | 54 --
2 files changed, 22 insertions(+), 50 deletions(-)



pgsql: Fix EXPLAIN (SETTINGS) to follow policy about when to print empt

2020-01-26 Thread Tom Lane
Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.

In non-TEXT output formats, the "Settings" field should appear when
requested, even if it would be empty.

Also, get rid of the premature optimization of counting all the
GUC_EXPLAIN variables at startup.  Since there was no provision for
adjusting that count later, all it'd take would be some extension marking
a parameter as GUC_EXPLAIN to risk an assertion failure or memory stomp.
We could make get_explain_guc_options() count those variables on-the-fly,
or dynamically resize its array ... but TBH I do not think that making a
transient array of pointers a bit smaller is worth any extra complication,
especially when you consider all the other transient space EXPLAIN eats.
So just allocate that array at the max possible size.

In HEAD, also add some regression test coverage for this feature.

Because of the memory-stomp hazard, back-patch to v12 where this
feature was added.

Discussion: https://postgr.es/m/[email protected]

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/3ec20c7091e97a554e7447ac2b7f4ed795631395

Modified Files
--
src/backend/commands/explain.c| 18 +---
src/backend/utils/misc/guc.c  | 54 ++-
src/test/regress/expected/explain.out | 20 +
src/test/regress/sql/explain.sql  | 12 
4 files changed, 54 insertions(+), 50 deletions(-)



pgsql: Fix some memory leaks and improve restricted token handling on W

2020-01-26 Thread Michael Paquier
Fix some memory leaks and improve restricted token handling on Windows

The leaks have been detected by a Coverity run on Windows.  No backpatch
is done as the leaks are minor.

While on it, make restricted token creation more consistent in its error
handling by logging an error instead of a warning if missing
advapi32.dll, which was missing in the NT4 days.  Any modern platform
should have this DLL around.  Now, if the library is not there, an error
is still reported back to the caller, and nothing is done do there is no
behavior change done in this commit.

Author: Ranier Vilela
Discussion: 
https://postgr.es/m/CAEudQApa9MG0foPkgPX87fipk=vhnf2xfg+cfuyr08h4r7m...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/10a525230fb18331dbcfd6a4a7248d76f55c331c

Modified Files
--
src/backend/libpq/auth.c| 14 ++
src/backend/postmaster/postmaster.c |  6 ++
src/common/restricted_token.c   | 34 ++
3 files changed, 42 insertions(+), 12 deletions(-)



pgsql: Avoid unnecessary shm writes in Parallel Hash Join.

2020-01-26 Thread Thomas Munro
Avoid unnecessary shm writes in Parallel Hash Join.

Currently, Parallel Hash Join cannot be used for full/right joins,
so there is no point in setting the match flag.  It turns out that
the cache coherence traffic generated by those writes slows down
large systems running many-core joins, so let's stop doing that.
In future, if we need to use match bits in parallel joins, we might
want to consider setting them only if not already set.

Back-patch to 11, where Parallel Hash Join arrived.

Reported-by: Deng, Gang
Discussion: 
https://postgr.es/m/0F44E799048C4849BAE4B91012DB910462E9897A%40SHSMSX103.ccr.corp.intel.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/3e4818e9dd5be294d97ca67012528cb1c0b0ccaa

Modified Files
--
src/backend/executor/nodeHashjoin.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)



pgsql: Avoid unnecessary shm writes in Parallel Hash Join.

2020-01-26 Thread Thomas Munro
Avoid unnecessary shm writes in Parallel Hash Join.

Currently, Parallel Hash Join cannot be used for full/right joins,
so there is no point in setting the match flag.  It turns out that
the cache coherence traffic generated by those writes slows down
large systems running many-core joins, so let's stop doing that.
In future, if we need to use match bits in parallel joins, we might
want to consider setting them only if not already set.

Back-patch to 11, where Parallel Hash Join arrived.

Reported-by: Deng, Gang
Discussion: 
https://postgr.es/m/0F44E799048C4849BAE4B91012DB910462E9897A%40SHSMSX103.ccr.corp.intel.com

Branch
--
REL_12_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/f9d0be241aacc4913302080594f5a342b8b9dc15

Modified Files
--
src/backend/executor/nodeHashjoin.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)



pgsql: Avoid unnecessary shm writes in Parallel Hash Join.

2020-01-26 Thread Thomas Munro
Avoid unnecessary shm writes in Parallel Hash Join.

Currently, Parallel Hash Join cannot be used for full/right joins,
so there is no point in setting the match flag.  It turns out that
the cache coherence traffic generated by those writes slows down
large systems running many-core joins, so let's stop doing that.
In future, if we need to use match bits in parallel joins, we might
want to consider setting them only if not already set.

Back-patch to 11, where Parallel Hash Join arrived.

Reported-by: Deng, Gang
Discussion: 
https://postgr.es/m/0F44E799048C4849BAE4B91012DB910462E9897A%40SHSMSX103.ccr.corp.intel.com

Branch
--
REL_11_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/6a9fc75d0391e41877ed0a7e6d5696090015

Modified Files
--
src/backend/executor/nodeHashjoin.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)