Commit Graph

66 Commits

Author SHA1 Message Date
Robert Haas 4bbf6edfbd postgres_fdw: Avoid 'outer pathkeys do not match mergeclauses' error.
When pushing down a join to a foreign server, postgres_fdw constructs
an alternative plan to be used for any EvalPlanQual rechecks that
prove to be necessary.  This plan is stored as the outer subplan of
the Foreign Scan implementing the pushed-down join.  Previously, this
alternative plan could have a different nominal sort ordering than its
parent, which seemed OK since there will only be one tuple per base
table anyway in the case of an EvalPlanQual recheck.  Actually,
though, it caused a problem if that path was used as a building block
for the EvalPlanQual recheck plan of a higher-level foreign join,
because we could end up with a merge join one of whose inputs was not
labelled with the correct sort order.  Repair by injecting an extra
Sort node into the EvalPlanQual recheck plan whenever it would
otherwise fail to be sorted at least as well as its parent Foreign
Scan.

Report by Jeff Janes.  Patch by me, reviewed by Tom Lane, who also
provided the test case and comment text.

Discussion: http://postgr.es/m/CAMkU=1y2G8VOVBHv3iXU2TMAj7-RyBFFW1uhkr5sm9LQ2=X35g@mail.gmail.com
2018-01-17 16:18:39 -05:00
Tom Lane e9f2703ab7 Fix postgres_fdw to cope with duplicate GROUP BY entries.
Commit 7012b132d, which added the ability to push down aggregates and
grouping to the remote server, wasn't careful to ensure that the remote
server would have the same idea we do about which columns are the grouping
columns, in cases where there are textually identical GROUP BY expressions.
Such cases typically led to "targetlist item has multiple sortgroupref
labels" errors.

To fix this reliably, switch over to using "GROUP BY column-number" syntax
rather than "GROUP BY expression" in transmitted queries, and adjust
foreign_grouping_ok() to be more careful about duplicating the sortgroupref
labeling of the local pathtarget.

Per bug #14890 from Sean Johnston.  Back-patch to v10 where the buggy code
was introduced.

Jeevan Chalke, reviewed by Ashutosh Bapat

Discussion: https://postgr.es/m/20171107134948.1508.94783@wrigleys.postgresql.org
2018-01-12 16:52:49 -05:00
Robert Haas 82c5c533d1 postgres_fdw: Fix failing regression test.
Commit ab3f008a2d broke this.

Report by Stephen Frost.

Discussion: http://postgr.es/m/20171205180342.GO4628@tamriel.snowman.net
2017-12-05 13:12:00 -05:00
Robert Haas 9502227805 postgres_fdw: Fix test that didn't test what it claimed.
Antonin Houska reported that the planner does consider pushing
postgres_fdw_abs() to the remote side, which happens because we make
it shippable earlier in the test case file.

Jeevan Chalke provided this patch, which changes the join
condition to use random(), which is not shippable, instead.
Antonin reviewed the patch.

Discussion: http://postgr.es/m/15265.1511985971@localhost
2017-12-01 13:49:11 -05:00
Tom Lane 9a785ad573 Fix creation of resjunk tlist entries for inherited mixed UPDATE/DELETE.
rewriteTargetListUD's processing is dependent on the relkind of the query's
target table.  That was fine at the time it was made to act that way, even
for queries on inheritance trees, because all tables in an inheritance tree
would necessarily be plain tables.  However, the 9.5 feature addition
allowing some members of an inheritance tree to be foreign tables broke the
assumption that rewriteTargetListUD's output tlist could be applied to all
child tables with nothing more than column-number mapping.  This led to
visible failures if foreign child tables had row-level triggers, and would
also break in cases where child tables belonged to FDWs that used methods
other than CTID for row identification.

To fix, delay running rewriteTargetListUD until after the planner has
expanded inheritance, so that it is applied separately to the (already
mapped) tlist for each child table.  We can conveniently call it from
preprocess_targetlist.  Refactor associated code slightly to avoid the
need to heap_open the target relation multiple times during
preprocess_targetlist.  (The APIs remain a bit ugly, particularly around
the point of which steps scribble on parse->targetList and which don't.
But avoiding such scribbling would require a change in FDW callback APIs,
which is more pain than it's worth.)

Also fix ExecModifyTable to ensure that "tupleid" is reset to NULL when
we transition from rows providing a CTID to rows that don't.  (That's
really an independent bug, but it manifests in much the same cases.)

Add a regression test checking one manifestation of this problem, which
was that row-level triggers on a foreign child table did not work right.

Back-patch to 9.5 where the problem was introduced.

