Commit Graph

513 Commits

Author SHA1 Message Date
Tom Lane a3021aafce Make postgres_fdw request remote time zone 'GMT' not 'UTC'.
This should have the same results for all practical purposes.
The advantage of selecting 'GMT' is that it's guaranteed to work
even when the remote system's timezone database is missing
entries, because pg_tzset() hard-wires handling of that,
at least in 9.2 and later.

(It seems like it would be a good idea to similarly hard-wire
correct handling of 'UTC', but that'll be a little more invasive
than I want to consider back-patching.  Leave that for another
day when we're not in feature freeze.)

Per trouble report from Adnan Dautovic.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/465248.1712211585@sss.pgh.pa.us
2024-04-21 13:46:20 -04:00
Etsuro Fujita 56c6703bd0 postgres_fdw: Improve comment about handling of asynchronous requests.
We updated this comment in back branches (see commit f6f61a4bd et al);
let's do so in HEAD as well for consistency.

Discussion: https://postgr.es/m/CAPmGK142V1kqDfjo2H%2Bb54JTn2woVBrisFq%2B%3D9jwXwxr0VvbgA%40mail.gmail.com
2024-04-11 19:25:00 +09:00
David Rowley 8461424fd7 Fixup various StringInfo function usages
This adjusts various appendStringInfo* function calls to use a more
appropriate and efficient function with the same behavior.  For example,
use appendStringInfoChar() when appending a single character rather than
appendStringInfo() and appendStringInfoString() when no formatting is
required rather than using appendStringInfo().

All adjustments made here are in code that's new to v17, so it makes
sense to fix these now rather than wait a few years and make
backpatching harder.

Discussion: https://postgr.es/m/CAApHDvojY2UvMiO+9_55ArTj10P1LBNJyyoGB+C65BLDNT0GsQ@mail.gmail.com
Reviewed-by: Nathan Bossart, Tom Lane
2024-04-10 11:53:32 +12:00
Alvaro Herrera b8b37e41ba
Make libpqsrv_cancel's return const char *, not char *
Per headerscheck's C++ check.

Discussion: https://postgr.es/m/372769.1712179784@sss.pgh.pa.us
2024-04-05 18:23:10 +02:00
Etsuro Fujita dd24098cd6 postgres_fdw: Remove useless ternary expression.
There is no case where we would call pgfdw_exec_cleanup_query or
pgfdw_exec_cleanup_query_{begin,end} with a NULL query string, so this
expression is pointless; remove it and instead add to the latter
functions an assertion ensuring the given query string is not NULL.

Thinko in commit 815d61fcd.

Discussion: https://postgr.es/m/CAPmGK14mm%2B%3DUjyjoWj_Hu7c%2BQqX-058RFfF%2BqOkcMZ_Nj52v-A%40mail.gmail.com
2024-04-04 17:55:00 +09:00
Alvaro Herrera ecf741cfae
Stabilize postgres_fdw test
The test fails when RESET statement_timeout takes longer than 10ms.
Avoid the problem by using SET LOCAL instead.

Overall, this test is not ideal: 10ms could be shorter than the time to
have sent the query to the "remote" server, so it's possible that on
some machines this test doesn't actually witness a remote query being
cancelled.  We may want to improve on this someday by using some other
testing technique, but for now it's better than nothing.  I verified
manually that one round of remote cancellation occurs when this runs on
my machine.

Discussion: https://postgr.es/m/CAGECzQRsdWnj=YaaPCnA8d7E1AdbxRPBYmyBQRMPUijR2MpM_w@mail.gmail.com
2024-03-30 14:16:54 +01:00
Alvaro Herrera 2466d6654f
libpq-be-fe-helpers.h: wrap new cancel APIs
Commit 61461a300c introduced new functions to libpq for cancelling
queries.  This commit introduces a helper function that backend-side
libraries and extensions can use to invoke those.  This function takes a
timeout and can itself be interrupted while it is waiting for a cancel
request to be sent and processed, instead of being blocked.

This replaces the usage of the old functions in postgres_fdw and dblink.

Finally, it also adds some test coverage for the cancel support in
postgres_fdw.

Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/CAGECzQT_VgOWWENUqvUV9xQmbaCyXjtRRAYO8W07oqashk_N+g@mail.gmail.com
2024-03-28 11:31:03 +01:00
David Rowley 66c0185a3d Allow planner to use Merge Append to efficiently implement UNION
Until now, UNION queries have often been suboptimal as the planner has
only ever considered using an Append node and making the results unique
by either using a Hash Aggregate, or by Sorting the entire Append result
and running it through the Unique operator.  Both of these methods
always require reading all rows from the union subqueries.

Here we adjust the union planner so that it can request that each subquery
produce results in target list order so that these can be Merge Appended
together and made unique with a Unique node.  This can improve performance
significantly as the union child can make use of the likes of btree
indexes and/or Merge Joins to provide the top-level UNION with presorted
input.  This is especially good if the top-level UNION contains a LIMIT
node that limits the output rows to a small subset of the unioned rows as
cheap startup plans can be used.

Author: David Rowley
Reviewed-by: Richard Guo, Andy Fan
Discussion: https://postgr.es/m/CAApHDvpb_63XQodmxKUF8vb9M7CxyUyT4sWvEgqeQU-GB7QFoQ@mail.gmail.com
2024-03-25 14:31:14 +13:00
Tom Lane fd0398fcb0 Improve EXPLAIN's display of SubPlan nodes and output parameters.
Historically we've printed SubPlan expression nodes as "(SubPlan N)",
which is pretty uninformative.  Trying to reproduce the original SQL
for the subquery is still as impractical as before, and would be
mighty verbose as well.  However, we can still do better than that.
Displaying the "testexpr" when present, and adding a keyword to
indicate the SubLinkType, goes a long way toward showing what's
really going on.

In addition, this patch gets rid of EXPLAIN's use of "$n" to represent
subplan and initplan output Params.  Instead we now print "(SubPlan
N).colX" or "(InitPlan N).colX" to represent the X'th output column
of that subplan.  This eliminates confusion with the use of "$n" to
represent PARAM_EXTERN Params, and it's useful for the first part of
this change because it eliminates needing some other indication of
which subplan is referenced by a SubPlan that has a testexpr.

