aboutsummaryrefslogtreecommitdiff
path: root/src/backend/executor
Commit message (Collapse)AuthorAge
* Fix parsetree representation of XMLTABLE(XMLNAMESPACES(DEFAULT ...)).Tom Lane2018-09-17
| | | | | | | | | | | | | | | | | | | | | | | The original coding for XMLTABLE thought it could represent a default namespace by a T_String Value node with a null string pointer. That's not okay, though; in particular outfuncs.c/readfuncs.c are not on board with such a representation, meaning you'll get a null pointer crash if you try to store a view or rule containing this construct. To fix, change the parsetree representation so that we have a NULL list element, instead of a bogus Value node. This isn't really a functional limitation since default XML namespaces aren't yet implemented in the executor; you'd just get "DEFAULT namespace is not supported" anyway. But crashes are not nice, so back-patch to v10 where this syntax was added. Ordinarily we'd consider a parsetree representation change to be un-backpatchable; but since existing releases would crash on the way to storing such constructs, there can't be any existing views/rules to be incompatible with. Per report from Andrey Lepikhov. Discussion: https://postgr.es/m/3690074f-abd2-56a9-144a-aa5545d7a291@postgrespro.ru
* Fix failure with initplans used conditionally during EvalPlanQual rechecks.Tom Lane2018-09-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The EvalPlanQual machinery assumes that any initplans (that is, uncorrelated sub-selects) used during an EPQ recheck would have already been evaluated during the main query; this is implicit in the fact that execPlan pointers are not copied into the EPQ estate's es_param_exec_vals. But it's possible for that assumption to fail, if the initplan is only reached conditionally. For example, a sub-select inside a CASE expression could be reached during a recheck when it had not been previously, if the CASE test depends on a column that was just updated. This bug is old, appearing to date back to my rewrite of EvalPlanQual in commit 9f2ee8f28, but was not detected until Kyle Samson reported a case. To fix, force all not-yet-evaluated initplans used within the EPQ plan subtree to be evaluated at the start of the recheck, before entering the EPQ environment. This could be inefficient, if such an initplan is expensive and goes unused again during the recheck --- but that's piling one layer of improbability atop another. It doesn't seem worth adding more complexity to prevent that, at least not in the back branches. It was convenient to use the new-in-v11 ExecEvalParamExecParams function to implement this, but I didn't like either its name or the specifics of its API, so revise that. Back-patch all the way. Rather than rewrite the patch to avoid depending on bms_next_member() in the oldest branches, I chose to back-patch that function into 9.4 and 9.3. (This isn't the first time back-patches have needed that, and it exhausted my patience.) I also chose to back-patch some test cases added by commits 71404af2a and 342a1ffa2 into 9.4 and 9.3, so that the 9.x versions of eval-plan-qual.spec are all the same. Andrew Gierth diagnosed the problem and contributed the added test cases, though the actual code changes are by me. Discussion: https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com
* Move PartitionDispatchData struct definition to execPartition.cAlvaro Herrera2018-09-14
| | | | | | | There's no reason to expose the struct definition, so don't. Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Discussion: https://postgr.es/m/d3fa24c1-bc65-7133-81df-6474387ccc4f@lab.ntt.co.jp
* Save/restore SPI's global variables in SPI_connect() and SPI_finish().Tom Lane2018-09-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch removes two sources of interference between nominally independent functions when one SPI-using function calls another, perhaps without knowing that it does so. Chapman Flack pointed out that xml.c's query_to_xml_internal() expects SPI_tuptable and SPI_processed to stay valid across datatype output function calls; but it's possible that such a call could involve re-entrant use of SPI. It seems likely that there are similar hazards elsewhere, if not in the core code then in third-party SPI users. Previously SPI_finish() reset SPI's API globals to zeroes/nulls, which would typically make for a crash in such a situation. Restoring them to the values they had at SPI_connect() seems like a considerably more useful behavior, and it still meets the design goal of not leaving any dangling pointers to tuple tables of the function being exited. Also, cause SPI_connect() to reset these variables to zeroes/nulls after saving them. This prevents interference in the opposite direction: it's possible that a SPI-using function that's only ever been tested standalone contains assumptions that these variables start out as zeroes. That was the case as long as you were the outermost SPI user, but not so much for an inner user. Now it's consistent. Report and fix suggestion by Chapman Flack, actual patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net
* Set scan direction appropriately for SubPlans (bug #15336)Andrew Gierth2018-08-17
| | | | | | | | | | | | | | | | | | | | 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
* Fix executor prune failure when plan already prunedAlvaro Herrera2018-08-16
| | | | | | | | | | | In a multi-layer partitioning setup, if at plan time all the sub-partitions are pruned but the intermediate one remains, the executor later throws a spurious error that there's nothing to prune. That is correct, but there's no reason to throw an error. Therefore, don't. Reported-by: Andreas Seltenreich <seltenreich@gmx.de> Author: David Rowley <david.rowley@2ndquadrant.com> Discussion: https://postgr.es/m/87in4h98i0.fsf@ansel.ydns.eu
* Adjust comment atop ExecShutdownNode.Amit Kapila2018-08-13
| | | | | | | | | After commits a315b967cc and b805b63ac2, part of the comment atop ExecShutdownNode is redundant. Adjust it. Author: Amit Kapila Backpatch-through: 10 where both the mentioned commits are present. Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info
* Prohibit shutting down resources if there is a possibility of back up.Amit Kapila2018-08-13
| | | | | | | | | | | | | | | Currently, we release the asynchronous resources as soon as it is evident that no more rows will be needed e.g. when a Limit is filled. This can be problematic especially for custom and foreign scans where we can scan backward. Fix that by disallowing the shutting down of resources in such cases. Reported-by: Robert Haas Analysed-by: Robert Haas and Amit Kapila Author: Amit Kapila Reviewed-by: Robert Haas Backpatch-through: 9.6 where this code was introduced Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info
* Avoid query-lifetime memory leaks in XMLTABLE (bug #15321)Andrew Gierth2018-08-13
| | | | | | | | | | | | | | | | | | | Multiple calls to XMLTABLE in a query (e.g. laterally applying it to a table with an xml column, an important use-case) were leaking large amounts of memory into the per-query context, blowing up memory usage. Repair by reorganizing memory context usage in nodeTableFuncscan; use the usual per-tuple context for row-by-row evaluations instead of perValueCxt, and use the explicitly created context -- renamed from perValueCxt to perTableCxt -- for arguments and state for each individual table-generation operation. Backpatch to PG10 where this code was introduced. Original report by IRC user begriffs; analysis and patch by me. Reviewed by Tom Lane and Pavel Stehule. Discussion: https://postgr.es/m/153394403528.10284.7530399040974170549@wrigleys.postgresql.org
* Revert changes in execMain.c from commit 16828d5c0273bAndrew Dunstan2018-08-10
| | | | | | | | | | These changes were put in at some stage of the development process, but are unnecessary and should not have made it into the final patch. Mea culpa. Per gripe from Andreas Freund Backpatch to REL_11_STABLE
* Fix buffer usage stats for parallel nodes.Amit Kapila2018-08-03
| | | | | | | | | | | | | | | | | | | | | | | | The buffer usage stats is accounted only for the execution phase of the node. For Gather and Gather Merge nodes, such stats are accumulated at the time of shutdown of workers which is done after execution of node due to which we missed to account them for such nodes. Fix it by treating nodes as running while we shut down them. We can also miss accounting for a Limit node when Gather or Gather Merge is beneath it, because it can finish the execution before shutting down such nodes. So we allow a Limit node to shut down the resources before it completes the execution. In the passing fix the gather node code to allow workers to shut down as soon as we find that all the tuples from the workers have been retrieved. The original code use to do that, but is accidently removed by commit 01edb5c7fc. Reported-by: Adrien Nayrat Author: Amit Kapila and Robert Haas Reviewed-by: Robert Haas and Andres Freund Backpatch-through: 9.6 where this code was introduced Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info
* Match the buffer usage tracking for leader and worker backends.Amit Kapila2018-08-03
| | | | | | | | | | | | | In the leader backend, we don't track the buffer usage for ExecutorStart phase whereas in worker backend we track it for ExecutorStart phase as well. This leads to different value for buffer usage stats for the parallel and non-parallel query. Change the code so that worker backend also starts tracking buffer usage after ExecutorStart. Author: Amit Kapila and Robert Haas Reviewed-by: Robert Haas and Andres Freund Backpatch-through: 9.6 where this code was introduced Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info
* Fix run-time partition pruning for appends with multiple source rels.Tom Lane2018-08-01
| | | | | | | | | | | | | | | | | | | | | | The previous coding here supposed that if run-time partitioning applied to a particular Append/MergeAppend plan, then all child plans of that node must be members of a single partitioning hierarchy. This is totally wrong, since an Append could be formed from a UNION ALL: we could have multiple hierarchies sharing the same Append, or child plans that aren't part of any hierarchy. To fix, restructure the related plan-time and execution-time data structures so that we can have a separate list or array for each partitioning hierarchy. Also track subplans that are not part of any hierarchy, and make sure they don't get pruned. Per reports from Phil Florent and others. Back-patch to v11, since the bug originated there. David Rowley, with a lot of cosmetic adjustments by me; thanks also to Amit Langote for review. Discussion: https://postgr.es/m/HE1PR03MB17068BB27404C90B5B788BCABA7B0@HE1PR03MB1706.eurprd03.prod.outlook.com
* Fix per-tuple memory leak in partition tuple routingAlvaro Herrera2018-08-01
| | | | | | | | | | | Some operations were being done in a longer-lived memory context, causing intra-query leaks. It's not noticeable unless you're doing a large COPY, but if you are, it eats enough memory to cause a problem. Co-authored-by: Kohei KaiGai <kaigai@heterodb.com> Co-authored-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAOP8fzYtVFWZADq4c=KoTAqgDrHWfng+AnEPEZccyxqxPVbbWQ@mail.gmail.com
* Verify range bounds to bms_add_range when necessaryAlvaro Herrera2018-07-30
| | | | | | | | Now that the bms_add_range boundary protections are gone, some alternative ones are needed in a few places. Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Discussion: https://postgr.es/m/3437ccf8-a144-55ff-1e2f-fc16b437823b@lab.ntt.co.jp
* LLVMJIT: Release JIT context after running ExprContext shutdown callbacks.Andres Freund2018-07-25
| | | | | | | | | | | | Due to inlining it previously was possible that an ExprContext's shutdown callback pointed to a JITed function. As the JIT context previously was shut down before the shutdown callbacks were called, that could lead to segfaults. Fix the ordering. Reported-By: Dmitry Dolgov Author: Andres Freund Discussion: https://postgr.es/m/CA+q6zcWO7CeAJtHBxgcHn_hj+PenM=tvG0RJ93X1uEJ86+76Ug@mail.gmail.com Backpatch: 11-, where JIT compilation was added
* Rephrase a few comments for clarity.Heikki Linnakangas2018-07-19
| | | | | | | | I was confused by what "intended to be parallel serially" meant, until Robert Haas and David G. Johnston explained it. Rephrase the comment to make it more clear, using David's suggested wording. Discussion: https://www.postgresql.org/message-id/1fec9022-41e8-e484-70ce-2179b08c2092%40iki.fi
* Fix misc typos, mostly in comments.Heikki Linnakangas2018-07-18
| | | | | | | | A collection of typos I happened to spot while reading code, as well as grepping for common mistakes. Backpatch to all supported versions, as applicable, to avoid conflicts when backporting other commits in the future.
* Allow using the updated tuple while moving it to a different partition.Amit Kapila2018-07-12
| | | | | | | | | | | | | | | | | | An update that causes the tuple to be moved to a different partition was missing out on re-constructing the to-be-updated tuple, based on the latest tuple in the update chain. Instead, it's simply deleting the latest tuple and inserting a new tuple in the new partition based on the old tuple. Commit 2f17844104 didn't consider this case, so some of the updates were getting lost. In passing, change the argument order for output parameter in ExecDelete and add some commentary about it. Reported-by: Pavan Deolasee Author: Amit Khandekar, with minor changes by me Reviewed-by: Dilip Kumar, Amit Kapila and Alvaro Herrera Backpatch-through: 11 Discussion: https://postgr.es/m/CAJ3gD9fRbEzDqdeDq1jxqZUb47kJn+tQ7=Bcgjc8quqKsDViKQ@mail.gmail.com
* Fix bugs with degenerate window ORDER BY clauses in GROUPS/RANGE mode.Tom Lane2018-07-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | nodeWindowAgg.c failed to cope with the possibility that no ordering columns are defined in the window frame for GROUPS mode or RANGE OFFSET mode, leading to assertion failures or odd errors, as reported by Masahiko Sawada and Lukas Eder. In RANGE OFFSET mode, an ordering column is really required, so add an Assert about that. In GROUPS mode, the code would work, except that the node initialization code wasn't in sync with the execution code about when to set up tuplestore read pointers and spare slots. Fix the latter for consistency's sake (even though I think the changes described below make the out-of-sync cases unreachable for now). Per SQL spec, a single ordering column is required for RANGE OFFSET mode, and at least one ordering column is required for GROUPS mode. The parser enforced the former but not the latter; add a check for that. We were able to reach the no-ordering-column cases even with fully spec compliant queries, though, because the planner would drop partitioning and ordering columns from the generated plan if they were redundant with earlier columns according to the redundant-pathkey logic, for instance "PARTITION BY x ORDER BY y" in the presence of a "WHERE x=y" qual. While in principle that's an optimization that could save some pointless comparisons at runtime, it seems unlikely to be meaningful in the real world. I think this behavior was not so much an intentional optimization as a side-effect of an ancient decision to construct the plan node's ordering-column info by reverse-engineering the PathKeys of the input path. If we give up redundant-column removal then it takes very little code to generate the plan node info directly from the WindowClause, ensuring that we have the expected number of ordering columns in all cases. (If anyone does complain about this, the planner could perhaps be taught to remove redundant columns only when it's safe to do so, ie *not* in RANGE OFFSET mode. But I doubt anyone ever will.) With these changes, the WindowAggPath.winpathkeys field is not used for anything anymore, so remove it. The test cases added here are not actually very interesting given the removal of the redundant-column-removal logic, but they would represent important corner cases if anyone ever tries to put that back. Tom Lane and Masahiko Sawada. Back-patch to v11 where RANGE OFFSET and GROUPS modes were added. Discussion: https://postgr.es/m/CAD21AoDrWqycq-w_+Bx1cjc+YUhZ11XTj9rfxNiNDojjBx8Fjw@mail.gmail.com Discussion: https://postgr.es/m/153086788677.17476.8002640580496698831@wrigleys.postgresql.org
* Fix assert in nested SQL procedure callPeter Eisentraut2018-07-06
| | | | | | | | | | | | | | | | | When executing CALL in PL/pgSQL, we need to set a snapshot before invoking the to-be-called procedure. Otherwise, the to-be-called procedure might end up running without a snapshot. For LANGUAGE SQL procedures, this would result in an assertion failure. (For most other languages, this is usually not a problem, because those use SPI and SPI sets snapshots in most cases.) Setting the snapshot restores the behavior of how CALL worked when it was handled as a generic SQL statement in PL/pgSQL (exec_stmt_execsql()). This change revealed another problem: In SPI_commit(), we popped the active snapshot before committing the transaction, to avoid "snapshot %p still active" errors. However, there is no particular reason why only at most one snapshot should be on the stack. So change this to pop all active snapshots instead of only one.
* pgindent run prior to branchingAndrew Dunstan2018-06-30
|
* Fix thinko in comments.Amit Kapila2018-06-27
| | | | | | | | | A slot can not be stored in a tuple but it's vice versa. Reported-by: Ashutosh Bapat Author: Ashutosh Bapat Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAFjFpRcHhNhXdegyJv3KKDWrwO1_NB_KYZM_ZSDeMOZaL1A5jQ@mail.gmail.com
* Fix some ill-chosen names for globally-visible partition support functions.Tom Lane2018-06-13
| | | | | "compute_hash_value" is particularly gratuitously generic, but IMO all of these ought to have names clearly related to partitioning.
* Fix up run-time partition pruning's use of relcache's partition data.Tom Lane2018-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous coding saved pointers into the partitioned table's relcache entry, but then closed the relcache entry, causing those pointers to nominally become dangling. Actual trouble would be seen in the field only if a relcache flush occurred mid-query, but that's hardly out of the question. While we could fix this by copying all the data in question at query start, it seems better to just hold the relcache entry open for the whole query. While at it, improve the handling of support-function lookups: do that once per query not once per pruning test. There's still something to be desired here, in that we fail to exploit the possibility of caching data across queries in the fn_extra fields of the relcache's FmgrInfo structs, which could happen if we just used those structs in-place rather than copying them. However, combining that with the possibility of per-query lookups of cross-type comparison functions seems to require changes in the APIs of a lot of the pruning support functions, so it's too invasive to consider as part of this patch. A win would ensue only for complex partition key data types (e.g. arrays), so it may not be worth the trouble. David Rowley and Tom Lane Discussion: https://postgr.es/m/17850.1528755844@sss.pgh.pa.us
* Improve ExecFindInitialMatchingSubPlans's subplan renumbering logic.Tom Lane2018-06-11
| | | | | | | | | | | | We don't need two passes if we scan child partitions before parents, as that way the children's present_parts are up to date before they're needed. I (tgl) think there's actually a bug being fixed here, for the case of an intermediate partitioned table with no direct leaf children, but haven't attempted to construct a test case to prove it. David Rowley Discussion: https://postgr.es/m/CAKJS1f-6GODRNgEtdPxCnAPme2h2hTztB6LmtfdmcYAAOE0kQg@mail.gmail.com
* Don't needlessly check the partition contraint twiceAlvaro Herrera2018-06-11
| | | | | | | | | | | | | | | | | | | | | Starting with commit f0e44751d717, ExecConstraints was in charge of running the partition constraint; commit 19c47e7c8202 modified that so that caller could request to skip that checking depending on some conditions, but that commit and 15ce775faa42 together introduced a small bug there which caused ExecInsert to request skipping the constraint check but have this not be honored -- in effect doing the check twice. This could have been fixed in a very small patch, but on further analysis of the involved function and its callsites, it turns out to be simpler to give the responsibility of checking the partition constraint fully to the caller, and return ExecConstraints to its original (pre-partitioning) shape where it only checked tuple descriptor-related constraints. Each caller must do partition constraint checking on its own schedule, which is more convenient after commit 2f178441044 anyway. Reported-by: David Rowley Author: David Rowley, Álvaro Herrera Reviewed-by: Amit Langote, Amit Khandekar, Simon Riggs Discussion: https://postgr.es/m/CAKJS1f8w8+awsxgea8wt7_UX8qzOQ=Tm1LD+U1fHqBAkXxkW2w@mail.gmail.com
* Make new error code name match SQL standard more closelyPeter Eisentraut2018-06-11
| | | | Discussion: https://www.postgresql.org/message-id/dff3d555-bea4-ac24-29b2-29521b9d08e8%402ndquadrant.com
* Assorted cosmetic cleanup of run-time-partition-pruning code.Tom Lane2018-06-10
| | | | | | | | | | Use "subplan" rather than "subnode" to refer to the child plans of a partitioning Append; this seems a bit more specific and hence clearer. Improve assorted comments. No non-cosmetic changes. David Rowley and Tom Lane Discussion: https://postgr.es/m/CAFj8pRBjrufA3ocDm8o4LPGNye9Y+pm1b9kCwode4X04CULG3g@mail.gmail.com
* Relocate partition pruning structs to a saner place.Tom Lane2018-06-10
| | | | | | | | | | | | | | | | These struct definitions were originally dropped into primnodes.h, which is a poor choice since that's mainly intended for primitive expression node types; these are not in that category. What they are is auxiliary info in Plan trees, so move them to plannodes.h. For consistency, also relocate some related code that was apparently placed with the aid of a dartboard. There's no interesting code changes in this commit, just reshuffling. David Rowley and Tom Lane Discussion: https://postgr.es/m/CAFj8pRBjrufA3ocDm8o4LPGNye9Y+pm1b9kCwode4X04CULG3g@mail.gmail.com
* Improve run-time partition pruning to handle any stable expression.Tom Lane2018-06-10
| | | | | | | | | | | | | The initial coding of the run-time-pruning feature only coped with cases where the partition key(s) are compared to Params. That is a bit silly; we can allow it to work with any non-Var-containing stable expression, as long as we take special care with expressions containing PARAM_EXEC Params. The code is hardly any longer this way, and it's considerably clearer (IMO at least). Per gripe from Pavel Stehule. David Rowley, whacked around a bit by me Discussion: https://postgr.es/m/CAFj8pRBjrufA3ocDm8o4LPGNye9Y+pm1b9kCwode4X04CULG3g@mail.gmail.com
* Limit Parallel Hash's bucket array to MaxAllocSize.Thomas Munro2018-06-10
| | | | | | | | | | | | Make sure that we don't exceed MaxAllocSize when increasing the number of buckets. Perhaps later we'll remove that limit and use DSA_ALLOC_HUGE, but for now just prevent further increases like the non-parallel code. This change avoids the error from bug report #15225. Author: Thomas Munro Reviewed-By: Tom Lane Reported-by: Frits Jalvingh Discussion: https://postgr.es/m/152802081668.26724.16985037679312485972%40wrigleys.postgresql.org
* Fix typoPeter Eisentraut2018-06-08
|
* Fix obsolete comment.Heikki Linnakangas2018-06-07
| | | | | | | | The 'orig_slot' argument was removed in commit c0a8ae7be392, but that commit forgot to update the comment. Author: Amit Langote Discussion: https://www.postgresql.org/message-id/194ac4bf-7b4a-c887-bf26-bc1a85ea995a@lab.ntt.co.jp
* Improve spelling of new FINALFUNC_MODIFY aggregate attribute.Tom Lane2018-05-21
| | | | | | | | I'd used SHARABLE as a value originally, but Peter Eisentraut points out that dictionaries agree that SHAREABLE is the preferred spelling. Run around and change that before it's too late. Discussion: https://postgr.es/m/d2e1afd4-659c-50d6-1b20-7cfd3675e909@2ndquadrant.com
* Further adjust comment in get_partition_dispatch_recurse.Robert Haas2018-05-18
| | | | | | | | | In editing 09b12d52db1cf1a4c72d876f3fb6c9d06919e51a I made it wrong; fix that and try to more clearly explain the situation. Patch by me, reviewed by David Rowley and Amit Langote Discussion: http://postgr.es/m/CA+TgmobAq+mA5hzm0a5OS38qQY5758DDDGqa3sBJN4hvir-H9w@mail.gmail.com
* Improve comment in get_partition_dispatch_recurse.Robert Haas2018-05-16
| | | | | | David Rowley, reviewed by Amit Langote, and revised a bit by me. Discussion: http://postgr.es/m/CAKJS1f9yyimYyFzbHM4EwE+tkj4jvrHqSH0H4S4Kbas=UFpc9Q@mail.gmail.com
* Fix type checking for support functions of parallel VARIADIC aggregates.Tom Lane2018-05-15
| | | | | | | | | | | | | | | | | | | | | The impact of VARIADIC on the combine/serialize/deserialize support functions of an aggregate wasn't thought through carefully. There is actually no impact, because variadicity isn't passed through to these functions (and it doesn't seem like it would need to be). However, lookup_agg_function was mistakenly told to check things as though it were passed through. The net result was that it was impossible to declare an aggregate that had both VARIADIC input and parallelism support functions. In passing, fix a runtime check in nodeAgg.c for the combine function's strictness to make its error message agree with the creation-time check. The previous message was actually backwards, and it doesn't seem like there's a good reason to have two versions of this message text anyway. Back-patch to 9.6 where parallel aggregation was introduced. Alexey Bashtanov; message fix by me Discussion: https://postgr.es/m/f86dde87-fef4-71eb-0480-62754aaca01b@imap.cc
* Fix SPI error cleanup and memory leakPeter Eisentraut2018-05-03
| | | | | | | | | | | | Since the SPI stack has been moved from TopTransactionContext to TopMemoryContext, setting _SPI_stack to NULL in AtEOXact_SPI() leaks memory. In fact, we don't need to do that anymore: We just leave the allocated stack around for the next SPI use. Also, refactor the SPI cleanup so that it is run both at transaction end and when returning to the main loop on an exception. The latter is necessary when a procedure calls a COMMIT or ROLLBACK command that itself causes an error.
* Clean up warnings from -Wimplicit-fallthrough.Tom Lane2018-05-01
| | | | | | | | | | | | | | | | | | | | | | | | | Recent gcc can warn about switch-case fall throughs that are not explicitly labeled as intentional. This seems like a good thing, so clean up the warnings exposed thereby by labeling all such cases with comments that gcc will recognize. In files that already had one or more suitable comments, I generally matched the existing style of those. Otherwise I went with /* FALLTHROUGH */, which is one of the spellings approved at the more-restrictive-than-default level -Wimplicit-fallthrough=4. (At the default level you can also spell it /* FALL ?THRU */, and it's not picky about case. What you can't do is include additional text in the same comment, so some existing comments containing versions of this aren't good enough.) Testing with gcc 8.0.1 (Fedora 28's current version), I found that I also had to put explicit "break"s after elog(ERROR) or ereport(ERROR); apparently, for this purpose gcc doesn't recognize that those don't return. That seems like possibly a gcc bug, but it's fine because in most places we did that anyway; so this amounts to a visit from the style police. Discussion: https://postgr.es/m/15083.1525207729@sss.pgh.pa.us
* Fix interaction of foreign tuple routing with remote triggers.Robert Haas2018-05-01
| | | | | | | | | | | | | | | | | | | | | | | | Without these fixes, changes to the inserted tuple made by remote triggers are ignored when building local RETURNING tuples. In the core code, call ExecInitRoutingInfo at a later point from within ExecInitPartitionInfo so that the FDW callback gets invoked after the returning list has been built. But move CheckValidResultRel out of ExecInitRoutingInfo so that it can happen at an earlier stage. In postgres_fdw, refactor assorted deparsing functions to work with the RTE rather than the PlannerInfo, which saves us having to construct a fake PlannerInfo in cases where we don't have a real one. Then, we can pass down a constructed RTE that yields the correct deparse result when no real one exists. Unfortunately, this necessitates a hack that understands how the core code manages RT indexes for update tuple routing, which is ugly, but we don't have a better idea right now. Original report, analysis, and patch by Etsuro Fujita. Heavily refactored by me. Then worked over some more by Amit Langote. Discussion: http://postgr.es/m/5AD4882B.10002@lab.ntt.co.jp
* Post-feature-freeze pgindent run.Tom Lane2018-04-26
| | | | Discussion: https://postgr.es/m/15719.1523984266@sss.pgh.pa.us
* Update ExecInitPartitionInfo commentAlvaro Herrera2018-04-24
| | | | | | | | | Remove the words "if not already done." This obsolete wording corresponds to an early development version of what became edd44738bc8. Author: Etsuro Fujita Reviewed-by: Amit Langote Discussion: https://postgr.es/m/5ADF117B.5030606@lab.ntt.co.jp
* Initialize ExprStates once in run-time partition pruningAlvaro Herrera2018-04-24
| | | | | | | | | | | Instead of doing ExecInitExpr every time a Param needs to be evaluated in run-time partition pruning, do it once during run-time pruning set-up and cache the exprstate in PartitionPruneContext, saving a lot of work. Author: David Rowley Reviewed-by: Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f8-x+q-90QAPDu_okhQBV4DPEtPz8CJ=m0940GyT4DA4w@mail.gmail.com
* Add missing "static" marker.Tom Lane2018-04-21
| | | | Per pademelon.
* Remove quick path in ExecInitPartitionInfo for equal tupdescsAlvaro Herrera2018-04-19
| | | | | | | | | | | | I added this "optimization" on top of Amit Langote's 158b7bc6d779, but the quick path is never taken because the partition uses a different pg_type oid than its parent table (causing equalTupleDescs to return false). Changing that requires more analysis and is too considered dangerous at this point in the cycle, so revert it. We might make it work someday, but not for pg11. Discussion: https://postgr.es/m/825031be-942c-8c24-6163-13c27f217a3d@lab.ntt.co.jp
* Update Append's idea of first_partial_planAlvaro Herrera2018-04-17
| | | | | | | | | | | | It turns out that after runtime partition pruning, Append's first_partial_plan does not accurately represent partial plans to run, if any of those got pruned. This could limit participation of workers in some partial subplans, if other subplans got pruned. Fix it by keeping an index of the first valid partial subplan in the state node, determined at execnode Init time. Author: David Rowley, with cosmetic changes by me. Discussion: https://postgr.es/m/CAKJS1f8o2Yd=rOP=Et3A0FWgF+gSAOkFSU6eNhnGzTPV7nN8sQ@mail.gmail.com
* Ignore whole-rows in INSERT/CONFLICT with partitioned tablesAlvaro Herrera2018-04-16
| | | | | | | | | | | | | | | | We had an Assert() preventing whole-row expressions from being used in the SET clause of INSERT ON CONFLICT, but it seems unnecessary, given some tests, so remove it. Add a new test to exercise the case. Still at ExecInitPartitionInfo, we used map_partition_varattnos (which constructs an attribute map, then calls map_variable_attnos) using the same two relations many times in different expressions and with different parameters. Constructing the map over and over is a waste. To avoid this repeated work, construct the map once, and use map_variable_attnos() directly instead. Author: Amit Langote, per comments by me (Álvaro) Discussion: https://postgr.es/m/20180326142016.m4st5e34chrzrknk@alvherre.pgsql
* Reorganize partitioning codeAlvaro Herrera2018-04-14
| | | | | | | | | | | | | | | | | | | | | | There's been a massive addition of partitioning code in PostgreSQL 11, with little oversight on its placement, resulting in a catalog/partition.c with poorly defined boundaries and responsibilities. This commit tries to set a couple of distinct modules to separate things a little bit. There are no code changes here, only code movement. There are three new files: src/backend/utils/cache/partcache.c src/include/partitioning/partdefs.h src/include/utils/partcache.h The previous arrangement of #including catalog/partition.h almost everywhere is no more. Authors: Amit Langote and Álvaro Herrera Discussion: https://postgr.es/m/98e8d509-790a-128c-be7f-e48a5b2d8d97@lab.ntt.co.jp https://postgr.es/m/11aa0c50-316b-18bb-722d-c23814f39059@lab.ntt.co.jp https://postgr.es/m/143ed9a4-6038-76d4-9a55-502035815e68@lab.ntt.co.jp https://postgr.es/m/20180413193503.nynq7bnmgh6vs5vm@alvherre.pgsql
* Revert MERGE patchSimon Riggs2018-04-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commits d204ef63776b8a00ca220adec23979091564e465, 83454e3c2b28141c0db01c7d2027e01040df5249 and a few more commits thereafter (complete list at the end) related to MERGE feature. While the feature was fully functional, with sufficient test coverage and necessary documentation, it was felt that some parts of the executor and parse-analyzer can use a different design and it wasn't possible to do that in the available time. So it was decided to revert the patch for PG11 and retry again in the future. Thanks again to all reviewers and bug reporters. List of commits reverted, in reverse chronological order: f1464c5380 Improve parse representation for MERGE ddb4158579 MERGE syntax diagram correction 530e69e59b Allow cpluspluscheck to pass by renaming variable 01b88b4df5 MERGE minor errata 3af7b2b0d4 MERGE fix variable warning in non-assert builds a5d86181ec MERGE INSERT allows only one VALUES clause 4b2d44031f MERGE post-commit review 4923550c20 Tab completion for MERGE aa3faa3c7a WITH support in MERGE 83454e3c2b New files for MERGE d204ef6377 MERGE SQL Command following SQL:2016 Author: Pavan Deolasee Reviewed-by: Michael Paquier