Etsuro Fujita, reviewed by Ildus Kurbangaliev and Ashutosh Bapat

Discussion: https://postgr.es/m/20170514150525.0346ba72@postgrespro.ru
2017-11-27 17:54:07 -05:00
Robert Haas f49842d1ee Basic partition-wise join functionality.
Instead of joining two partitioned tables in their entirety we can, if
it is an equi-join on the partition keys, join the matching partitions
individually.  This involves teaching the planner about "other join"
rels, which are related to regular join rels in the same way that
other member rels are related to baserels.  This can use significantly
more CPU time and memory than regular join planning, because there may
now be a set of "other" rels not only for every base relation but also
for every join relation.  In most practical cases, this probably
shouldn't be a problem, because (1) it's probably unusual to join many
tables each with many partitions using the partition keys for all
joins and (2) if you do that scenario then you probably have a big
enough machine to handle the increased memory cost of planning and (3)
the resulting plan is highly likely to be better, so what you spend in
planning you'll make up on the execution side.  All the same, for now,
turn this feature off by default.

Currently, we can only perform joins between two tables whose
partitioning schemes are absolutely identical.  It would be nice to
cope with other scenarios, such as extra partitions on one side or the
other with no match on the other side, but that will have to wait for
a future patch.

Ashutosh Bapat, reviewed and tested by Rajkumar Raghuwanshi, Amit
Langote, Rafia Sabih, Thomas Munro, Dilip Kumar, Antonin Houska, Amit
Khandekar, and by me.  A few final adjustments by me.

Discussion: http://postgr.es/m/CAFjFpRfQ8GrQvzp3jA2wnLqrHmaXna-urjm_UY9BqXj=EaDTSA@mail.gmail.com
Discussion: http://postgr.es/m/CAFjFpRcitjfrULr5jfuKWRPsGUX0LQ0k8-yG0Qw2+1LBGNpMdw@mail.gmail.com
2017-10-06 11:11:10 -04:00
Robert Haas 7086be6e36 When WCOs are present, disable direct foreign table modification.
If the user modifies a view that has CHECK OPTIONs and this gets
translated into a modification to an underlying relation which happens
to be a foreign table, the check options should be enforced.  In the
normal code path, that was happening properly, but it was not working
properly for "direct" modification because the whole operation gets
pushed to the remote side in that case and we never have an option to
enforce the constraint against individual tuples.  Fix by disabling
direct modification when there is a need to enforce CHECK OPTIONs.

Etsuro Fujita, reviewed by Kyotaro Horiguchi and by me.

Discussion: http://postgr.es/m/f8a48f54-6f02-9c8a-5250-9791603171ee@lab.ntt.co.jp
2017-07-24 15:57:24 -04:00
Tom Lane 88f48b57fd Stabilize postgres_fdw regression tests.
The new test cases added in commit 8bf58c0d9 turn out to have output
that can vary depending on the lc_messages setting prevailing on the
test server.  Hide the remote end's error messages to ensure stable
output.  This isn't a terribly desirable solution; we'd rather know
that the connection failed for the expected reason and not some other
one.  But there seems little choice for the moment.

Per buildfarm.

Discussion: https://postgr.es/m/18419.1500658570@sss.pgh.pa.us
2017-07-21 14:20:43 -04:00
Tom Lane 8bf58c0d9b Re-establish postgres_fdw connections after server or user mapping changes.
Previously, postgres_fdw would keep on using an existing connection even
if the user did ALTER SERVER or ALTER USER MAPPING commands that should
affect connection parameters.  Teach it to watch for catcache invals
on these catalogs and re-establish connections when the relevant catalog
entries change.  Per bug #14738 from Michal Lis.

In passing, clean up some rather crufty decisions in commit ae9bfc5d6
about where fields of ConnCacheEntry should be reset.  We now reset
all the fields whenever we open a new connection.

Kyotaro Horiguchi, reviewed by Ashutosh Bapat and myself.
Back-patch to 9.3 where postgres_fdw appeared.

Discussion: https://postgr.es/m/20170710113917.7727.10247@wrigleys.postgresql.org
2017-07-21 12:51:38 -04:00
Peter Eisentraut 332bec1e60 postgres_fdw: Fix join push down with extensions
Objects in an extension are shippable to a foreign server if the
extension is part of the foreign server definition's shippable
extensions list.  But this was not properly considered in some cases
when checking whether a join condition can be pushed to a foreign server
and the join condition uses an object from a shippable extension.  So
the join would never be pushed down in those cases.

So, the list of extensions needs to be made available in fpinfo of the
relation being considered to be pushed down before any expressions are
assessed for being shippable.  Fix foreign_join_ok() to do that for a
join relation.