In passing, this adds simple regression test coverage of the
ROWCOMPARE_SUBLINK code paths, which were entirely unburdened
by testing before.

Tom Lane and Dean Rasheed, reviewed by Aleksander Alekseev.
Thanks to Chantal Keller for raising the question of whether
this area couldn't be improved.

Discussion: https://postgr.es/m/2838538.1705692747@sss.pgh.pa.us
2024-03-19 18:19:24 -04:00
Peter Eisentraut 97d85be365 Make the order of the header file includes consistent
Similar to commit 7e735035f2.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-WhpCFMbXCjtJ%2BFzmjfPrp7Hw1pk4p%2BZpU95Kh3ofZ1A%40mail.gmail.com
2024-03-13 15:07:00 +01:00
David Rowley c399248b3b Fix deparsing of Consts in postgres_fdw ORDER BY
For UNION ALL queries where a union child query contained a foreign
table, if the targetlist of that query contained a constant, and the
top-level query performed an ORDER BY which contained the column for the
constant value, then postgres_fdw would find the EquivalenceMember with
the Const and then try to produce an ORDER BY containing that Const.

This caused problems with INT typed Consts as these could appear to be
requests to order by an ordinal column position rather than the constant
value.  This could lead to either an error such as:

ERROR:  ORDER BY position <int const> is not in select list

or worse, if the constant value is a valid column, then we could just
sort by the wrong column altogether.

Here we fix this issue by just not including these Consts in the ORDER
BY clause.

In passing, add a new section for testing ORDER BY in the postgres_fdw
tests and move two existing tests which were misplaced in the WHERE
clause testing section into it.

Reported-by: Michał Kłeczek
Reviewed-by: Ashutosh Bapat, Richard Guo
Bug: #18381
Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org
Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org
Backpatch-through: 12, oldest supported version
2024-03-11 12:27:11 +13:00
Alexander Korotkov 9f13376396 Pull up ANY-SUBLINK with the necessary lateral support.
For ANY-SUBLINK, we adopted a two-stage pull-up approach to handle
different types of scenarios. In the first stage, the sublink is pulled up
as a subquery. Because of this, when writing this code, we did not have
the ability to perform lateral joins, and therefore, we were unable to
pull up Var with varlevelsup=1. Now that we have the ability to use
lateral joins, we can eliminate this limitation.

Author: Andy Fan <zhihui.fan1213@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2024-02-15 12:06:12 +02:00
David Rowley b262ad440e Add better handling of redundant IS [NOT] NULL quals
Until now PostgreSQL has not been very smart about optimizing away IS
NOT NULL base quals on columns defined as NOT NULL.  The evaluation of
these needless quals adds overhead.  Ordinarily, anyone who came
complaining about that would likely just have been told to not include
the qual in their query if it's not required.  However, a recent bug
report indicates this might not always be possible.

