Correctifs appliqués

Heikki Linnakangas pushed:

Magnus Hagander pushed:

Tom Lane pushed:

  • More code review for get_qual_for_list(). Avoid trashing the input PartitionBoundSpec; while that might be safe for current callers, it's certainly trouble waiting to happen. In the same vein, make sure that all of the result data structure is freshly palloc'd, rather than some of it being pointers into the input data structures (which we don't know the lifespans of). Simplify the logic for tacking on IS NULL or IS NOT NULL conditions some more; commit 85c2b9a15 left a lot on the table there. And rearrange the construction of the nodes into (what seems to me) a more logical order. In passing, make sure that get_qual_for_range() also returns a freshly palloc'd structure, since there's no value in having that guarantee for only one kind of partitioning. And improve some comments there. Jeevan Ladhe, with further tweaking by me Discussion:
  • Allow NumericOnly to be "+ FCONST". The NumericOnly grammar production accepted ICONST, + ICONST, - ICONST, FCONST, and - FCONST, but for some reason not + FCONST. This led to strange inconsistencies like regression=# set random_page_cost = +4; SET regression=# set random_page_cost = 4000000000; SET regression=# set random_page_cost = +4000000000; ERROR: syntax error at or near "4000000000" (because 4000000000 is too large to be an ICONST). While there's no actual functional reason to need to write a "+", if we allow it for integers it seems like we should allow it for numerics too. It's been like that forever, so back-patch to all supported branches. Discussion:
  • Prevent running pg_resetwal/pg_resetxlog against wrong-version data dirs. pg_resetwal (formerly pg_resetxlog) doesn't insist on finding a matching version number in pg_control, and that seems like an important thing to preserve since recovering from corrupt pg_control is a prime reason to need to run it. However, that means you can try to run it against a data directory of a different major version, which is at best useless and at worst disastrous. So as to provide some protection against that type of pilot error, inspect PG_VERSION at startup and refuse to do anything if it doesn't match. PG_VERSION is read-only after initdb, so it's unlikely to get corrupted, and even if it were corrupted it would be easy to fix by hand. This hazard has been there all along, so back-patch to all supported branches. Michael Paquier, with some kibitzing by me Discussion:
  • Make edge-case behavior of jsonb_populate_record match json_populate_record. json_populate_record throws an error if asked to convert a JSON scalar or array into a composite type. jsonb_populate_record was returning a record full of NULL fields instead. It seems better to make it throw an error for this case as well. Nikita Glukhov Discussion:
  • Fix thinko in JsObjectSize() macro. The macro gave the wrong answers for a JsObject with is_json == 0: it would return 1 if jsonb_cont == NULL, or if that wasn't NULL, it would return 1 for any non-zero size. We could fix that, but the only use of this macro at present is in the JsObjectIsEmpty() macro, so it seems simpler and clearer to get rid of JsObjectSize() and put corrected logic into JsObjectIsEmpty(). Thinko in commit cf35346e8, so no need for back-patch. Nikita Glukhov Discussion:
  • Fix improper quoting of format_type_be() output. Per our message style guidelines, error messages incorporating the results of format_type_be() and its siblings should not add quotes around those results, because those functions already add quotes at need. Fix a few places that hadn't gotten that memo.
  • Code review focused on new node types added by partitioning support. Fix failure to check that we got a plain Const from const-simplification of a coercion request. This is the cause of bug #14666 from Tian Bing: there is an int4 to money cast, but it's only stable not immutable (because of dependence on lc_monetary), resulting in a FuncExpr that the code was miserably unequipped to deal with, or indeed even to notice that it was failing to deal with. Add test cases around this coercion behavior. In view of the above, sprinkle the code liberally with castNode() macros, in hope of catching the next such bug a bit sooner. Also, change some functions that were randomly declared to take Node* to take more specific pointer types. And change some struct fields that were declared Node* but could be given more specific types, allowing removal of assorted explicit casts. Place PARTITION_MAX_KEYS check a bit closer to the code it's protecting. Likewise check only-one-key-for-list-partitioning restriction in a less random place. Avoid not-per-project-style usages like !strcmp(...). Fix assorted failures to avoid scribbling on the input of parse transformation. I'm not sure how necessary this is, but it's entirely silly for these functions to be expending cycles to avoid that and not getting it right. Add guards against partitioning on system columns. Put backend/nodes/ support code into an order that matches handling of these node types elsewhere. Annotate the fact that somebody added location fields to PartitionBoundSpec and PartitionRangeDatum but forgot to handle them in outfuncs.c/readfuncs.c. This is fairly harmless for production purposes (since readfuncs.c would just substitute -1 anyway) but it's still bogus. It's not worth forcing a post-beta1 initdb just to fix this, but if we have another reason to force initdb before 10.0, we should go back and clean this up. Contrariwise, somebody added location fields to PartitionElem and PartitionSpec but forgot to teach exprLocation() about them. Consolidate duplicative code in transformPartitionBound(). Improve a couple of error messages. Improve assorted commentary. Re-pgindent the files touched by this patch; this affects a few comment blocks that must have been added quite recently. Report:
  • Fix omission of locations in outfuncs/readfuncs partitioning node support. We could have limped along without this for v10, which was my intention when I annotated the bug in commit 76a3df6e5. But consensus is that it's better to fix it now and take the cost of a post-beta1 initdb (which is needed because these node types are stored in pg_class.relpartbound). Since we're forcing initdb anyway, take the opportunity to make the node type identification strings match the node struct names, instead of being randomly different from them. Discussion:
  • Sort syscache identifiers into alphabetical order. Not much point in having a convention about this if we don't enforce it. Mark Dilger Discussion:
  • Avoid -Wconversion warnings from direct use of GET_n_BYTES macros. The GET/SET_n_BYTES macros are meant to be infrastructure for the DatumGetFoo/FooGetDatum macros, which include a cast to the intended target type. Using them directly without a cast, as DatumGetFloat4 and friends previously did, can yield warnings when -Wconversion is on. This is of little significance when building Postgres proper, because there are such a huge number of such warnings in the server that nobody would think -Wconversion is of any use. But some extensions build with -Wconversion due to outside constraints. Commit 14cca1bf8 did a disservice to those extensions by moving DatumGetFloat4 et al into postgres.h, where they can now cause warnings in extension builds. To fix, use DatumGetInt32 and friends in place of the low-level macros. This is arguably a bit cleaner anyway. Chapman Flack Discussion:
  • Always use -fPIC, not -fpic, when building shared libraries with gcc. On some platforms, -fpic fails for sufficiently large shared libraries. We've mostly not hit that boundary yet, but there are some extensions such as Citus and pglogical where it's becoming a problem. A bit of research suggests that the penalty for -fPIC is small, in the single-digit-percentage range --- and there's none at all on popular platforms such as x86_64. So let's just default to -fPIC everywhere and provide one less thing for extension developers to worry about. Per complaint from Christoph Berg. Back-patch to all supported branches. (I did not bother to touch the recently-removed Makefiles for sco and unixware in the back branches, though. We'd have no way to test that it doesn't break anything on those platforms.) Discussion:
  • Fix old corner-case logic error in final_cost_nestloop(). When costing a nestloop with stop-at-first-inner-match semantics, and a non-indexscan inner path, final_cost_nestloop() wants to charge the full scan cost of the inner rel at least once, with additional scans charged at inner_rescan_run_cost which might be less. However the logic for doing this effectively assumed that outer_matched_rows is at least 1. If it's zero, which is not unlikely for a small outer rel, we ended up charging inner_run_cost plus N times inner_rescan_run_cost, as much as double the correct charge for an outer rel with only one row that we're betting won't be matched. (Unless the inner rel is materialized, in which case it has very small inner_rescan_run_cost and the cost is not so far off what it should have been.) The upshot of this was that the planner had a tendency to select plans that failed to make effective use of the stop-at-first-inner-match semantics, and that might have Materialize nodes in them even when the predicted number of executions of the Materialize subplan was only 1. This was not so obvious before commit 9c7f5229a, because the case only arose in connection with semi/anti joins where there's not freedom to reverse the join order. But with the addition of unique-inner joins, it could result in some fairly bad planning choices, as reported by Teodor Sigaev. Indeed, some of the test cases added by that commit have plans that look dubious on closer inspection, and are changed by this patch. Fix the logic to ensure that we don't charge for too many inner scans. I chose to adjust it so that the full-freight scan cost is associated with an unmatched outer row if possible, not a matched one, since that seems like a better model of what would happen at runtime. This is a longstanding bug, but given the lesser impact in back branches, and the lack of field complaints, I won't risk a back-patch. Discussion:
  • Fix <> and pattern-NOT-match estimators to handle nulls correctly. These estimators returned 1 minus the corresponding equality/match estimate, which is incorrect: we need to subtract off the fraction of nulls in the column, since those are neither equal nor not equal to the comparison value. The error only becomes obvious if the nullfrac is large, but it could be very bad in a mostly-nulls column, as reported in bug #14676 from Marko Tiikkaja. To fix the <> case, refactor eqsel() and neqsel() to call a common support routine, which can be made to account for nullfrac correctly. The pattern-match cases were already factored that way, and it was simply an oversight that patternsel() wasn't subtracting off nullfrac. neqjoinsel() has a similar problem, but since we're elsewhere discussing changing its behavior entirely, I left it alone for now. This is a very longstanding bug, but I'm hesitant to back-patch a fix for it. Given the lack of prior complaints, such cases must not come up often, so it's probably not worth the risk of destabilizing plans in stable branches. Discussion:
  • Add some missing backslash commands to psql's tab-completion knowledge. \if and related commands were overlooked here, as were \dRp and \dRs from the logical-replication patch, as was \?. While here, reformat the list to put each new first command letter on a separate line; perhaps that will limit the need to reflow the whole list when we add more commands in future. Masahiko Sawada (reformatting by me) Discussion:
  • Remove dead variables. Commit 512c7356b left a couple of variables unused except for being set. My compiler didn't whine about this, but some buildfarm members did.
  • Disallow CREATE INDEX if table is already in use in current session. If we allow this, whatever outer command has the table open will not know about the new index and may fail to update it as needed, as shown in a report from Laurenz Albe. We already had such a prohibition in place for ALTER TABLE, but the CREATE INDEX syntax missed the check. Fixing it requires an API change for DefineIndex(), which conceivably would break third-party extensions if we were to back-patch it. Given how long this problem has existed without being noticed, fixing it in the back branches doesn't seem worth that risk. Discussion:
  • #ifdef out assorted unused GEQO code. I'd always assumed that backend/optimizer/geqo/'s remarkably poor showing on code coverage metrics was because we weren't exercising it much in the regression tests. But it turns out that a good chunk of the problem is that there's a bunch of code that is physically unreachable (because the calls to it are #ifdef'd out in geqo_main.c) but is being built anyway. Making the called code have #if guards similar to the calling code saves a couple of kilobytes of executable size and should make the coverage numbers more reflective of reality. It's arguable that we should just delete all the unused recombination mechanisms altogether, but I didn't feel a need to go that far today.

Peter Eisentraut pushed:

�lvaro Herrera pushed:

Robert Haas pushed:

Andres Freund pushed:

Correctifs en attente

Ashutosh Bapat sent in a patch to add some convenience macros for declaratively partitioned tables.

Andrew Borodin sent in another revision of a patch to allow uncompressed GiST indexes.

Micha�l Paquier sent in a patch to add a pgdump TAP test for empty opclasses.

Masahiko Sawada sent in a patch to fix an off-by-one mistake in a comment in GetOldestXmin.

Thomas Munro sent in another revision of a patch to enable sharing record typmods among backends.

Dang Minh Huong sent in another revision of a patch to fix so that it recursively removes all accents rather than stopping when it has removed the first accent.

Mithun Cy sent in three more revisions of a patch to add auto_prewarm.

Jeevan Ladhe sent in four more revisions of a patch to add support for a default partition in declarative partitioning.

Kyotaro HORIGUCHI sent in a patch to fix walsender timeouts by timeout.

Fabien COELHO sent in another revision of a patch to add pgbench TAP tests.

Craig Ringer sent in a patch to allow overriding PostgresNode in get_new_node.

Masahiko Sawada sent in a patch to fix ALTER SUBSCRIPTION REFRESH and table sync.

Peter Eisentraut sent in a patch to split background worker name into type and name.

Neha Khatri sent in another revision of a patch to fix a typo in xlogfuncs.c.

David Rowley and Tom Lane traded patches to fix a performance regression in 10.

Dilip Kumar and Peter Eisentraut traded patches to fix an issue where creating subscription fails when the server is running in single user mode.

Peter Eisentraut sent in a patch to prevent parallel query in walsender.

Masahiko Sawada sent in a patch to fix signal handling in logical workers, fake tablesync worker exit when apply dies while it was waiting for it, receive invalidation messages correctly in tablesync worker, and wait for table sync worker to finish when apply worker exits.

Petr Jel�nek sent in two revisions of a patch to improve handover logic between sync and apply workers.

Beena Emerson and Rafia Sabih traded patches to add a default partition for range partitions.

Emre Hasegeli sent in another revision of a patch to refactor geometric functions and operators code, provide a header file for built-in float datatypes, use the built-in float datatype to implement geometric types, and fix obvious problems around the line datatype.

Tom Lane sent in a patch to respond to inner selectivity even without stats.

Sandro Santilli sent in a patch to ensure that the check Makefile rule imported by PGXS returns failure code when it is unsupported.

Amit Langote sent in a patch to check the partition constraint even after tuple-routing in order to ensure that BEFORE ROW triggers not violate same.

Heikki Linnakangas sent in a patch to fix double-free bug in GSS authentication.

Alexander Korotkov sent in a patch to add a SET STATISTICS option to ALTER INDEX.

Rafia Sabih and Robert Haas traded patches to improve PostgreSQL FDW abort behavior.

Fabr�zio de Royes Mello sent in a patch to ensure that pg_replslot is properly cleaned when subtransactions are happening.

Andres Freund sent in a patch to revert "Prevent panic during shutdown checkpoint", have walsenders participate in procsignal infrastructure, prevent the possibility of panics during shutdown checkpoint, and wire up query cancel interrupt for walsender backends.

Peter Eisentraut sent in a patch to fix an ALTER SUBSCRIPTION grammar ambiguity.

Petr Jel�nek sent in two revisions of a patch to use virtual transaction instead of normal ones in exported snapshots and not assign xids to logical decoding snapshots.

Amit Kapila sent in another revision of a patch to try to reattach shared memory segments under Cygwin.

Dmitry Dolgov sent in a patch to give a more useful hint when people attempt to use DROP SUBSCRIPTION to disassociate the subscription from a slot.

Thomas Munro sent in a patch to reject wCTEs with transition tables.

Tom Lane sent in a patch to fix an issue where indexes created in a BEFORE trigger were not updated during INSERT.