The code to save FDW options in fpinfo is scattered at multiple places.
Bring all of that together into functions apply_server_options(),
apply_table_options(), and merge_fdw_options().

David Rowley and Ashutosh Bapat, per report from David Rowley
2017-04-24 22:50:07 -04:00
Peter Eisentraut afd79873a0 Capitalize names of PLs consistently
Author: Daniel Gustafsson <daniel@yesql.se>
2017-04-05 00:38:25 -04:00
Robert Haas f49bcd4ef3 postgres_fdw: Teach IMPORT FOREIGN SCHEMA about partitioning.
Don't import partitions.  Do import partitioned tables which are
not themselves partitions.

Report by Stephen Frost.  Design and patch by Michael Paquier,
reviewed by Amit Langote.  Documentation revised by me.

Discussion: http://postgr.es/m/20170309141531.GD9812@tamriel.snowman.net
2017-03-31 15:06:34 -04:00
Robert Haas b30fb56b07 postgres_fdw: Push down FULL JOINs with restriction clauses.
The previous deparsing logic wasn't smart enough to produce subqueries
when deparsing; make it smart enough to do that.  However, we only do
it that way when necessary, because it generates more complicated SQL
which will be harder for any humans reading the queries to understand.

Etsuro Fujita, reviewed by Ashutosh Bapat

Discussion: http://postgr.es/m/c449261a-b033-dc02-9254-2fe5b7044795@lab.ntt.co.jp
2017-03-16 13:34:59 -04:00
Heikki Linnakangas 181bdb90ba Fix typos in comments.
Backpatch to all supported versions, where applicable, to make backpatching
of future fixes go more smoothly.

Josh Soref

Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com
2017-02-06 11:33:58 +02:00
Tom Lane aa7f593b1f Improve speed of contrib/postgres_fdw regression tests.
Commit 7012b132d added some tests that consumed an excessive amount of
time, more than tripling the time needed for "make installcheck" for this
module.  Add filter conditions to reduce the number of rows scanned,
bringing the runtime down to within hailing distance of what it was before.

Jeevan Chalke and Ashutosh Bapat, per a gripe from me

Discussion: https://postgr.es/m/16565.1478104765@sss.pgh.pa.us
2017-01-25 08:31:31 -05:00
Tom Lane c52d37c8b3 Invalidate cached plans on FDW option changes.
This fixes problems where a plan must change but fails to do so,
as seen in a bug report from Rajkumar Raghuwanshi.

For ALTER FOREIGN TABLE OPTIONS, do this through the standard method of
forcing a relcache flush on the table.  For ALTER FOREIGN DATA WRAPPER
and ALTER SERVER, just flush the whole plan cache on any change in
pg_foreign_data_wrapper or pg_foreign_server.  That matches the way
we handle some other low-probability cases such as opclass changes, and
it's unclear that the case arises often enough to be worth working harder.
Besides, that gives a patch that is simple enough to back-patch with
confidence.

Back-patch to 9.3.  In principle we could apply the code change to 9.2 as
well, but (a) we lack postgres_fdw to test it with, (b) it's doubtful that
anyone is doing anything exciting enough with FDWs that far back to need
this desperately, and (c) the patch doesn't apply cleanly.

Patch originally by Amit Langote, reviewed by Etsuro Fujita and Ashutosh
Bapat, who each contributed substantial changes as well.

Discussion: https://postgr.es/m/CAKcux6m5cA6rRPTKkqVdJ-R=KKDfe35Q_ZuUqxDSV_4hwga=og@mail.gmail.com
2017-01-06 14:12:52 -05:00
Robert Haas f5d6bce63c postgres_fdw: Try again to stabilize aggregate pushdown regression tests.
A query that only aggregates one row isn't a great argument for pushdown,
and buildfarm member brolga decides against it.  Adjust the query a bit
in the hopes of getting remote aggregation to win consistently.

Jeevan Chalke, per suggestion from Tom Lane
2016-10-24 22:36:24 -04:00
Robert Haas ad13a09d76 postgres_fdw: Attempt to stabilize regression results.
Set enable_hashagg to false for tests involving least_agg(), so that
we get the same plan regardless of local costing variances.  Also,
remove a test involving sqrt(); it's there to test deparsing of
HAVING clauses containing expressions, but that's tested elsewhere
anyway, and sqrt(2) deparses with different amounts of precision on
different machines.