Bug 17540 highlighted that when we optimize Min/Max aggregates the IS NOT
NULL qual that the planner adds to make the rewritten plan ignore NULLs
can cause issues with poor index choice.  That particular case
demonstrated that other quals, especially ones where no statistics are
available to allow the planner a chance at estimating an approximate
selectivity for can result in poor index choice due to cheap startup paths
being prefered with LIMIT 1.

Here we take generic approach to fixing this by having the planner check
for NOT NULL columns and just have the planner remove these quals (when
they're not needed) for all queries, not just when optimizing Min/Max
aggregates.

Additionally, here we also detect IS NULL quals on a NOT NULL column and
transform that into a gating qual so that we don't have to perform the
scan at all.  This also works for join relations when the Var is not
nullable by any outer join.

This also helps with the self-join removal work as it must replace
strict join quals with IS NOT NULL quals to ensure equivalence with the
original query.

Author: David Rowley, Richard Guo, Andy Fan
Reviewed-by: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAApHDvqg6XZDhYRPz0zgOcevSMo0d3vxA9DvHrZtKfqO30WTnw@mail.gmail.com
Discussion: https://postgr.es/m/17540-7aa1855ad5ec18b4%40postgresql.org
2024-01-23 18:09:18 +13:00
Noah Misch d3c5f37dd5 Make dblink interruptible, via new libpqsrv APIs.
This replaces dblink's blocking libpq calls, allowing cancellation and
allowing DROP DATABASE (of a database not involved in the query).  Apart
from explicit dblink_cancel_query() calls, dblink still doesn't cancel
the remote side.  The replacement for the blocking calls consists of
new, general-purpose query execution wrappers in the libpqsrv facility.
Out-of-tree extensions should adopt these.  Use them in postgres_fdw,
replacing a local implementation from which the libpqsrv implementation
derives.  This is a bug fix for dblink.  Code inspection identified the
bug at least thirteen years ago, but user complaints have not appeared.
Hence, no back-patch for now.

Discussion: https://postgr.es/m/20231122012945.74@rfd.leadboat.com
2024-01-08 11:39:56 -08:00
Bruce Momjian 29275b1d17 Update copyright for 2024
Reported-by: Michael Paquier

Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz

Backpatch-through: 12
2024-01-03 20:49:05 -05:00
Robert Haas 0d9937d118 Fix typos in comments and in one isolation test.
Dagfinn Ilmari Mannsåker, reviewed by Shubham Khanna. Some subtractions
by me.

Discussion: http://postgr.es/m/87le9fmi01.fsf@wibble.ilmari.org
2024-01-02 12:05:41 -05:00
Alexander Korotkov 824dbea3e4 Add support for deparsing semi-joins to contrib/postgres_fdw
SEMI-JOIN is deparsed as the EXISTS subquery. It references outer and inner
relations, so it should be evaluated as the condition in the upper-level WHERE
clause. The signatures of deparseFromExprForRel() and deparseRangeTblRef() are
revised so that they can add conditions to the upper level.

PgFdwRelationInfo now has a hidden_subquery_rels field, referencing the relids
used in the inner parts of semi-join.  They can't be referred to from upper
relations and should be used internally for equivalence member searches.

The planner can create semi-join, which refers to inner rel vars in its target
list. However, we deparse semi-join as an exists() subquery. So we skip the
case when the target list references to inner rel of semi-join.

Author: Alexander Pyhalov
Reviewed-by: Ashutosh Bapat, Ian Lawrence Barwick, Yuuki Fujii, Tomas Vondra
Discussion: https://postgr.es/m/c9e2a757cf3ac2333714eaf83a9cc184@postgrespro.ru
2023-12-05 22:53:12 +02:00
Peter Eisentraut 7e5f517799 Improve "user mapping not found" error message
Display the name of the foreign server for which the user mapping was
not found.

Author: Ian Lawrence Barwick <barwick@gmail.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/CAB8KJ=jFzNaeyFtLcTZNOc6fd1+F93pGVLFa-wyt31wn7VNxqQ@mail.gmail.com
2023-11-30 05:34:28 +01:00
Heikki Linnakangas 50c67c2019 Use ResourceOwner to track WaitEventSets.
A WaitEventSet holds file descriptors or event handles (on Windows).
If FreeWaitEventSet is not called, those fds or handles are leaked.
Use ResourceOwners to track WaitEventSets, to clean those up
automatically on error.

This was a live bug in async Append nodes, if a FDW's
ForeignAsyncRequest function failed. (In back branches, I will apply a
more localized fix for that based on PG_TRY-PG_FINALLY.)

The added test doesn't check for leaking resources, so it passed even
before this commit. But at least it covers the code path.

In the passing, fix misleading comment on what the 'nevents' argument
to WaitEventSetWait means.

Report by Alexander Lakhin, analysis and suggestion for the fix by
Tom Lane. Fixes bug #17828.

Reviewed-by: Alexander Lakhin, Thomas Munro
Discussion: https://www.postgresql.org/message-id/472235.1678387869@sss.pgh.pa.us
2023-11-23 13:31:36 +02:00
David Rowley b690e5facb Stabilize postgres_fdw tests on 32-bit machines
cac169d68 adjusted DEFAULT_FDW_TUPLE_COST and that seems to have caused
a test to become unstable on 32-bit machines.

4b14e1871 tried to fix this as originally the plan was flipping between
a Nested Loop and Hash Join.  That commit forced the Nested Loop, but
there's still flexibility to push or not push the sort to the remote
server and 32-bit seems to prefer to push and on 64-bit, the costs
prefer not to.

Here let's just turn off enable_sort to significantly encourage the sort
to take place on the remote server.

Reported-by: Michael Paquier, Richard Guo
Discussion: https://postgr.es/m/ZUM2IhA8X2lrG50K@paquier.xyz
2023-11-03 12:35:37 +13:00
David Rowley 4b14e18714 Attempt to stabilize postgres_fdw tests
cac169d68 adjusted DEFAULT_FDW_TUPLE_COST and that seems to have caused
a test to become unstable on 32-bit machines.  Try to make it stable
again.

Reported-by: Michael Paquier
Discussion: https://postgr.es/m/ZUM2IhA8X2lrG50K@paquier.xyz
2023-11-02 23:16:34 +13:00
David Rowley cac169d686 Increase DEFAULT_FDW_TUPLE_COST from 0.01 to 0.2
0.01 was unrealistically low as it's the same as the default
cpu_tuple_cost and 10x cheaper than the default parallel_tuple_cost.
It's hard to imagine a situation where fetching a tuple from a foreign
server would be cheaper than fetching one from a parallel worker.

After some experimentation on a loopback server, somewhere between 0.15
and 0.3 seems more realistic.  Here we split the difference and set it
to 0.2.

This will cause operations that reduce the number of tuples (e.g.
aggregation) to be more likely to take place on the foreign server.

Adjusting this causes some plan changes in the postgres_fdw regression
tests.  This is because penalizing each Path with the additional tuple
costs causes some dilution of the costs of the other operations being
charged for and results in various paths appearing to be closer to the
same costs such that add_path's STD_FUZZ_FACTOR is more likely to see two
paths as costing (fuzzily) the same.  This isn't ideal, but it shouldn't
be reason enough to use artificially low costs.

Discussion: https://postgr.es/m/CAApHDvopVjjfh5c1Ed2HRvDdfom2dEpMwwiu5-f1AnmYprJngA@mail.gmail.com
2023-11-02 14:30:15 +13:00
Peter Eisentraut 611806cd72 Add trailing commas to enum definitions
Since C99, there can be a trailing comma after the last value in an
enum definition.  A lot of new code has been introducing this style on
the fly.  Some new patches are now taking an inconsistent approach to
this.  Some add the last comma on the fly if they add a new last
value, some are trying to preserve the existing style in each place,
some are even dropping the last comma if there was one.  We could
nudge this all in a consistent direction if we just add the trailing
commas everywhere once.

I omitted a few places where there was a fixed "last" value that will
always stay last.  I also skipped the header files of libpq and ecpg,
in case people want to use those with older compilers.  There were
also a small number of cases where the enum type wasn't used anywhere
(but the enum values were), which ended up confusing pgindent a bit,
so I left those alone.

Discussion: https://www.postgresql.org/message-id/flat/386f8c45-c8ac-4681-8add-e3b0852c1620%40eisentraut.org
2023-10-26 09:20:54 +02:00
Michael Paquier d61f2538a3 postgres_fdw: Replace WAIT_EVENT_EXTENSION with custom wait events
Three custom wait events are added here:
- "PostgresFdwCleanupResult", waiting while cleaning up PQgetResult() on
transaction abort.
- "PostgresFdwConnect", waiting to establish a connection to a remote
server.
- "PostgresFdwGetResult", waiting to receive a result from a remote
server.

Author: Masahiro Ikeda
Discussion: https://postgr.es/m/197bce267fa691a0ac62c86c4ab904c4@oss.nttdata.com
2023-10-05 09:50:42 +09:00
Etsuro Fujita 05c821294e postgres_fdw: Fix test for parameterized foreign scan.
Commit e4106b252 should have updated this test, but did not; back-patch
to all supported branches.

Reviewed by Richard Guo.

Discussion: http://postgr.es/m/CAPmGK15nR0NXLSCKQAcqbZbTzrzd5MozowWnTnGfPkayndF43Q%40mail.gmail.com
2023-08-30 17:15:00 +09:00
Etsuro Fujita 89be0b89ae Fix code indentation vioaltion introduced in commit 9e9931d2b.
Per buildfarm member koel
2023-08-15 17:45:00 +09:00
Etsuro Fujita 9e9931d2bf Re-allow FDWs and custom scan providers to replace joins with pseudoconstant quals.
This was disabled in commit 6f80a8d9c due to the lack of support for
handling of pseudoconstant quals assigned to replaced joins in
createplan.c.  To re-allow it, this patch adds the support by 1)
modifying the ForeignPath and CustomPath structs so that if they
represent foreign and custom scans replacing a join with a scan, they
store the list of RestrictInfo nodes to apply to the join, as in
JoinPaths, and by 2) modifying create_scan_plan() in createplan.c so
that it uses that list in that case, instead of the baserestrictinfo
list, to get pseudoconstant quals assigned to the join, as mentioned in
the commit message for that commit.

