56 Commits (a67c75f8258742945f4f3703126a1222adeeb85b)

Author	SHA1	Message	Date
Tom Lane	6478896675	Teach hash_ok_operator() that record_eq is only sometimes hashable. ... The need for this was foreseen long ago, but when record_eq actually became hashable (in commit 01e658fa7), we missed updating this spot. Per bug #17363 from Elvis Pranskevichus. Back-patch to v14 where the faulty commit came in. Discussion: https://postgr.es/m/17363-f6d42fd0d726be02@postgresql.org	4 years ago
Tom Lane	e8638d78a2	Fix planner error with multiple copies of an AlternativeSubPlan. ... It's possible for us to copy an AlternativeSubPlan expression node into multiple places, for example the scan quals of several partition children. Then it's possible that we choose a different one of the alternatives as optimal in each place. Commit 41efb8340 failed to consider this scenario, so its attempt to remove "unused" subplans could remove subplans that were still used elsewhere. Fix by delaying the removal logic until we've examined all the AlternativeSubPlans in a given query level. (This does assume that AlternativeSubPlans couldn't get copied to other query levels, but for the foreseeable future that's fine; cf qual_is_pushdown_safe.) Per report from Rajkumar Raghuwanshi. Back-patch to v14 where the faulty logic came in. Discussion: https://postgr.es/m/CAKcux6==O3NNZC3bZ2prRYv3cjm3_Zw1GfzmOjEVqYN4jub2+Q@mail.gmail.com	5 years ago
Peter Eisentraut	c06d6aa4c3	Clean up ancient test style ... Many older tests where written in a style like SELECT '' AS two, i.* FROM INT2_TBL where the first column indicated the number of expected result rows. This has gotten increasingly out of date, as the test data fixtures have expanded, so a lot of these were wrong and misleading. Moreover, this style isn't really necessary, since the psql output already shows the number of result rows. To clean this up, remove all those extra columns. Discussion: https://www.postgresql.org/message-id/flat/1a25312b-2686-380d-3c67-7a69094a999f%40enterprisedb.com	6 years ago
Tom Lane	41efb83408	Move resolution of AlternativeSubPlan choices to the planner. ... When commit bd3daddaf introduced AlternativeSubPlans, I had some ambitions towards allowing the choice of subplan to change during execution. That has not happened, or even been thought about, in the ensuing twelve years; so it seems like a failed experiment. So let's rip that out and resolve the choice of subplan at the end of planning (in setrefs.c) rather than during executor startup. This has a number of positive benefits: * Removal of a few hundred lines of executor code, since AlternativeSubPlans need no longer be supported there. * Removal of executor-startup overhead (particularly, initialization of subplans that won't be used). * Removal of incidental costs of having a larger plan tree, such as tree-scanning and copying costs in the plancache; not to mention setrefs.c's own costs of processing the discarded subplans. * EXPLAIN no longer has to print a weird (and undocumented) representation of an AlternativeSubPlan choice; it sees only the subplan actually used. This should mean less confusion for users. * Since setrefs.c knows which subexpression of a plan node it's working on at any instant, it's possible to adjust the estimated number of executions of the subplan based on that. For example, we should usually estimate more executions of a qual expression than a targetlist expression. The implementation used here is pretty simplistic, because we don't want to expend a lot of cycles on the issue; but it's better than ignoring the point entirely, as the executor had to. That last point might possibly result in shifting the choice between hashed and non-hashed EXISTS subplans in a few cases, but in general this patch isn't meant to change planner choices. Since we're doing the resolution so late, it's really impossible to change any plan choices outside the AlternativeSubPlan itself. Patch by me; thanks to David Rowley for review. Discussion: https://postgr.es/m/1992952.1592785225@sss.pgh.pa.us	6 years ago
Tom Lane	1e7629d2c9	Be more careful about the shape of hashable subplan clauses. ... nodeSubplan.c expects that the testexpr for a hashable ANY SubPlan has the form of one or more OpExprs whose LHS is an expression of the outer query's, while the RHS is an expression over Params representing output columns of the subquery. However, the planner only went as far as verifying that the clauses were all binary OpExprs. This works 99.99% of the time, because the clauses have the right shape when emitted by the parser --- but it's possible for function inlining to break that, as reported by PegoraroF10. To fix, teach the planner to check that the LHS and RHS contain the right things, or more accurately don't contain the wrong things. Given that this has been broken for years without anyone noticing, it seems sufficient to just give up hashing when it happens, rather than go to the trouble of commuting the clauses back again (which wouldn't necessarily work anyway). While poking at that, I also noticed that nodeSubplan.c had a baked-in assumption that the number of hash clauses is identical to the number of subquery output columns. Again, that's fine as far as parser output goes, but it's not hard to break it via function inlining. There seems little reason for that assumption though --- AFAICS, the only thing it's buying us is not having to store the number of hash clauses explicitly. Adding code to the planner to reject such cases would take more code than getting nodeSubplan.c to cope, so I fixed it that way. This has been broken for as long as we've had hashable SubPlans, so back-patch to all supported branches. Discussion: https://postgr.es/m/1549209182255-0.post@n3.nabble.com	6 years ago
Tom Lane	a742ecf9c6	Cope with lateral references in the quals of a subquery RTE. ... The qual pushdown logic assumed that all Vars in a restriction clause must be Vars referencing subquery outputs; but since we introduced LATERAL, it's possible for such a Var to be a lateral reference instead. This led to an assertion failure in debug builds. In a non-debug build, there might be no ill effects (if qual_is_pushdown_safe decided the qual was unsafe anyway), or we could get failures later due to construction of an invalid plan. I've not gone to much length to characterize the possible failures, but at least segfaults in the executor have been observed. Given that this has been busted since 9.3 and it took this long for anybody to notice, I judge that the case isn't worth going to great lengths to optimize. Hence, fix by just teaching qual_is_pushdown_safe that such quals are unsafe to push down, matching the previous behavior when it accidentally didn't fail. Per report from Tom Ellis. Back-patch to all supported branches. Discussion: https://postgr.es/m/20200713175124.GQ8220@cloudinit-builder	6 years ago
Tom Lane	58c47ccfff	Correctly re-use hash tables in buildSubPlanHash(). ... Commit 356687bd8 omitted to remove leftover code for destroying a hashed subplan's hash tables, with the result that the tables were always rebuilt not reused; this leads to severe memory leakage if a hashed subplan is re-executed enough times. Moreover, the code for reusing the hashnulls table had a typo that would have made it do the wrong thing if it were reached. Looking at the code coverage report shows severe under-coverage of the potential callers of ResetTupleHashTable, so add some test cases that exercise them. Andreas Karlsson and Tom Lane, per reports from Ranier Vilela and Justin Pryzby. Backpatch to v11, as the faulty commit was. Discussion: https://postgr.es/m/edb62547-c453-c35b-3ed6-a069e4d6b937@proxel.se Discussion: https://postgr.es/m/CAEudQAo=DCebm1RXtig9OH+QivpS97sMkikt0A9qHmMUs+g6ZA@mail.gmail.com Discussion: https://postgr.es/m/20200210032547.GA1412@telsasoft.com	6 years ago
Tom Lane	41c6f9db25	Repair more failures with SubPlans in multi-row VALUES lists. ... Commit 9b63c13f0 turns out to have been fundamentally misguided: the parent node's subPlan list is by no means the only way in which a child SubPlan node can be hooked into the outer execution state. As shown in bug #16213 from Matt Jibson, we can also get short-lived tuple table slots added to the outer es_tupleTable list. At this point I have little faith that there aren't other possible connections as well; the long time it took to notice this problem shows that this isn't a heavily-exercised situation. Therefore, revert that fix, returning to the coding that passed a NULL parent plan pointer down to the transiently-built subexpressions. That gives us a pretty good guarantee that they won't hook into the outer executor state in any way. But then we need some other solution to make SubPlans work. Adopt the solution speculated about in the previous commit's log message: do expression initialization at plan startup for just those VALUES rows containing SubPlans, abandoning the goal of reclaiming memory intra-query for those rows. In practice it seems unlikely that queries containing a vast number of VALUES rows would be using SubPlans in them, so this should not give up much. (BTW, this test case also refutes my claim in connection with the prior commit that the issue only arises with use of LATERAL. That was just wrong: some variants of SubLink always produce SubPlans.) As with previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16213-871ac3bc208ecf23@postgresql.org	6 years ago
Tom Lane	b925a00f4e	Fix "force_parallel_mode = regress" to work with ANALYZE + VERBOSE. ... force_parallel_mode = regress is supposed to force use of a Gather node without having any impact on EXPLAIN output. But it failed to accomplish that if both ANALYZE and VERBOSE are given, because that enables per-worker output data that you wouldn't see if the Gather hadn't been inserted. Improve the logic so that we suppress the per-worker data too. This allows putting the new test case added by commit 5935917ce back into the originally intended form (cf. 776a2c887, 22864f6e0). We can also get rid of a kluge in subselect.sql, which previously had to clean up after force_parallel_mode's failure to do what it said on the tin. Discussion: https://postgr.es/m/18445.1576177309@sss.pgh.pa.us	7 years ago
Tom Lane	4766dce0dd	Fix choice of comparison operators for cross-type hashed subplans. ... Commit bf6c614a2 rearranged the lookup of the comparison operators needed in a hashed subplan, and in so doing, broke the cross-type case: it caused the original LHS-vs-RHS operator to be used to compare hash table entries too (which of course are all of the RHS type). This leads to C functions being passed a Datum that is not of the type they expect, with the usual hazards of crashes and unauthorized server memory disclosure. For the set of hashable cross-type operators present in v11 core Postgres, this bug is nearly harmless on 64-bit machines, which may explain why it escaped earlier detection. But it is a live security hazard on 32-bit machines; and of course there may be extensions that add more hashable cross-type operators, which would increase the risk. Reported by Andreas Seltenreich. Back-patch to v11 where the problem came in. Security: CVE-2019-10209	7 years ago
Tom Lane	9476131278	Prevent inlining of multiply-referenced CTEs with outer recursive refs. ... This has to be prevented because inlining would result in multiple self-references, which we don't support (and in fact that's disallowed by the SQL spec, see statements about linearly vs. nonlinearly recursive queries). Bug fix for commit 608b167f9. Per report from Yaroslav Schekin (via Andrew Gierth) Discussion: https://postgr.es/m/87wolmg60q.fsf@news-spur.riddles.org.uk	7 years ago
Peter Eisentraut	5e1963fb76	Collations with nondeterministic comparison ... This adds a flag "deterministic" to collations. If that is false, such a collation disables various optimizations that assume that strings are equal only if they are byte-wise equal. That then allows use cases such as case-insensitive or accent-insensitive comparisons or handling of strings with different Unicode normal forms. This functionality is only supported with the ICU provider. At least glibc doesn't appear to have any locales that work in a nondeterministic way, so it's not worth supporting this for the libc provider. The term "deterministic comparison" in this context is from Unicode Technical Standard #10 (https://unicode.org/reports/tr10/#Deterministic_Comparison). This patch makes changes in three areas: - CREATE COLLATION DDL changes and system catalog changes to support this new flag. - Many executor nodes and auxiliary code are extended to track collations. Previously, this code would just throw away collation information, because the eventually-called user-defined functions didn't use it since they only cared about equality, which didn't need collation information. - String data type functions that do equality comparisons and hashing are changed to take the (non-)deterministic flag into account. For comparison, this just means skipping various shortcuts and tie breakers that use byte-wise comparison. For hashing, we first need to convert the input string to a canonical "sort key" using the ICU analogue of strxfrm(). Reviewed-by: Daniel Verite <daniel@manitou-mail.org> Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://www.postgresql.org/message-id/flat/1ccc668f-4cbc-0bef-af67-450b47cdfee7@2ndquadrant.com	7 years ago
Tom Lane	3396138a6d	Check we don't misoptimize a NOT IN where the subquery returns no rows. ... Future-proofing against a common mistake in attempts to optimize NOT IN. We don't have such an optimization right now, but attempts to do so are in the works, and some of 'em are buggy. Add a regression test case covering the point. David Rowley Discussion: https://postgr.es/m/CAKJS1f90E9agVZryVyUpbHQbjTt5ExqS2Fsodmt5_A7E_cEyVA@mail.gmail.com	7 years ago
Tom Lane	608b167f9f	Allow user control of CTE materialization, and change the default behavior. ... Historically we've always materialized the full output of a CTE query, treating WITH as an optimization fence (so that, for example, restrictions from the outer query cannot be pushed into it). This is appropriate when the CTE query is INSERT/UPDATE/DELETE, or is recursive; but when the CTE query is non-recursive and side-effect-free, there's no hazard of changing the query results by pushing restrictions down. Another argument for materialization is that it can avoid duplicate computation of an expensive WITH query --- but that only applies if the WITH query is called more than once in the outer query. Even then it could still be a net loss, if each call has restrictions that would allow just a small part of the WITH query to be computed. Hence, let's change the behavior for WITH queries that are non-recursive and side-effect-free. By default, we will inline them into the outer query (removing the optimization fence) if they are called just once. If they are called more than once, we will keep the old behavior by default, but the user can override this and force inlining by specifying NOT MATERIALIZED. Lastly, the user can force the old behavior by specifying MATERIALIZED; this would mainly be useful when the query had deliberately been employing WITH as an optimization fence to prevent a poor choice of plan. Andreas Karlsson, Andrew Gierth, David Fetter Discussion: https://postgr.es/m/87sh48ffhb.fsf@news-spur.riddles.org.uk	7 years ago
Tom Lane	a391ff3c3d	Build out the planner support function infrastructure. ... Add support function requests for estimating the selectivity, cost, and number of result rows (if a SRF) of the target function. The lack of a way to estimate selectivity of a boolean-returning function in WHERE has been a recognized deficiency of the planner since Berkeley days. This commit finally fixes it. In addition, non-constant estimates of cost and number of output rows are now possible. We still fall back to looking at procost and prorows if the support function doesn't service the request, of course. To make concrete use of the possibility of estimating output rowcount for SRFs, this commit adds support functions for array_unnest(anyarray) and the integer variants of generate_series; the lack of plausible rowcount estimates for those, even when it's obvious to a human, has been a repeated subject of complaints. Obviously, much more could now be done in this line, but I'm mostly just trying to get the infrastructure in place. Discussion: https://postgr.es/m/15193.1548028093@sss.pgh.pa.us	7 years ago
Andrew Gierth	520acab171	Set scan direction appropriately for SubPlans (bug #15336) ... When executing a SubPlan in an expression, the EState's direction field was left alone, resulting in an attempt to execute the subplan backwards if it was encountered during a backwards scan of a cursor. Also, though much less likely, it was possible to reach the execution of an InitPlan while in backwards-scan state. Repair by saving/restoring estate->es_direction and forcing forward scan mode in the relevant places. Backpatch all the way, since this has been broken since 8.3 (prior to commit c7ff7663e, SubPlans had their own EStates rather than sharing the parent plan's, so there was no confusion over scan direction). Per bug #15336 reported by Vladimir Baranoff; analysis and patch by me, review by Tom Lane. Discussion: https://postgr.es/m/153449812167.1304.1741624125628126322@wrigleys.postgresql.org	8 years ago
Andres Freund	4f63e85eb1	Add COSTS off to two EXPLAIN using tests. ... Discussion: https://postgr.es/m/20180312222023.i4sgkbl4oqtstus3@alap3.anarazel.de	8 years ago
Tom Lane	9b63c13f0a	Repair failure with SubPlans in multi-row VALUES lists. ... When nodeValuesscan.c was written, it was impossible to have a SubPlan in VALUES --- any sub-SELECT there would have to be uncorrelated and thereby would produce an InitPlan instead. We therefore took a shortcut in the logic that throws away a ValuesScan's per-row expression evaluation data structures. This was broken by the introduction of LATERAL however; a sub-SELECT containing a lateral reference produces a correlated SubPlan. The cleanest fix for this would be to give up the optimization of discarding the expression eval state. But that still seems pretty unappetizing for long VALUES lists. It seems to work to just prevent the subexpressions from hooking into the ValuesScan node's subPlan list, so let's do that and see how well it works. (If this breaks, due to additional connections between the subexpressions and the outer query structures, we might consider compromises like throwing away data only for VALUES rows not containing SubPlans.) Per bug #14924 from Christian Duta. Back-patch to 9.3 where LATERAL was introduced. Discussion: https://postgr.es/m/20171124120836.1463.5310@wrigleys.postgresql.org	9 years ago
Tom Lane	896eb5efbd	In the planner, delete joinaliasvars lists after we're done with them. ... Although joinaliasvars lists coming out of the parser are quite simple, those lists can contain arbitrarily complex expressions after subquery pullup. We do not perform expression preprocessing on them, meaning that expressions in those lists will not meet the expectations of later phases of the planner (for example, that they do not contain SubLinks). This had been thought pretty harmless, since we don't intentionally touch those lists in later phases --- but Andreas Seltenreich found a case in which adjust_appendrel_attrs() could recurse into a joinaliasvars list and then die on its assertion that it never sees a SubLink. We considered a couple of localized fixes to prevent that specific case from looking at the joinaliasvars lists, but really this seems like a generic hazard for all expression processing in the planner. Therefore, probably the best answer is to delete the joinaliasvars lists from the parsetree at the end of expression preprocessing, so that there are no reachable expressions that haven't been through preprocessing. The case Andreas found seems to be harmless in non-Assert builds, and so far there are no field reports suggesting that there are user-visible effects in other cases. I considered back-patching this anyway, but it turns out that Andreas' test doesn't fail at all in 9.4-9.6, because in those versions adjust_appendrel_attrs contains code (added in commit 842faa714 and removed again in commit 215b43cdc) to process SubLinks rather than complain about them. Barring discovery of another path by which unprocessed joinaliasvars lists can cause trouble, the most prudent compromise seems to be to patch this into v10 but not further. Patch by me, with thanks to Amit Langote for initial investigation and review. Discussion: https://postgr.es/m/87r2tvt9f1.fsf@ansel.ydns.eu	9 years ago
Robert Haas	bf11e7ee2e	Propagate sort instrumentation from workers back to leader. ... Up until now, when parallel query was used, no details about the sort method or space used by the workers were available; details were shown only for any sorting done by the leader. Fix that. Commit 1177ab1dab forced the test case added by commit 1f6d515a67 to run without parallelism; now that we have this infrastructure, allow that again, with a little tweaking to make it pass with and without force_parallel_mode. Robert Haas and Tom Lane Discussion: http://postgr.es/m/CA+Tgmoa2VBZW6S8AAXfhpHczb=Rf6RqQ2br+zJvEgwJ0uoD_tQ@mail.gmail.com	9 years ago
Tom Lane	1177ab1dab	Make new regression test case parallel-safe, and improve its output. ... The test case added by commit 1f6d515a6 fails on buildfarm members that have force_parallel_mode turned on, because we currently don't report sort performance details from worker processes back to the master. To fix that, just make the test table be temp rather than regular; that's a good idea anyway to forestall any possible interference from auto-analyze. (The restriction that workers can't access temp tables might go away someday, but almost certainly not before the other thing gets fixed.) Also, improve the test so that we retain as much as possible of the EXPLAIN ANALYZE output. This aids debugging failures, and might also expose problems that the preceding version masked. Discussion: http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com	9 years ago
Robert Haas	1f6d515a67	Push limit through subqueries to underlying sort, where possible. ... Douglas Doole, reviewed by Ashutosh Bapat and by me. Minor formatting change by me. Discussion: http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com	9 years ago
Tom Lane	555494d1bc	Fix placement of initPlans when forcibly materializing a subplan. ... If we forcibly place a Material node atop a finished subplan, we need to move any initPlans attached to the subplan up to the Material node, in order to keep SS_finalize_plan() happy. I'd figured this out in commit 7b67a0a49 for the case of materializing a cursor plan, but out of an abundance of caution, I put the initPlan movement hack at the call site for that case, rather than inside materialize_finished_plan(). That was the wrong thing, because it turns out to also be necessary for the only other caller of materialize_finished_plan(), ie subselect.c. We lacked any test cases that exposed the mistake, but bug#14524 from Wei Congrui shows that it's possible to get an initPlan reference into the top tlist in that case too, and then SS_finalize_plan() complains. Hence, move the hack into materialize_finished_plan(). In HEAD, also relocate some recently-added tests in subselect.sql, which I'd unthinkingly dropped into the middle of a sequence of related tests. Report: https://postgr.es/m/20170202060020.1400.89021@wrigleys.postgresql.org	9 years ago
Tom Lane	1e7c4bb004	Change unknown-type literals to type text in SELECT and RETURNING lists. ... Previously, we left such literals alone if the query or subquery had no properties forcing a type decision to be made (such as an ORDER BY or DISTINCT clause using that output column). This meant that "unknown" could be an exposed output column type, which has never been a great idea because it could result in strange failures later on. For example, an outer query that tried to do any operations on an unknown-type subquery output would generally fail with some weird error like "failed to find conversion function from unknown to text" or "could not determine which collation to use for string comparison". Also, if the case occurred in a CREATE VIEW's query then the view would have an unknown-type column, causing similar failures in queries trying to use the view. To fix, at the tail end of parse analysis of a query, forcibly convert any remaining "unknown" literals in its SELECT or RETURNING list to type text. However, provide a switch to suppress that, and use it in the cases of SELECT inside a set operation or INSERT command. In those cases we already had type resolution rules that make use of context information from outside the subquery proper, and we don't want to change that behavior. Also, change creation of an unknown-type column in a relation from a warning to a hard error. The error should be unreachable now in CREATE VIEW or CREATE MATVIEW, but it's still possible to explicitly say "unknown" in CREATE TABLE or CREATE (composite) TYPE. We want to forbid that because it's nothing but a foot-gun. This change creates a pg_upgrade failure case: a matview that contains an unknown-type column can't be pg_upgraded, because reparsing the matview's defining query will now decide that the column is of type text, which doesn't match the cstring-like storage that the old materialized column would actually have. Add a checking pass to detect that. While at it, we can detect tables or composite types that would fail, essentially for free. Those would fail safely anyway later on, but we might as well fail earlier. This patch is by me, but it owes something to previous investigations by Rahila Syed. Also thanks to Ashutosh Bapat and Michael Paquier for review. Discussion: https://postgr.es/m/CAH2L28uwwbL9HUM-WR=hromW1Cvamkn7O-g8fPY2m=_7muJ0oA@mail.gmail.com	9 years ago
Tom Lane	72daabc7a3	Disallow pushing volatile quals past set-returning functions. ... Pushing an upper-level restriction clause into an unflattened subquery-in-FROM is okay when the subquery contains no SRFs in its targetlist, or when it does but the SRFs are unreferenced by the clause *and the clause is not volatile*. Otherwise, we're changing the number of times the clause is evaluated, which is bad for volatile quals, and possibly changing the result, since a volatile qual might succeed for some SRF output rows and not others despite not referencing any of the changing columns. (Indeed, if the clause is something like "random() > 0.5", the user is probably expecting exactly that behavior.) We had most of these restrictions down, but not the one about the upper clause not being volatile. Fix that, and add a regression test to illustrate the expected behavior. Although this is definitely a bug, it doesn't seem like back-patch material, since possibly some users don't realize that the broken behavior is broken and are relying on what happens now. Also, while the added test is quite cheap in the wake of commit a4c35ea1c, it would be much more expensive (or else messier) in older branches. Per report from Tom van Tilburg. Discussion: <CAP3PPDiucxYCNev52=YPVkrQAPVF1C5PFWnrQPT7iMzO1fiKFQ@mail.gmail.com>	10 years ago
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.	11 years ago
Tom Lane	b62f94c603	Allow simplification of EXISTS() subqueries containing LIMIT. ... The locution "EXISTS(SELECT ... LIMIT 1)" seems to be rather common among people who don't realize that the database already performs optimizations equivalent to putting LIMIT 1 in the sub-select. Unfortunately, this was actually making things worse, because it prevented us from optimizing such EXISTS clauses into semi or anti joins. Teach simplify_EXISTS_query() to suppress constant-positive LIMIT clauses. That fixes the semi/anti-join case, and may help marginally even for cases that have to be left as sub-SELECTs. Marti Raudsepp, reviewed by David Rowley	12 years ago
Tom Lane	d685814835	Fix bug with whole-row references to append subplans. ... ExecEvalWholeRowVar incorrectly supposed that it could "bless" the source TupleTableSlot just once per query. But if the input is coming from an Append (or, perhaps, other cases?) more than one slot might be returned over the query run. This led to "record type has not been registered" errors when a composite datum was extracted from a non-blessed slot. This bug has been there a long time; I guess it escaped notice because when dealing with subqueries the planner tends to expand whole-row Vars into RowExprs, which don't have the same problem. It is possible to trigger the problem in all active branches, though, as illustrated by the added regression test.	12 years ago
Tom Lane	9e2f2d7a05	Don't assume a subquery's output is unique if there's a SRF in its tlist. ... While the x output of "select x from t group by x" can be presumed unique, this does not hold for "select x, generate_series(1,10) from t group by x", because we may expand the set-returning function after the grouping step. (Perhaps that should be re-thought; but considering all the other oddities involved with SRFs in targetlists, it seems unlikely we'll change it.) Put a check in query_is_distinct_for() so it's not fooled by such cases. Back-patch to all supported branches. David Rowley	12 years ago
Tom Lane	1147035203	Disallow pushing volatile qual expressions down into DISTINCT subqueries. ... A WHERE clause applied to the output of a subquery with DISTINCT should theoretically be applied only once per distinct row; but if we push it into the subquery then it will be evaluated at each row before duplicate elimination occurs. If the qual is volatile this can give rise to observably wrong results, so don't do that. While at it, refactor a little bit to allow subquery_is_pushdown_safe to report more than one kind of restrictive condition without indefinitely expanding its argument list. Although this is a bug fix, it seems unwise to back-patch it into released branches, since it might de-optimize plans for queries that aren't giving any trouble in practice. So apply to 9.4 but not further back.	12 years ago
Tom Lane	9ec6199d18	Fix possible crash with nested SubLinks. ... An expression such as WHERE (... x IN (SELECT ...) ...) IN (SELECT ...) could produce an invalid plan that results in a crash at execution time, if the planner attempts to flatten the outer IN into a semi-join. This happens because convert_testexpr() was not expecting any nested SubLinks and would wrongly replace any PARAM_SUBLINK Params belonging to the inner SubLink. (I think the comment denying that this case could happen was wrong when written; it's certainly been wrong for quite a long time, since very early versions of the semijoin flattening logic.) Per report from Teodor Sigaev. Back-patch to all supported branches.	13 years ago
Tom Lane	7ab321404c	Fix crash in assign_collations_walker for EXISTS with empty SELECT list. ... We (I think I, actually) forgot about this corner case while coding collation resolution. Per bug #8648 from Arjen Nienhuis.	13 years ago
Tom Lane	b97ee66cc1	Make contain_volatile_functions/contain_mutable_functions look into SubLinks. ... This change prevents us from doing inappropriate subquery flattening in cases such as dangerous functions hidden inside a sub-SELECT in the targetlist of another sub-SELECT. That could result in unexpected behavior due to multiple evaluations of a volatile function, as in a recent complaint from Etienne Dube. It's been questionable from the very beginning whether these functions should look into subqueries (as noted in their comments), and this case seems to provide proof that they should. Because the new code only descends into SubLinks, not SubPlans or InitPlans, the change only affects the planner's behavior during prepjointree processing and not later on --- for example, you can still get it to use a volatile function in an indexqual if you wrap the function in (SELECT ...). That's a historical behavior, for sure, but it's reasonable given that the executor's evaluation rules for subplans don't depend on whether there are volatile functions inside them. In any case, we need to constrain the behavioral change as narrowly as we can to make this reasonable to back-patch.	13 years ago
Tom Lane	670a6c7a22	Fix grammar for subscripting or field selection from a sub-SELECT result. ... Such cases should work, but the grammar failed to accept them because of our ancient precedence hacks to convince bison that extra parentheses around a sub-SELECT in an expression are unambiguous. (Formally, they *are* ambiguous, but we don't especially care whether they're treated as part of the sub-SELECT or part of the expression. Bison cares, though.) Fix by adding a redundant-looking production for this case. This is a fine example of why fixing shift/reduce conflicts via precedence declarations is more dangerous than it looks: you can easily cause the parser to reject cases that should work. This has been wrong since commit 3db4056e22 or maybe before, and apparently some people have been working around it by inserting no-op casts. That method introduces a dump/reload hazard, as illustrated in bug #7838 from Jan Mate. Hence, back-patch to all active branches.	14 years ago
Tom Lane	4816d2ea32	Fix cross-type case in partial row matching for hashed subplans. ... When hashing a subplan like "WHERE (a, b) NOT IN (SELECT x, y FROM ...)", findPartialMatch() attempted to match rows using the hashtable's internal equality operators, which of course are for x and y's datatypes. What we need to use are the potentially cross-type operators for a=x, b=y, etc. Failure to do that leads to wrong answers or even crashes. The scope for problems is limited to cases where we have different types with compatible hash functions (else we'd not be using a hashed subplan), but for example int4 vs int8 can cause the problem. Per bug #7597 from Bo Jensen. This has been wrong since the hashed-subplan code was written, so patch all the way back.	14 years ago
Tom Lane	8e617e29aa	Fix whole-row Var evaluation to cope with resjunk columns (again). ... When a whole-row Var is reading the result of a subquery, we need it to ignore any "resjunk" columns that the subquery might have evaluated for GROUP BY or ORDER BY purposes. We've hacked this area before, in commit 68e40998d0, but that fix only covered whole-row Vars of named composite types, not those of RECORD type; and it was mighty klugy anyway, since it just assumed without checking that any extra columns in the result must be resjunk. A proper fix requires getting hold of the subquery's targetlist so we can actually see which columns are resjunk (whereupon we can use a JunkFilter to get rid of them). So bite the bullet and add some infrastructure to make that possible. Per report from Andrew Dunstan and additional testing by Merlin Moncure. Back-patch to all supported branches. In 8.3, also back-patch commit 292176a118, which for some reason I had not done at the time, but it's a prerequisite for this change.	14 years ago
Peter Eisentraut	21f1e15aaf	Unify spelling of "canceled", "canceling", "cancellation" ... We had previously (af26857a27) established the U.S. spellings as standard.	15 years ago
Tom Lane	cd1f0d04bf	Fix thinko in previous patch for optimizing EXISTS-within-EXISTS. ... When recursing after an optimization in pull_up_sublinks_qual_recurse, the available_rels value passed down must include only the relations that are in the righthand side of the new SEMI or ANTI join; it's incorrect to pull up a sub-select that refers to other relations, as seen in the added test case. Per report from BangarRaju Vadapalli. While at it, rethink the idea of recursing below a NOT EXISTS. That is essentially the same situation as pulling up ANY/EXISTS sub-selects that are in the ON clause of an outer join, and it has the same disadvantage: we'd force the two joins to be evaluated according to the syntactic nesting order, because the lower join will most likely not be able to commute with the ANTI join. That could result in having to form a rather large join product, whereas the handling of a correlated subselect is not quite that dumb. So until we can handle those cases better, #ifdef NOT_USED that case. (I think it's okay to pull up in the EXISTS/ANY cases, because SEMI joins aren't so inflexible about ordering.) Back-patch to 8.4, same as for previous patch in this area. Fortunately that patch hadn't made it into any shipped releases yet.	15 years ago
Peter Eisentraut	fc946c39ae	Remove useless whitespace at end of lines	16 years ago
Tom Lane	133924e13e	Fix potential failure when hashing the output of a subplan that produces ... a pass-by-reference datatype with a nontrivial projection step. We were using the same memory context for the projection operation as for the temporary context used by the hashtable routines in execGrouping.c. However, the hashtable routines feel free to reset their temp context at any time, which'd lead to destroying input data that was still needed. Report and diagnosis by Tao Ma. Back-patch to 8.1, where the problem was introduced by the changes that allowed us to work with "virtual" tuples instead of materializing intermediate tuple values everywhere. The earlier code looks quite similar, but it doesn't suffer the problem because the data gets copied into another context as a result of having to materialize ExecProject's output tuple.	16 years ago
Tom Lane	d4d32eefdf	Fix "cannot handle unplanned sub-select" error that can occur when a ... sub-select contains a join alias reference that expands into an expression containing another sub-select. Per yesterday's report from Merlin Moncure and subsequent off-list investigation. Back-patch to 7.4. Older versions didn't attempt to flatten sub-selects in ways that would trigger this problem.	16 years ago
Tom Lane	f685cbbac8	Fix mishandling of whole-row Vars referencing a view or sub-select. ... If such a Var appeared within a nested sub-select, we failed to translate it correctly during pullup of the view, because the recursive call to replace_rte_variables_mutator was looking for the wrong sublevels_up value. Bug was introduced during the addition of the PlaceHolderVar mechanism. Per bug #5514 from Marcos Castedo.	16 years ago
Tom Lane	4886dc92e0	Fix set_subquery_pathlist() to copy the RTE's subquery before it gets mangled ... by the planning process. This prevents the "failed to locate grouping columns" error recently reported by Dickson Guedes. That happens because planning replaces SubLinks by SubPlans in the subquery's targetlist, and exprTypmod() is smarter about the former than the latter, causing the apparent type of the subquery's output columns to change. This seems to be a deficiency we should fix in exprTypmod(), but that will be a much more invasive patch with possible side-effects elsewhere, so I'll do that only in HEAD. Back-patch to 8.3. Arguably the lack of a copying step is broken/dangerous all the way back, but in the absence of known problems I'll refrain from making the older branches pay the extra cost. (The reason this particular symptom didn't appear before is that exprTypmod() wasn't smart about SubLinks either, until 8.3.)	17 years ago
Tom Lane	772a6d45ef	Fix mis-calculation of extParam/allParam sets for plan nodes, as seen in ... bug #4290. The fundamental bug is that masking extParam by outer_params, as finalize_plan had been doing, caused us to lose the information that an initPlan depended on the output of a sibling initPlan. On reflection the best thing to do seemed to be not to try to adjust outer_params for this case but get rid of it entirely. The only thing it was really doing for us was to filter out param IDs associated with SubPlan nodes, and that can be done (with greater accuracy) while processing individual SubPlan nodes in finalize_primnode. This approach was vindicated by the discovery that the masking method was hiding a second bug: SS_finalize_plan failed to remove extParam bits for initPlan output params that were referenced in the main plan tree (it only got rid of those referenced by other initPlans). It's not clear that this caused any real problems, given the limited use of extParam by the executor, but it's certainly not what was intended. I originally thought that there was also a problem with needing to include indirect dependencies on external params in initPlans' param sets, but it turns out that the executor handles this correctly so long as the depended-on initPlan is earlier in the initPlans list than the one using its output. That seems a bit of a fragile assumption, but it is true at the moment, so I just documented it in some code comments rather than making what would be rather invasive changes to remove the assumption. Back-patch to 8.1. Previous versions don't have the case of initPlans referring to other initPlans' outputs, so while the existing logic is still questionable for them, there are not any known bugs to be fixed. So I'll refrain from changing them for now.	18 years ago
Tom Lane	ff673f558a	Fix convert_IN_to_join to properly handle the case where the subselect's ... output is not of the same type that's needed for the IN comparison (ie, where the parser inserted an implicit coercion above the subselect result). We should record the coerced expression, not just a raw Var referencing the subselect output, as the quantity that needs to be unique-ified if we choose to implement the IN as Unique followed by a plain join. As of 8.3 this error was causing crashes, as seen in bug #4113 from Javier Hernandez, because the executor was being told to hash or sort the raw subselect output column using operators appropriate to the coerced type. In prior versions there was no crash because the executor chose the hash or sort operators for itself based on the column type it saw. However, that's still not really right, because what's unique for one data type might not be unique for another. In corner cases we could get multiple outputs of a row that should appear only once, as demonstrated by the regression test case included in this commit. However, this patch doesn't apply cleanly to 8.2 or before, and the code involved has shifted enough over time that I'm hesitant to try to back-patch. Given the lack of complaints from the field about such corner cases, I think the bug may not be important enough to risk breaking other things with a back-patch.	18 years ago
Tom Lane	d514ea3fda	Fix an old thinko in SS_make_initplan_from_plan, which is used when optimizing ... a MIN or MAX aggregate call into an indexscan: the initplan is being made at the current query nesting level and so we shouldn't increment query_level. Though usually harmless, this mistake could lead to bogus "plan should not reference subplan's variable" failures on complex queries. Per bug report from David Sanchez i Gregori.	19 years ago
Tom Lane	31edbadf4a	Downgrade implicit casts to text to be assignment-only, except for the ones ... from the other string-category types; this eliminates a lot of surprising interpretations that the parser could formerly make when there was no directly applicable operator. Create a general mechanism that supports casts to and from the standard string types (text,varchar,bpchar) for *every* datatype, by invoking the datatype's I/O functions. These new casts are assignment-only in the to-string direction, explicit-only in the other, and therefore should create no surprising behavior. Remove a bunch of thereby-obsoleted datatype-specific casting functions. The "general mechanism" is a new expression node type CoerceViaIO that can actually convert between *any* two datatypes if their external text representations are compatible. This is more general than needed for the immediate feature, but might be useful in plpgsql or other places in future. This commit does nothing about the issue that applying the concatenation operator || to non-text types will now fail, often with strange error messages due to misinterpreting the operator as array concatenation. Since it often (not always) worked before, we should either make it succeed or at least give a more user-friendly error; but details are still under debate. Peter Eisentraut and Tom Lane	19 years ago
Tom Lane	19ff959bff	Fix problems with rewriter failing to set Query.hasSubLinks when inserting ... a SubLink expression into a rule query. Pre-8.1 we essentially did this unconditionally; 8.1 tries to do it only when needed, but was missing a couple of cases. Per report from Kyle Bateman. Add some regression test cases covering this area.	21 years ago
Tom Lane	0182951bc8	Fix overenthusiastic optimization of 'x IN (SELECT DISTINCT ...)' and related ... cases: we can't just consider whether the subquery's output is unique on its own terms, we have to check whether the set of output columns we are going to use will be unique. Per complaint from Luca Pireddu and test case from Michael Fuhr.	21 years ago
Tom Lane	4171bb869f	Detect overflow in integer arithmetic operators (integer, smallint, and ... bigint variants). Clean up some inconsistencies in error message wording. Fix scanint8 to allow trailing whitespace in INT64_MIN case. Update int8-exp-three-digits.out, which seems to have been ignored by the last couple of people to modify the int8 regression test, and remove int8-exp-three-digits-win32.out which is thereby exposed as redundant.	22 years ago