Per buildfarm.
2016-10-21 11:29:33 -04:00
Robert Haas 7012b132d0 postgres_fdw: Push down aggregates to remote servers.
Now that the upper planner uses paths, and now that we have proper hooks
to inject paths into the upper planning process, it's possible for
foreign data wrappers to arrange to push aggregates to the remote side
instead of fetching all of the rows and aggregating them locally.  This
figures to be a massive win for performance, so teach postgres_fdw to
do it.

Jeevan Chalke and Ashutosh Bapat.  Reviewed by Ashutosh Bapat with
additional testing by Prabhat Sahu.  Various mostly cosmetic changes
by me.
2016-10-21 09:54:29 -04:00
Heikki Linnakangas ae025a1598 Support OID system column in postgres_fdw.
You can use ALTER FOREIGN TABLE SET WITH OIDS on a foreign table, but the
oid column read out as zeros, because the postgres_fdw didn't know about
it. Teach postgres_fdw how to fetch it.

Etsuro Fujita, with an additional test case by me.

Discussion: <56E90A76.5000503@lab.ntt.co.jp>
2016-08-26 16:33:57 +03:00
Tom Lane 45639a0525 Avoid invalidating all foreign-join cached plans when user mappings change.
We must not push down a foreign join when the foreign tables involved
should be accessed under different user mappings.  Previously we tried
to enforce that rule literally during planning, but that meant that the
resulting plans were dependent on the current contents of the
pg_user_mapping catalog, and we had to blow away all cached plans
containing any remote join when anything at all changed in pg_user_mapping.
This could have been improved somewhat, but the fact that a syscache inval
callback has very limited info about what changed made it hard to do better
within that design.  Instead, let's change the planner to not consider user
mappings per se, but to allow a foreign join if both RTEs have the same
checkAsUser value.  If they do, then they necessarily will use the same
user mapping at runtime, and we don't need to know specifically which one
that is.  Post-plan-time changes in pg_user_mapping no longer require any
plan invalidation.

This rule does give up some optimization ability, to wit where two foreign
table references come from views with different owners or one's from a view
and one's directly in the query, but nonetheless the same user mapping
would have applied.  We'll sacrifice the first case, but to not regress
more than we have to in the second case, allow a foreign join involving
both zero and nonzero checkAsUser values if the nonzero one is the same as
the prevailing effective userID.  In that case, mark the plan as only
runnable by that userID.

The plancache code already had a notion of plans being userID-specific,
in order to support RLS.  It was a little confused though, in particular
lacking clarity of thought as to whether it was the rewritten query or just
the finished plan that's dependent on the userID.  Rearrange that code so
that it's clearer what depends on which, and so that the same logic applies
to both RLS-injected role dependency and foreign-join-injected role
dependency.

Note that this patch doesn't remove the other issue mentioned in the
original complaint, which is that while we'll reliably stop using a foreign
join if it's disallowed in a new context, we might fail to start using a
foreign join if it's now allowed, but we previously created a generic
cached plan that didn't use one.  It was agreed that the chance of winning
that way was not high enough to justify the much larger number of plan
invalidations that would have to occur if we tried to cause it to happen.

In passing, clean up randomly-varying spelling of EXPLAIN commands in
postgres_fdw.sql, and fix a COSTS ON example that had been allowed to
leak into the committed tests.

This reverts most of commits fbe5a3fb7 and 5d4171d1c, which were the
previous attempt at ensuring we wouldn't push down foreign joins that
span permissions contexts.

Etsuro Fujita and Tom Lane

Discussion: <d49c1e5b-f059-20f4-c132-e9752ee0113e@lab.ntt.co.jp>
2016-07-15 17:23:02 -04:00
Robert Haas 86437ddf8c postgres_fdw: Fix cache lookup failure while creating error context.
This is fallout from join pushdown; get_relid_attribute_name can't
handle an attribute number of 0, indicating a whole-row reference,
and shouldn't be called in that case.