Important item for the release notes: this is non-backwards-compatible
since it modifies the ForeignPath and CustomPath structs, as mentioned
above, and changes the argument lists for FDW helper functions
create_foreignscan_path(), create_foreign_join_path(), and
create_foreign_upper_path().

Richard Guo, with some additional changes by me, reviewed by Nishant
Sharma, Suraj Kharage, and Richard Guo.

Discussion: https://postgr.es/m/CADrsxdbcN1vejBaf8a%2BQhrZY5PXL-04mCd4GDu6qm6FigDZd6Q%40mail.gmail.com
2023-08-15 16:45:00 +09:00
Etsuro Fujita 6f80a8d9c1 Disallow replacing joins with scans in problematic cases.
Commit e7cb7ee14, which introduced the infrastructure for FDWs and
custom scan providers to replace joins with scans, failed to add support
handling of pseudoconstant quals assigned to replaced joins in
createplan.c, leading to an incorrect plan without a gating Result node
when postgres_fdw replaced a join with such a qual.

To fix, we could add the support by 1) modifying the ForeignPath and
CustomPath structs to store the list of RestrictInfo nodes to apply to
the join, as in JoinPaths, if they represent foreign and custom scans
replacing a join with a scan, and by 2) modifying create_scan_plan() in
createplan.c to use that list in that case, instead of the
baserestrictinfo list, to get pseudoconstant quals assigned to the join;
but #1 would cause an ABI break.  So fix by modifying the infrastructure
to just disallow replacing joins with such quals.

