aboutsummaryrefslogtreecommitdiff
path: root/contrib/postgres_fdw
Commit message (Collapse)AuthorAge
...
* chomp PQerrorMessage() in backend usesPeter Eisentraut2017-02-27
| | | | | | | | PQerrorMessage() returns an error message with a trailing newline, but in backend use (dblink, postgres_fdw, libpqwalreceiver), we want to have the error message without that for emitting via ereport(). To simplify that, add a function pchomp() that returns a pstrdup'ed string with the trailing newline characters removed.
* Fix typos in comments.Heikki Linnakangas2017-02-06
| | | | | | | | | 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
* Fix typo in comment.Robert Haas2017-01-27
| | | | Etsuro Fujita
* Use the new castNode() macro in a number of places.Andres Freund2017-01-26
| | | | | | | | | This is far from a pervasive conversion, but it's a good starting point. Author: Peter Eisentraut, with some minor changes by me Reviewed-By: Tom Lane Discussion: https://postgr.es/m/c5d387d9-3440-f5e0-f9d4-71d53b9fbe52@2ndquadrant.com
* Improve speed of contrib/postgres_fdw regression tests.Tom Lane2017-01-25
| | | | | | | | | | | 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
* Move some things from builtins.h to new header filesPeter Eisentraut2017-01-20
| | | | This avoids that builtins.h has to include additional header files.
* Remove obsoleted code relating to targetlist SRF evaluation.Andres Freund2017-01-19
| | | | | | | | | | | | | Since 69f4b9c plain expression evaluation (and thus normal projection) can't return sets of tuples anymore. Thus remove code dealing with that possibility. This will require adjustments in external code using ExecEvalExpr()/ExecProject() - that should neither be hard nor very common. Author: Andres Freund and Tom Lane Discussion: https://postgr.es/m/20160822214023.aaxz5l4igypowyri@alap3.anarazel.de
* Invalidate cached plans on FDW option changes.Tom Lane2017-01-06
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Update copyright via script for 2017Bruce Momjian2017-01-03
|
* Improve dblink error message when remote does not provide itJoe Conway2016-12-21
| | | | | | | | | | | | | When dblink or postgres_fdw detects an error on the remote side of the connection, it will try to construct a local error message as best it can using libpq's PQresultErrorField(). When no primary message is available, it was bailing out with an unhelpful "unknown error". Make that message better and more style guide compliant. Per discussion on hackers. Backpatch to 9.2 except postgres_fdw which didn't exist before 9.3. Discussion: https://postgr.es/m/19872.1482338965%40sss.pgh.pa.us
* postgres_fdw: Fix typo in comment.Robert Haas2016-11-04
| | | | Etsuro Fujita
* psql: Split up "Modifiers" column in \d and \dDPeter Eisentraut2016-11-03
| | | | | | Make separate columns "Collation", "Nullable", "Default". Reviewed-by: Kuntal Ghosh <kuntalghosh.2007@gmail.com>
* Use NIL instead of NULL for an empty List.Robert Haas2016-11-03
| | | | Tatsuro Yamada, reviewed by Ashutosh Bapat
* Don't convert Consts into Vars during setrefs.c processing.Tom Lane2016-11-02
| | | | | | | | | | | | | | | | | | | | | | | | | While converting expressions in an upper-level plan node so that they reference Vars and expressions provided by the input plan node(s), don't convert plain Const items, even if there happens to be a matching Const in the input. It's silly to do so because a Var is more expensive to execute than a Const. Moreover, converting can fool ExecCheckPlanOutput's check that an insert or update query inserts nulls into dropped columns, leading to "query provides a value for a dropped column" errors during INSERT or UPDATE on a table with a dropped column. We could solve this by making that check more complicated, but I don't see the point; this fix should save a marginal number of cycles, and it also makes for less messy EXPLAIN output, as shown by the ensuing regression test result changes. Per report from Pavel HanĂ¡k. I have not incorporated a test case based on that example, as there doesn't seem to be a simple way of checking this in isolation without making a bunch of assumptions about other planner and SQL-function behavior. Back-patch to 9.6. This setrefs.c behavior exists much further back, but there is not currently reason to think that it causes problems before 9.6. Discussion: <83shraampf.fsf@is-it.eu>
* postgres_fdw: Fix typo in comment.Robert Haas2016-11-01
| | | | Etsuro Fujita
* Suppress unused-variable warning in non-assert builds.Tom Lane2016-10-26
| | | | | | Introduced in commit 7012b132d. Kyotaro Horiguchi
* postgres_fdw: Try again to stabilize aggregate pushdown regression tests.Robert Haas2016-10-24
| | | | | | | | 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
* postgres_fdw: Attempt to stabilize regression results.Robert Haas2016-10-21
| | | | | | | | | | | 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.
* postgres_fdw: Push down aggregates to remote servers.Robert Haas2016-10-21
| | | | | | | | | | | | | 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.
* Rename WAIT_* constants to PG_WAIT_*.Robert Haas2016-10-05
| | | | | | | | Windows apparently has a constant named WAIT_TIMEOUT, and some of these other names are pretty generic, too. Insert "PG_" at the front of each name in order to disambiguate. Michael Paquier
* Extend framework from commit 53be0b1ad to report latch waits.Robert Haas2016-10-04
| | | | | | | | | | | | | | | | | | | | | | WaitLatch, WaitLatchOrSocket, and WaitEventSetWait now taken an additional wait_event_info parameter; legal values are defined in pgstat.h. This makes it possible to uniquely identify every point in the core code where we are waiting for a latch; extensions can pass WAIT_EXTENSION. Because latches were the major wait primitive not previously covered by this patch, it is now possible to see information in pg_stat_activity on a large number of important wait events not previously addressed, such as ClientRead, ClientWrite, and SyncRep. Unfortunately, many of the wait events added by this patch will fail to appear in pg_stat_activity because they're only used in background processes which don't currently appear in pg_stat_activity. We should fix this either by creating a separate view for such information, or else by deciding to include them in pg_stat_activity after all. Michael Paquier and Robert Haas, reviewed by Alexander Korotkov and Thomas Munro.
* Add macros to make AllocSetContextCreate() calls simpler and safer.Tom Lane2016-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I found that half a dozen (nearly 5%) of our AllocSetContextCreate calls had typos in the context-sizing parameters. While none of these led to especially significant problems, they did create minor inefficiencies, and it's now clear that expecting people to copy-and-paste those calls accurately is not a great idea. Let's reduce the risk of future errors by introducing single macros that encapsulate the common use-cases. Three such macros are enough to cover all but two special-purpose contexts; those two calls can be left as-is, I think. While this patch doesn't in itself improve matters for third-party extensions, it doesn't break anything for them either, and they can gradually adopt the simplified notation over time. In passing, change TopMemoryContext to use the default allocation parameters. Formerly it could only be extended 8K at a time. That was probably reasonable when this code was written; but nowadays we create many more contexts than we did then, so that it's not unusual to have a couple hundred K in TopMemoryContext, even without considering various dubious code that sticks other things there. There seems no good reason not to let it use growing blocks like most other contexts. Back-patch to 9.6, mostly because that's still close enough to HEAD that it's easy to do so, and keeping the branches in sync can be expected to avoid some future back-patching pain. The bugs fixed by these changes don't seem to be significant enough to justify fixing them further back. Discussion: <21072.1472321324@sss.pgh.pa.us>
* Support OID system column in postgres_fdw.Heikki Linnakangas2016-08-26
| | | | | | | | | | 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>
* postgres_fdw: Cosmetic cleanup.Robert Haas2016-08-24
| | | | Etsuro Fujita
* Fix assorted fallout from IS [NOT] NULL patch.Tom Lane2016-07-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits 4452000f3 et al established semantics for NullTest.argisrow that are a bit different from its initial conception: rather than being merely a cache of whether we've determined the input to have composite type, the flag now has the further meaning that we should apply field-by-field testing as per the standard's definition of IS [NOT] NULL. If argisrow is false and yet the input has composite type, the construct instead has the semantics of IS [NOT] DISTINCT FROM NULL. Update the comments in primnodes.h to clarify this, and fix ruleutils.c and deparse.c to print such cases correctly. In the case of ruleutils.c, this merely results in cosmetic changes in EXPLAIN output, since the case can't currently arise in stored rules. However, it represents a live bug for deparse.c, which would formerly have sent a remote query that had semantics different from the local behavior. (From the user's standpoint, this means that testing a remote nested-composite column for null-ness could have had unexpected recursive behavior much like that fixed in 4452000f3.) In a related but somewhat independent fix, make plancat.c set argisrow to false in all NullTest expressions constructed to represent "attnotnull" constructs. Since attnotnull is actually enforced as a simple null-value check, this is a more accurate representation of the semantics; we were previously overpromising what it meant for composite columns, which might possibly lead to incorrect planner optimizations. (It seems that what the SQL spec expects a NOT NULL constraint to mean is an IS NOT NULL test, so arguably we are violating the spec and should fix attnotnull to do the other thing. If we ever do, this part should get reverted.) Back-patch, same as the previous commit. Discussion: <10682.1469566308@sss.pgh.pa.us>
* Fix typo in comment.Fujii Masao2016-07-25
| | | | Author: Masahiko Sawada
* Avoid invalidating all foreign-join cached plans when user mappings change.Tom Lane2016-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* postgres_fdw: Fix cache lookup failure while creating error context.Robert Haas2016-07-01
| | | | | | | | 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
* postgres_fdw: Remove schema-qualification from cast to text.Robert Haas2016-07-01
| | | | | | As pointed out by Ashutosh Bapat, the header comments for this file say that schema-qualification is needed for all and only those types outside pg_catalog. pg_catalog.text is not outside pg_catalog.
* postgres_fdw: Fix incorrect NULL handling in join pushdown.Robert Haas2016-06-24
| | | | | | | | | | 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.
* postgres_fdw: Remove useless return statement.Robert Haas2016-06-24
| | | | Etsuro Fujita
* postgres_fdw: Rephrase comment.Robert Haas2016-06-17
| | | | | Per gripe from Thomas Munro, who only complained about a more localized problem, but I couldn't resist a bit more wordsmithing.
* postgres_fdw: Check PlaceHolderVars before pushing down a join.Robert Haas2016-06-14
| | | | | | | | | | | | | | | | | | | 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.
* postgres_fdw: Promote an Assert() to elog().Robert Haas2016-06-14
| | | | | | | | | | Andreas Seltenreich reports that it is possible for a PlaceHolderVar to creep into this tlist, and I fear that even after that's fixed we might have other, similar bugs in this area either now or in the future. There's a lot of action-at-a-distance here, because the validity of this assertion depends on core planner behavior; so, let's use elog() to make sure we catch this even in non-assert builds, rather than just crashing.
* pgindent run for 9.6Robert Haas2016-06-09
|
* postgres_fdw: Fix the fix for crash when pushing down multiple joins.Robert Haas2016-05-16
| | | | | | | | | | | | | | Commit 3151f16e1874db82ed85a005dac15368903ca9fb 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
* Fix multiple problems in postgres_fdw query cancellation logic.Robert Haas2016-05-16
| | | | | | | | | | | First, even if we cancel a query, we still have to roll back the containing transaction; otherwise, the session will be left in a failed transaction state. Second, we need to support canceling queries whe aborting a subtransaction as well as when aborting a toplevel transaction. Etsuro Fujita, reviewed by Michael Paquier
* Allow queries submitted by postgres_fdw to be canceled.Robert Haas2016-04-21
| | | | | | | | | | This fixes a problem which is not new, but with the advent of direct foreign table modification in 0bf3ae88af330496517722e391e7c975e6bad219, it's somewhat more likely to be annoying than previously. So, arrange for a local query cancelation to propagate to the remote side. Michael Paquier, reviewed by Etsuro Fujita. Original report by Thom Brown.
* postgres_fdw: Don't push down certain full joins.Robert Haas2016-04-20
| | | | | | | | | | | | | | | | 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 e4106b2528727c4b48639c0e12bf2f70a766b910. Ashutosh Bapat, per report from Rajkumar Raghuwanshi.
* postgres_fdw: Clean up handling of system columns.Robert Haas2016-04-15
| | | | | | | | | | | | | | | | Previously, querying the xmin column of a single postgres_fdw foreign table fetched the tuple length, xmax the typmod, and cmin or cmax the composite type OID of the tuple. However, when you queried several such tables and the join got shipped to the remote side, these columns ended up containing the remote values of the corresponding columns. Both behaviors are rather unprincipled, the former for obvious reasons and the latter because the remote values of these columns don't have any local significance; our transaction IDs are in a different space than those of the remote machine. Clean this up by setting all of these fields to 0 in both cases. Also fix the handling of tableoid to be sane. Robert Haas and Ashutosh Bapat, reviewed by Etsuro Fujita.
* Run pgindent on a batch of (mostly-planner-related) source files.Tom Lane2016-04-06
| | | | | Getting annoyed at the amount of unrelated chatter I get from pgindent'ing Rowley's unique-joins patch. Re-indent all the files it touches.
* Don't require a user mapping for FDWs to work.Robert Haas2016-03-28
| | | | | | | | | Commit fbe5a3fb73102c2cfec11aaaa4a67943f4474383 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.
* postgres_fdw: Fix crash when pushing down multiple joins.Robert Haas2016-03-23
| | | | | | | | | 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.
* Clean up some Coverity complaints about commit 0bf3ae88af330496.Tom Lane2016-03-21
| | | | | | | | | The two get_tle_by_resno() calls introduced by this commit lacked any check for a NULL return, unlike any other calls of that function anywhere in our tree. Coverity quite properly complained about it. Also fix a misindented line in process_query_params(), which Coverity also complained about on the grounds that the bad indentation suggested possible programmer misinterpretation.
* Directly modify foreign tables.Robert Haas2016-03-18
| | | | | | | | | 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.
* postgres_fdw: make_tuple_from_result_row should set cur_attno for ctid.Robert Haas2016-03-15
| | | | | | | | | There's no reason for this function to do this for every other attribute number and omit it for CTID, especially since conversion_error_callback has code to handle that case. This seems to be an oversight in commit e690b9515072fd7767fdeca5c54166f6a77733bc. Etsuro Fujita
* Allow callers of create_foreignscan_path to specify nondefault PathTarget.Tom Lane2016-03-14
| | | | | | | | | Although the default choice of rel->reltarget should typically be sufficient for scan or join paths, it's not at all sufficient for the purposes PathTargets were invented for; in particular not for upper-relation Paths. So break API compatibility by adding a PathTarget argument to create_foreignscan_path(). To ease updating of existing code, accept a NULL value of the argument as selecting rel->reltarget.
* Rethink representation of PathTargets.Tom Lane2016-03-14
| | | | | | | | | | | | | | In commit 19a541143a09c067 I did not make PathTarget a subtype of Node, and embedded a RelOptInfo's reltarget directly into it rather than having a separately-allocated Node. In hindsight that was misguided micro-optimization, enabled by the fact that at that point we didn't have any Paths with custom PathTargets. Now that PathTarget processing has been fleshed out some more, it's easier to see that it's better to have PathTarget as an indepedent Node type, even if it does cost us one more palloc to create a RelOptInfo. So change it while we still can. This commit just changes the representation, without doing anything more interesting than that.
* Update more comments for 96198d94cb7adc664bda341842dc8db671d8be72.Robert Haas2016-03-14
| | | | | Etsuro Fujita, reviewed (though not completely endorsed) by Ashutosh Bapat, and slightly expanded by me.
* Refactor pull_var_clause's API to make it less tedious to extend.Tom Lane2016-03-10
| | | | | | | | | | | | | | | | | | | | In commit 1d97c19a0f748e94 and later c1d9579dd8bf3c92, we extended pull_var_clause's API by adding enum-type arguments. That's sort of a pain to maintain, though, because it means every time we add a new behavior we must touch every last one of the call sites, even if there's a reasonable default behavior that most of them could use. Let's switch over to using a bitmask of flags, instead; that seems more maintainable and might save a nanosecond or two as well. This commit changes no behavior in itself, though I'm going to follow it up with one that does add a new behavior. In passing, remove flatten_tlist(), which has not been used since 9.1 and would otherwise need the same API changes. Removing these enums means that optimizer/tlist.h no longer needs to depend on optimizer/var.h. Changing that caused a number of C files to need addition of #include "optimizer/var.h" (probably we can thank old runs of pgrminclude for that); but on balance it seems like a good change anyway.