Etsuro Fujita, reviewed by Ashutosh Bapat
2016-07-01 11:29:25 -04:00
Robert Haas 9e9c38e159 postgres_fdw: Fix incorrect NULL handling in join pushdown.
something.* IS NOT NULL means that every attribute of the row is not
NULL, not that the row itself is non-NULL (e.g. because it's coming
from below an outer join.  Use (somevar.*)::pg_catalog.text IS NOT
NULL instead.

Ashutosh Bapat, per a report by Rushabh Lathia.  Reviewed by
Amit Langote and Etsuro Fujita.  Schema-qualification added by me.
2016-06-24 15:14:15 -04:00
Robert Haas 131c7e70b4 postgres_fdw: Check PlaceHolderVars before pushing down a join.
As discovered by Andreas Seltenreich via sqlsmith, it's possible for a
remote join to need to generate a target list which contains a
PlaceHolderVar which would need to be evaluated on the remote server.
This happens when we try to push down a join tree which contains outer
joins and the nullable side of the join contains a subquery which
evauates some expression which can go to NULL above the level of the
join.  Since the deparsing logic can't build a remote query that
involves subqueries, it fails while trying to produce an SQL query
that can be sent to the remote side.  Detect such cases and don't try
to push down the join at all.

It's actually fine to push down the join if the PlaceHolderVar needs
to be evaluated at the current join level.  This patch makes a small
change to build_tlist_to_deparse so that this case will work.

Amit Langote, Ashutosh Bapat, and me.
2016-06-14 11:48:27 -04:00
Robert Haas 02a568a027 postgres_fdw: Fix the fix for crash when pushing down multiple joins.
Commit 3151f16e18 was intended to be
a commit of a patch from Ashutosh Bapat, but instead I mistakenly
committed an earlier version from Michael Paquier (because both
patches were submitted with the same filename, and I confused them).
Michael's patch fixes the crash but doesn't actually implement the
correct test.

Repair the incorrect logic, and also expand the comments considerably
so that this is all more clear.

Ashutosh Bapat and Robert Haas
2016-05-16 11:28:28 -04:00
Robert Haas 5b1f9ce1d9 postgres_fdw: Don't push down certain full joins.
If there's a filter condition on either side of a full outer join,
it is neither correct to attach it to the join's ON clause nor to
throw it into the toplevel WHERE clause.  Just don't push down the
join in that case.

To maximize the number of cases where we can still push down full
joins, push inner join conditions into the ON clause at the first
opportunity rather than postponing them to the top-level WHERE
clause.  This produces nicer SQL, anyway.

This bug was introduced in e4106b2528.

Ashutosh Bapat, per report from Rajkumar Raghuwanshi.
2016-04-20 23:54:19 -04:00
Robert Haas 5d4171d1c7 Don't require a user mapping for FDWs to work.
Commit fbe5a3fb73 accidentally changed
this behavior; put things back the way they were, and add some
regression tests.

Report by Andres Freund; patch by Ashutosh Bapat, with a bit of
kibitzing by me.
2016-03-28 21:50:28 -04:00
Robert Haas 3151f16e18 postgres_fdw: Fix crash when pushing down multiple joins.
A join clause might mention multiple relations on either side, so it
need not be the case that a given joinrel's constituent relations are
all on one side of the join clause or all on the other.

Report by Rajkumar Raghuwanshi.  Analysis and fix by Michael Paquier
and Ashutosh Bapat.
2016-03-23 12:28:01 -04:00
Robert Haas 0bf3ae88af Directly modify foreign tables.
postgres_fdw can now sent an UPDATE or DELETE statement directly to
the foreign server in simple cases, rather than sending a SELECT FOR
UPDATE statement and then updating or deleting rows one-by-one.

Etsuro Fujita, reviewed by Rushabh Lathia, Shigeru Hanada, Kyotaro
Horiguchi, Albe Laurenz, Thom Brown, and me.
2016-03-18 13:55:52 -04:00
Robert Haas aa09cd242f postgres_fdw: Consider foreign joining and foreign sorting together.
Commit ccd8f97922 gave us the ability to
request that the remote side sort the data, and, later, commit
e4106b2528 gave us the ability to
request that the remote side perform the join for us rather than doing
it locally.  But we could not do both things at the same time: a
remote SQL query that had an ORDER BY clause would never be a join.
This commit adds that capability.

Ashutosh Bapat, reviewed by me.
2016-03-09 10:51:49 -05:00
Robert Haas 3bea3f88d5 postgres_fdw: When sending ORDER BY, always include NULLS FIRST/LAST.
Previously, we included NULLS FIRST when appropriate but relied on the
default behavior to be NULLS LAST.  This is, however, not true for a
sort in descending order and seems like a fragile assumption anyway.

Report by Rajkumar Raghuwanshi.  Patch by Ashutosh Bapat.  Review
comments from Michael Paquier and Tom Lane.
2016-03-04 11:37:42 -05:00
Robert Haas bb4df42e6a postgres_fdw: Remove unstable regression test.
Per Tom Lane and the buildfarm.
2016-02-09 15:42:20 -05:00
Robert Haas e4106b2528 postgres_fdw: Push down joins to remote servers.
If we've got a relatively straightforward join between two tables,
this pushes that join down to the remote server instead of fetching
the rows for each table and performing the join locally.  Some cases
are not handled yet, such as SEMI and ANTI joins.  Also, we don't
yet attempt to create presorted join paths or parameterized join
paths even though these options do get tried for a base relation
scan.  Nevertheless, this seems likely to be a very significant win
in many practical cases.

Shigeru Hanada and Ashutosh Bapat, reviewed by Robert Haas, with
additional review at various points by Tom Lane, Etsuro Fujita,
KaiGai Kohei, and Jeevan Chalke.
2016-02-09 14:00:50 -05:00
Robert Haas 37c84570b1 postgres_fdw: Avoid possible misbehavior when RETURNING tableoid column only.
deparseReturningList ended up adding up RETURNING NULL to the code, but
code elsewhere saw an empty list of attributes and concluded that it
should not expect tuples from the remote side.

Etsuro Fujita and Robert Haas, reviewed by Thom Brown
2016-02-04 22:27:13 -05:00
Robert Haas dc203dc3ac postgres_fdw: Allow fetch_size to be set per-table or per-server.
The default fetch size of 100 rows might not be right in every
environment, so allow users to configure it.

Corey Huinker, reviewed by Kyotaro Horiguchi, Andres Freund, and me.
2016-02-03 09:07:35 -05:00
Tom Lane 7e22470471 Fix incorrect pattern-match processing in psql's \det command.
listForeignTables' invocation of processSQLNamePattern did not match up
with the other ones that handle potentially-schema-qualified names; it
failed to make use of pg_table_is_visible() and also passed the name
arguments in the wrong order.  Bug seems to have been aboriginal in commit
0d692a0dc9.  It accidentally sort of worked as long as you didn't
inquire too closely into the behavior, although the silliness was later
exposed by inconsistencies in the test queries added by 59efda3e50
(which I probably should have questioned at the time, but didn't).

Per bug #13899 from Reece Hart.  Patch by Reece Hart and Tom Lane.
Back-patch to all affected branches.
2016-01-29 10:28:02 +01:00
Robert Haas ccd8f97922 postgres_fdw: Consider requesting sorted data so we can do a merge join.
When use_remote_estimate is enabled, consider adding ORDER BY to the
query we sending to the remote server so that we can use that ordered
data for a merge join.  Commit f18c944b61
arranges to push down the query pathkeys, which seems like the case
mostly likely to be a win, but testing shows this can sometimes win,
too.

For a regular table, we know which indexes are present and therefore
test whether the ordering provided by each such index is useful.  Here,
we take the opposite approach: guess what orderings would be useful if
they could be generated cheaply, and then ask the remote side what those
will cost.

Ashutosh Bapat, with very substantial cosmetic revisions by me.  Also
reviewed by Rushabh Lathia.
2015-12-22 13:46:40 -05:00
Tom Lane b9f117d6cd Add regression tests for remote execution of extension operators/functions.
Rather than relying on other extensions to be available for installation,
let's just add some test objects to the postgres_fdw extension itself
within the regression script.
2015-11-04 12:03:30 -05:00
Robert Haas f18c944b61 postgres_fdw: Add ORDER BY to some remote SQL queries.
If the join problem's entire ORDER BY clause can be pushed to the
remote server, consider a path that adds this ORDER BY clause.  If
use_remote_estimate is on, we cost this path using an additional
remote EXPLAIN.  If not, we just estimate that the path costs 20%
more, which is intended to be large enough that we won't request a
remote sort when it's not helpful, but small enough that we'll have
the remote side do the sort when in doubt.  In some cases, the remote
sort might actually be free, because the remote query plan might
happen to produce output that is ordered the way we need, but without
remote estimates we have no way of knowing that.

It might also be useful to request sorted output from the remote side
if it enables an efficient merge join, but this patch doesn't attempt
to handle that case.

Ashutosh Bapat with revisions by me.  Also reviewed by Fabrízio de Royes
Mello and Jeevan Chalke.
2015-11-03 13:04:42 -05:00
Tom Lane 76f965ff1f Improve handling of collations in contrib/postgres_fdw.
If we have a local Var of say varchar type with default collation, and
we apply a RelabelType to convert that to text with default collation, we
don't want to consider that as creating an FDW_COLLATE_UNSAFE situation.
It should be okay to compare that to a remote Var, so long as the remote
Var determines the comparison collation.  (When we actually ship such an
expression to the remote side, the local Var would become a Param with
default collation, meaning the remote Var would in fact control the
comparison collation, because non-default implicit collation overrides
default implicit collation in parse_collate.c.)  To fix, be more precise
about what FDW_COLLATE_NONE means: it applies either to a noncollatable
data type or to a collatable type with default collation, if that collation
can't be traced to a remote Var.  (When it can, FDW_COLLATE_SAFE is
appropriate.)  We were essentially using that interpretation already at
the Var/Const/Param level, but we weren't bubbling it up properly.

An alternative fix would be to introduce a separate FDW_COLLATE_DEFAULT
value to describe the second situation, but that would add more code
without changing the actual behavior, so it didn't seem worthwhile.

Also, since we're clarifying the rule to be that we care about whether
operator/function input collations match, there seems no need to fail
immediately upon seeing a Const/Param/non-foreign-Var with nondefault
collation.  We only have to reject if it appears in a collation-sensitive
context (for example, "var IS NOT NULL" is perfectly safe from a collation
standpoint, whatever collation the var has).  So just set the state to
UNSAFE rather than failing immediately.

Per report from Jeevan Chalke.  This essentially corrects some sloppy
thinking in commit ed3ddf918b, so back-patch
to 9.3 where that logic appeared.
2015-09-24 12:47:29 -04:00
Andres Freund 168d5805e4 Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.
The newly added ON CONFLICT clause allows to specify an alternative to
raising a unique or exclusion constraint violation error when inserting.
ON CONFLICT refers to constraints that can either be specified using a
inference clause (by specifying the columns of a unique constraint) or
by naming a unique or exclusion constraint.  DO NOTHING avoids the
constraint violation, without touching the pre-existing row.  DO UPDATE
SET ... [WHERE ...] updates the pre-existing tuple, and has access to
both the tuple proposed for insertion and the existing tuple; the
optional WHERE clause can be used to prevent an update from being
executed.  The UPDATE SET and WHERE clauses have access to the tuple
proposed for insertion using the "magic" EXCLUDED alias, and to the
pre-existing tuple using the table name or its alias.

This feature is often referred to as upsert.

This is implemented using a new infrastructure called "speculative
insertion". It is an optimistic variant of regular insertion that first
does a pre-check for existing tuples and then attempts an insert.  If a
violating tuple was inserted concurrently, the speculatively inserted
tuple is deleted and a new attempt is made.  If the pre-check finds a
matching tuple the alternative DO NOTHING or DO UPDATE action is taken.
If the insertion succeeds without detecting a conflict, the tuple is
deemed inserted.

To handle the possible ambiguity between the excluded alias and a table
named excluded, and for convenience with long relation names, INSERT
INTO now can alias its target table.

Bumps catversion as stored rules change.

Author: Peter Geoghegan, with significant contributions from Heikki
    Linnakangas and Andres Freund. Testing infrastructure by Jeff Janes.
Reviewed-By: Heikki Linnakangas, Andres Freund, Robert Haas, Simon Riggs,
    Dean Rasheed, Stephen Frost and many others.
2015-05-08 05:43:10 +02:00
Tom Lane cb1ca4d800 Allow foreign tables to participate in inheritance.
Foreign tables can now be inheritance children, or parents.  Much of the
system was already ready for this, but we had to fix a few things of
course, mostly in the area of planner and executor handling of row locks.

As side effects of this, allow foreign tables to have NOT VALID CHECK
constraints (and hence to accept ALTER ... VALIDATE CONSTRAINT), and to
accept ALTER SET STORAGE and ALTER SET WITH/WITHOUT OIDS.  Continuing to
disallow these things would've required bizarre and inconsistent special
cases in inheritance behavior.  Since foreign tables don't enforce CHECK
constraints anyway, a NOT VALID one is a complete no-op, but that doesn't
mean we shouldn't allow it.  And it's possible that some FDWs might have
use for SET STORAGE or SET WITH OIDS, though doubtless they will be no-ops
for most.

An additional change in support of this is that when a ModifyTable node
has multiple target tables, they will all now be explicitly identified
in EXPLAIN output, for example:

 Update on pt1  (cost=0.00..321.05 rows=3541 width=46)
   Update on pt1
   Foreign Update on ft1
   Foreign Update on ft2
   Update on child3
   ->  Seq Scan on pt1  (cost=0.00..0.00 rows=1 width=46)
   ->  Foreign Scan on ft1  (cost=100.00..148.03 rows=1170 width=46)
   ->  Foreign Scan on ft2  (cost=100.00..148.03 rows=1170 width=46)
   ->  Seq Scan on child3  (cost=0.00..25.00 rows=1200 width=46)

This was done mainly to provide an unambiguous place to attach "Remote SQL"
fields, but it is useful for inherited updates even when no foreign tables
are involved.

Shigeru Hanada and Etsuro Fujita, reviewed by Ashutosh Bapat and Kyotaro
Horiguchi, some additional hacking by me
2015-03-22 13:53:21 -04:00
Tom Lane fc2ac1fb41 Allow CHECK constraints to be placed on foreign tables.
As with NOT NULL constraints, we consider that such constraints are merely
reports of constraints that are being enforced by the remote server (or
other underlying storage mechanism).  Their only real use is to allow
planner optimizations, for example in constraint-exclusion checks.  Thus,
the code changes here amount to little more than removal of the error that
was formerly thrown for applying CHECK to a foreign table.

(In passing, do a bit of cleanup of the ALTER FOREIGN TABLE reference page,
which had accumulated some weird decisions about ordering etc.)

Shigeru Hanada and Etsuro Fujita, reviewed by Kyotaro Horiguchi and
Ashutosh Bapat.
2014-12-17 17:00:53 -05:00
Tom Lane 8ec8760fc8 Revert misguided change to postgres_fdw FOR UPDATE/SHARE code.
In commit 462bd95705, I changed postgres_fdw
to rely on get_plan_rowmark() instead of get_parse_rowmark().  I still
think that's a good idea in the long run, but as Etsuro Fujita pointed out,
it doesn't work today because planner.c forces PlanRowMarks to have
markType = ROW_MARK_COPY for all foreign tables.  There's no urgent reason
to change this in the back branches, so let's just revert that part of
yesterday's commit rather than trying to design a better solution under
time pressure.

Also, add a regression test case showing what postgres_fdw does with FOR
UPDATE/SHARE.  I'd blithely assumed there was one already, else I'd have
realized yesterday that this code didn't work.
2014-12-12 12:41:49 -05:00
Tom Lane 9c58101117 Fix mishandling of system columns in FDW queries.
postgres_fdw would send query conditions involving system columns to the
remote server, even though it makes no effort to ensure that system
columns other than CTID match what the remote side thinks.  tableoid,
in particular, probably won't match and might have some use in queries.
Hence, prevent sending conditions that include non-CTID system columns.

Also, create_foreignscan_plan neglected to check local restriction
conditions while determining whether to set fsSystemCol for a foreign
scan plan node.  This again would bollix the results for queries that
test a foreign table's tableoid.

Back-patch the first fix to 9.3 where postgres_fdw was introduced.
Back-patch the second to 9.2.  The code is probably broken in 9.1 as
well, but the patch doesn't apply cleanly there; given the weak state
of support for FDWs in 9.1, it doesn't seem worth fixing.

Etsuro Fujita, reviewed by Ashutosh Bapat, and somewhat modified by me
2014-11-22 16:01:05 -05:00
Andres Freund 57ca1d4f01 Specify the port in dblink and postgres_fdw tests.
That allows to run those tests against a postmaster listening on a
nonstandard port without requiring to export PGPORT in postmaster's
environment.

This still doesn't support connecting to a nondefault host without
configuring it in postmaster's environment. That's harder and less
frequently used though. So this is a useful step.
2014-08-26 12:28:08 +02:00
Andres Freund ddc2504dbc Don't hardcode contrib_regression dbname in postgres_fdw and dblink tests.
That allows parallel installcheck to succeed inside contrib/. The
output is not particularly pretty unless make's -O option to
synchronize the output is used.

There's other tests, outside contrib, that use a hardcoded,
non-unique, database name. Those prohibit paralell installcheck to be
used across more directories; but that's something for a separate
patch.
2014-08-26 12:27:26 +02:00
Tom Lane 59efda3e50 Implement IMPORT FOREIGN SCHEMA.
This command provides an automated way to create foreign table definitions
that match remote tables, thereby reducing tedium and chances for error.
In this patch, we provide the necessary core-server infrastructure and
implement the feature fully in the postgres_fdw foreign-data wrapper.
Other wrappers will throw a "feature not supported" error until/unless
they are updated.

Ronan Dunklau and Michael Paquier, additional work by me
2014-07-10 15:01:43 -04:00
Tom Lane 5b68d81697 Fix contrib/postgres_fdw's remote-estimate representation of array Params.
We were emitting "(SELECT null::typename)", which is usually interpreted
as a scalar subselect, but not so much in the context "x = ANY(...)".
This led to remote-side parsing failures when remote_estimate is enabled.
A quick and ugly fix is to stick in an extra cast step,
"((SELECT null::typename)::typename)".  The cast will be thrown away as
redundant by parse analysis, but not before it's done its job of making
sure the grammar sees the ANY argument as an a_expr rather than a
select_with_parens.  Per an example from Hannu Krosing.
2014-04-16 17:21:57 -04:00
Noah Misch b2b2491b06 Don't test xmin/xmax columns of a postgres_fdw foreign table.
Their values are unspecified and system-dependent.

Per buildfarm member kouprey.
2014-03-23 03:48:17 -04:00