Back-patch to all supported branches.

Reported by Nishant Sharma.  Patch by me, reviewed by Nishant Sharma and
Richard Guo.

Discussion: https://postgr.es/m/CADrsxdbcN1vejBaf8a%2BQhrZY5PXL-04mCd4GDu6qm6FigDZd6Q%40mail.gmail.com
2023-07-28 15:45:00 +09:00
Tomas Vondra 087a933b21 Remove expensive test of postgres_fdw batch inserts
The test inserted 70k rows into a foreign table, in order to verify
correct behavior with more than 65535 parameters, and was added in
response to a bug report.

However, this is rather expensive, especially when running the tests
under valgrind, CLOBBER_CACHE_ALWAYS etc. It doesn't seem worth it to
keep running the test, so remove it from all branches (14+).

Backpatch-through: 14
Discussion: https://postgr.es/m/2131017.1623451468@sss.pgh.pa.us
2023-07-03 18:16:58 +02:00
Michael Paquier 2aeaf80e57 Refactor some code related to wait events "BufferPin" and "Extension"
The following changes are done:
- Addition of WaitEventBufferPin and WaitEventExtension, that hold a
list of wait events related to each category.
- Addition of two functions that encapsulate the list of wait events for
each category.
- Rename BUFFER_PIN to BUFFERPIN (only this wait event class used an
underscore, requiring a specific rule in the automation script).

These changes make a bit easier the automatic generation of all the code
and documentation related to wait events, as all the wait event
categories are now controlled by consistent structures and functions.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/c6f35117-4b20-4c78-1df5-d3056010dcf5@gmail.com
Discussion: https://postgr.es/m/77a86b3a-c4a8-5f5d-69b9-d70bbf2e9b98@gmail.com
2023-07-03 11:01:02 +09:00
Amit Langote 054ff3b33a Add a test case for a316a3bc
a316a3bc fixed the code in build_simpl_rel() that propagates
RelOptInfo.userid from parent to child rels so that it works
correctly for the child rels of a UNION ALL subquery rel, though
no tests were added in that commit.  So do so here.

As noted in the discussion, coming up with a test case in the core
regression suite for this fix has turned out to be tricky, so the
test case is added to the postgres_fdw's suite instead.
postgresGetForeignRelSize()'s use of user mapping for the user
specified in RelOptInfo.userid makes it relatively easier to craft
a test case around.

Discussion: https://postgr.es/m/CA%2BHiwqH91GaFNXcXbLAM9L%3DzBwUmSyv699Mtv3i1_xtk9Xec_A%40mail.gmail.com
Backpatch-through: 16
2023-06-30 15:51:34 +09:00
Tom Lane 991a3df227 Fix filtering of "cloned" outer-join quals some more.
We've had multiple issues with the clause_is_computable_at logic that
I introduced in 2489d76c4: it's been known to accept more than one
clone of the same qual at the same plan node, and also to accept no
clones at all.  It's looking impractical to get it 100% right on the
basis of the currently-stored information, so fix it by introducing a
new RestrictInfo field "incompatible_relids" that explicitly shows
which outer joins a given clone mustn't be pushed above.

In principle we could populate this field in every RestrictInfo, but
that would cost space and there doesn't presently seem to be a need
for it in general.  Also, while deconstruct_distribute_oj_quals can
easily fill the field with the remaining members of the commutative
join set that it's considering, computing it in the general case
seems again pretty complicated.  So for now, just fill it for
clone quals.

Along the way, fix a bug that may or may not be only latent:
equivclass.c was generating replacement clauses with is_pushed_down
and has_clone/is_clone markings that didn't match their
required_relids.  This led me to conclude that leaving the clone flags
out of make_restrictinfo's purview wasn't such a great idea after all,
so add them.

Per report from Richard Guo.

Discussion: https://postgr.es/m/CAMbWs48EYi_9-pSd0ORes1kTmTeAjT4Q3gu49hJtYCbSn2JyeA@mail.gmail.com
2023-05-25 10:28:33 -04:00
Tom Lane a2eb99a01e Expand some more uses of "deleg" to "delegation" or "delegated".
Complete the task begun in 9c0a0e2ed: we don't want to use the
abbreviation "deleg" for GSS delegation in any user-visible places.
(For consistency, this also changes most internal uses too.)

Abhijit Menon-Sen and Tom Lane

Discussion: https://postgr.es/m/949048.1684639317@sss.pgh.pa.us
2023-05-21 10:55:18 -04:00
Bruce Momjian 9c0a0e2ed9 rename "gss_accept_deleg" to "gss_accept_delegation".
This is more consistent with existing GUC spelling.

Discussion: https://postgr.es/m/ZGdnEsGtNj7+fZoa@momjian.us
2023-05-20 21:32:54 -04:00
Tom Lane 0245f8db36 Pre-beta mechanical code beautification.
Run pgindent, pgperltidy, and reformat-dat-files.

This set of diffs is a bit larger than typical.  We've updated to
pg_bsd_indent 2.1.2, which properly indents variable declarations that
have multi-line initialization expressions (the continuation lines are
now indented one tab stop).  We've also updated to perltidy version
20230309 and changed some of its settings, which reduces its desire to
add whitespace to lines to make assignments etc. line up.  Going
forward, that should make for fewer random-seeming changes to existing
code.

Discussion: https://postgr.es/m/20230428092545.qfb3y5wcu4cm75ur@alvherre.pgsql
2023-05-19 17:24:48 -04:00
Michael Paquier 806fad7573 Fix buffer refcount leak with FDW bulk inserts
The leak would show up when using batch inserts with foreign tables
included in a partition tree, as the slots used in the batch were not
reset once processed.  In order to fix this problem, some
ExecClearTuple() are added to clean up the slots used once a batch is
filled and processed, mapping with the number of slots currently in use
as tracked by the counter ri_NumSlots.

This buffer refcount leak has been introduced in b676ac4 with the
addition of the executor facility to improve bulk inserts for FDWs, so
backpatch down to 14.

Alexander has provided the patch (slightly modified by me).  The test
for postgres_fdw comes from me, based on the test case that the author
has sent in the report.

Author: Alexander Pyhalov
Discussion: https://postgr.es/m/b035780a740efd38dc30790c76927255@postgrespro.ru
Backpatch-through: 14
2023-04-25 09:42:19 +09:00
David Rowley b4dbf3e924 Fix various typos
This fixes many spelling mistakes in comments, but a few references to
invalid parameter names, function names and option names too in comments
and also some in string constants

Also, fix an #undef that was undefining the incorrect definition

Author: Alexander Lakhin
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com
2023-04-18 13:23:23 +12:00
Stephen Frost 6633cfb216 De-Revert "Add support for Kerberos credential delegation"
This reverts commit 3d03b24c3 (Revert Add support for Kerberos
credential delegation) which was committed on the grounds of concern
about portability, but on further review and discussion, it's clear that
we are better off explicitly requiring MIT Kerberos as that appears to
be the only GSSAPI library currently that's under proper maintenance
and ongoing development.  The API used for storing credentials was added
to MIT Kerberos over a decade ago while for the other libraries which
appear to be mainly based on Heimdal, which exists explicitly to be a
re-implementation of MIT Kerberos, the API never made it to a released
version (even though it was added to the Heimdal git repo over 5 years
ago..).

This post-feature-freeze change was approved by the RMT.

Discussion: https://postgr.es/m/ZDDO6jaESKaBgej0%40tamriel.snowman.net
2023-04-13 08:55:07 -04:00
Stephen Frost 3d03b24c35 Revert "Add support for Kerberos credential delegation"
This reverts commit 3d4fa227bc.

Per discussion and buildfarm, this depends on APIs that seem to not
be available on at least one platform (NetBSD).  Should be certainly
possible to rework to be optional on that platform if necessary but bit
late for that at this point.

Discussion: https://postgr.es/m/3286097.1680922218@sss.pgh.pa.us
2023-04-08 07:21:35 -04:00
Stephen Frost 3d4fa227bc Add support for Kerberos credential delegation
Support GSSAPI/Kerberos credentials being delegated to the server by a
client.  With this, a user authenticating to PostgreSQL using Kerberos
(GSSAPI) credentials can choose to delegate their credentials to the
PostgreSQL server (which can choose to accept them, or not), allowing
the server to then use those delegated credentials to connect to
another service, such as with postgres_fdw or dblink or theoretically
any other service which is able to be authenticated using Kerberos.

Both postgres_fdw and dblink are changed to allow non-superuser
password-less connections but only when GSSAPI credentials have been
delegated to the server by the client and GSSAPI is used to
authenticate to the remote system.

Authors: Stephen Frost, Peifeng Qiu
Reviewed-By: David Christensen
Discussion: https://postgr.es/m/CO1PR05MB8023CC2CB575E0FAAD7DF4F8A8E29@CO1PR05MB8023.namprd05.prod.outlook.com
2023-04-07 21:58:04 -04:00
Etsuro Fujita 983ec23007 postgres_fdw: Add support for parallel abort.
postgres_fdw aborts remote (sub)transactions opened on remote server(s)
in a local (sub)transaction one by one when the local (sub)transaction
aborts.  This patch allows it to abort the remote (sub)transactions in
parallel to improve performance.  This is enabled by the server option
"parallel_abort".  The default is false.

Etsuro Fujita, reviewed by David Zhang.

Discussion: http://postgr.es/m/CAPmGK15FuPVGx3TGHKShsbPKKtF1y58-ZLcKoxfN-nqLj1dZ%3Dg%40mail.gmail.com
2023-04-06 17:30:00 +09:00
Daniel Gustafsson fb6fad6ef1 Fix function reference in comment
Commit a61b1f748 renamed ExecCheckRTEPerms to ExecCheckPermissions
as part of a larger body of work, but missed this comment.  Fix by
updating the referenced function name to make the comment the same
as other occurrences.

Author: Koshi Shibagaki <shibagaki.koshi@fujitsu.com>
Discussion: https://postgr.es/m/OS3PR01MB653359ACBE8DBBE29EE2BC71FA909@OS3PR01MB6533.jpnprd01.prod.outlook.com
2023-04-05 09:06:32 +02:00
Etsuro Fujita 39a3bdc9eb postgres_fdw: Remove useless if-test in GetConnection().
Checking whether entry->conn is NULL after doing disconnect_pg_server()
for that entry is pointless, as that function ensures that it is NULL.

Thinko in commit 7fc1a81e4; this would be harmless, so patch HEAD only.

Reviewed-by: Richard Guo and Daniel Gustafsson
Discussion: https://postgr.es/m/CAPmGK169vQ83PQwQkoxO-AK2EeK1EsgsxixedM%2BBLWEAhZ_AqQ%40mail.gmail.com
2023-03-17 18:15:00 +09:00
Tom Lane 71a75626d5 Drop test view when done with it.
The view just added by commit 53fe7e6cb decompiles differently
in v15 than HEAD (presumably as a consequence of 47bb9db75).
That causes failures in cross-version upgrade testing.

We could teach AdjustUpgrade.pm to compensate for that, but it
seems less painful to just drop the view after we're done with it.

Per buildfarm.
2023-02-27 20:27:48 -05:00
Tom Lane 53fe7e6cb8 Harden postgres_fdw tests against unexpected cache flushes.
postgres_fdw will close its remote session if an sinval cache reset
occurs, since it's possible that that means some FDW parameters
changed.  We had two tests that were trying to ensure that the
session remains alive by setting debug_discard_caches = 0; but
that's not sufficient.  Even though the tests seem stable enough
in the buildfarm, they flap a lot under CI.

In the first test, which is checking the ability to recover from
a lost connection, we can stabilize the results by just not
caring whether pg_terminate_backend() finds a victim backend.
If a reset did happen, there won't be a session to terminate
anymore, but the test can proceed anyway.  (Arguably, we are
then not testing the unintentional-disconnect case, but as long
as that scenario is exercised in most runs I think it's fine;
testing the reset-driven case is of value too.)

In the second test, which is trying to verify the application_name
displayed in pg_stat_activity by a remote session, we had a race
condition in that the remote session might go away before we can
fetch its pg_stat_activity entry.  We can close that race and make
the test more certainly test what it intends to by arranging things
so that the remote session itself fetches its pg_stat_activity entry
(based on PID rather than a somewhat-circular assumption about the
application name).

Both tests now demonstrably pass under debug_discard_caches = 1,
so we can remove that hack.

Back-patch into relevant back branches.

Discussion: https://postgr.es/m/20230226194340.u44bkfgyz64c67i6@awork3.anarazel.de
2023-02-27 16:29:51 -05:00
Peter Eisentraut 62d56f6720 Fix comment indentation and whitespace
The previous layout satisfied pgindent but failed the git whitespace
check.  Fix by not putting the comment first in the line, which
pgindent does not handle well.

Discussion: https://www.postgresql.org/message-id/flat/480e3c67-b703-46ff-a418-d3b481d68372%40enterprisedb.com
2023-02-24 14:54:41 +01:00
Michael Paquier 8427ce4c37 Fix handling of escape sequences in postgres_fdw.application_name
postgres_fdw.application_name relies on MyProcPort to define the data
that should be added to escape sequences %u (user name) or %d (database
name).  However this code could be run in processes that lack a
MyProcPort, like an autovacuum process, causing crashes.

The code generating the application name is made more flexible with this
commit, so as it now generates no data for %u and %d if MyProcPort is
missing, and a simple "unknown" if MyProcPort exists, but the expected
fields are not set.

Reported-by: Alexander Lakhin
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: Hayato Kuroda, Masahiko Sawada
Discussion: https://postgr.es/m/17789-8b31c5a4672b74d9@postgresql.org
Backpatch-through: 15
2023-02-21 20:01:43 +09:00
Michael Paquier ef7002dbe0 Fix various typos in code and tests
Most of these are recent, and the documentation portions are new as of
v16 so there is no need for a backpatch.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20230208155644.GM1653@telsasoft.com
2023-02-09 14:43:53 +09:00
Peter Eisentraut 54a177a948 Remove useless casts to (void *) in hash_search() calls
Some of these appear to be leftovers from when hash_search() took a
char * argument (changed in 5999e78fc4).

Since after this there is some more horizontal space available, do
some light reformatting where suitable.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/fd9adf5d-b1aa-e82f-e4c7-263c30145807%40enterprisedb.com
2023-02-06 09:41:01 +01:00
Tom Lane 3bef56e116 Invent "join domains" to replace the below_outer_join hack.
EquivalenceClasses are now understood as applying within a "join
domain", which is a set of inner-joined relations (possibly underneath
an outer join).  We no longer need to treat an EC from below an outer
join as a second-class citizen.

I have hopes of eventually being able to treat outer-join clauses via
EquivalenceClasses, by means of only applying deductions within the
EC's join domain.  There are still problems in the way of that, though,
so for now the reconsider_outer_join_clause logic is still here.

I haven't been able to get rid of RestrictInfo.is_pushed_down either,
but I wonder if that could be recast using JoinDomains.

I had to hack one test case in postgres_fdw.sql to make it still test
what it was meant to, because postgres_fdw is inconsistent about
how it deals with quals containing non-shippable expressions; see
https://postgr.es/m/1691374.1671659838@sss.pgh.pa.us.  That should
be improved, but I don't think it's within the scope of this patch
series.

Patch by me; thanks to Richard Guo for review.

Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
2023-01-30 13:50:25 -05:00