Correctifs appliqués

Álvaro Herrera pushed:

Tom Lane pushed:

  • Fix broken collation-aware searches in SP-GiST text opclass. spg_text_leaf_consistent() supposed that it should compare only Min(querylen, entrylen) bytes of the two strings, and then deal with any excess bytes in one string or the other by assuming the longer string is greater if the prefixes are equal. Quite aside from the fact that that's just wrong in some locales (e.g., 'ch' is not less than 'd' in cs_CZ), it also risked passing incomplete multibyte characters to strcoll(), with ensuing bad results. Instead, just pass the full strings to varstr_cmp, and let it decide what to do about unequal-length strings. Fortunately, this error doesn't imply any index corruption, it's just that searches might return the wrong set of entries. Per report from Emre Hasegeli, though this is not his patch. Thanks to Peter Geoghegan for review and discussion. This code was born broken, so back-patch to all supported branches. In HEAD, I failed to resist the temptation to do a bit of cosmetic cleanup/pgindent'ing on 710d90da1, too. Discussion:
  • Rationalize handling of array type names in bootstrap data. Formerly, turned a C array type declaration in the catalog header files into a SQL type, e.g., 'foo[]'. Along the way, turned this into '_foo' for the purpose of type lookups, but wrote 'foo[]' to postgres.bki. During bootstrap, bootscanner.l had to have a special case rule to tokenize this, and then MapArrayTypeName() would turn 'foo[]' into '_foo' one more time. This seems unnecessarily complicated, especially since nobody cares that much about the readability of postgres.bki. Instead, make convert the C declaration into '_foo' to start with, and preserve that representation of the type name throughout bootstrap data processing. Then rip out the special-case code in bootscanner.l and bootstrap.c. This changes postgres.bki to the extent that array fields are now declared like proconfig = _text , rather than proconfig = text[] , No documentation update, since the SGML docs didn't mention any of this in the first place, and it's all pretty transparent to writers of catalog header files anyway. John Naylor Discussion:
  • Simplify's data quoting rules. During the bootstrap data format conversion, it seemed important for verifiability's sake that the generated postgres.bki file stayed the same as before. That resulted in adding a bunch of ad-hoc rules about when to quote emitted data values, to match previous manual decisions that had often quoted values unnecessarily. Now that the conversion is complete, it seems fine to remove all those ad-hoc rules. The net actual effect on the current contents of postgres.bki is that some fields that had been quoted despite containing only digits or only "-" lose their unnecessary quotes. Also, now that will always quote values containing a backslash, there's no need for bootscanner.l to allow unquoted octal escapes; so simplify its production for "id" by removing that possibility. John Naylor, slightly modified by me Discussion:
  • Rationalize handling of single and double quotes in bootstrap data. Change things around so that proper quoting of values interpolated into the BKI data by initdb is the responsibility of initdb, not something we half-heartedly handle by putting double quotes into the raw BKI data. (Note: experimentation shows that it still doesn't work to put a double quote into the initial superuser username, but that's the fault of inadequate quoting while interpolating the name into SQL scripts; the BKI aspect of it works fine now.) Having done that, we can remove the special-case handling of values that look like "something" from, and instead teach it to escape double --- and single --- quotes properly. This removes the nowhere-documented need to treat those specially in the BKI source data; whatever you write will be passed through unchanged into the inserted data value, modulo Perl's rules about single-quoted strings. Add documentation explaining the (pre-existing) handling of backslashes in the BKI data. Per an earlier discussion with John Naylor. Discussion:
  • Better fix for deadlock hazard in CREATE INDEX CONCURRENTLY. Commit 54eff5311 did not account for the possibility that we'd have a transaction snapshot due to default_transaction_isolation being set high enough to require one. The transaction snapshot is enough to hold back our advertised xmin and thus risk deadlock anyway. The only way to get rid of that snap is to start a new transaction, so let's do that instead. Also throw in an assert checking that we really have gotten to a state where no xmin is being advertised. Back-patch to 9.4, like the previous commit. Discussion:
  • Improve error detection/reporting in and Clean up error messages relating to mistakes in .dat files: make sure they provide the .dat file name and line number, not the place in the Perl script that's reporting the problem. Adopt more uniform message phrasing, too. Make spit up on unrecognized field names in the input hashes. Previously, it just silently ignored such fields, which could make a misspelled field name into a very hard-to-decipher problem. (This is in, *not*, because we don't want to complain about unrecognized fields. We'd rather it silently dropped them, to facilitate removing unwanted fields after a reorganization.)
  • Fix incorrect handling of join clauses pushed into parameterized paths. In some cases a clause attached to an outer join can be pushed down into the outer join's RHS even though the clause is not degenerate --- this can happen if we choose to make a parameterized path for the RHS. If the clause ends up attached to a lower outer join, we'd misclassify it as being a "join filter" not a plain "filter" condition at that node, leading to wrong query results. To fix, teach extract_actual_join_clauses to examine each join clause's required_relids, not just its is_pushed_down flag. (The latter now seems vestigial, or at least in need of rethinking, but we won't do anything so invasive as redefining it in a bug-fix patch.) This has been wrong since we introduced parameterized paths in 9.2, though it's evidently hard to hit given the lack of previous reports. The test case used here involves a lateral function call, and I think that a lateral reference may be required to get the planner to select a broken plan; though I wouldn't swear to that. In any case, even if LATERAL is needed to trigger the bug, it still affects all supported branches, so back-patch to all. Per report from Andreas Karlsson. Thanks to Andrew Gierth for preliminary investigation. Discussion:
  • Improve consistency of comments in system catalog headers. Use the term "system catalog" rather than "system relation" in assorted places where it's clearly referring to a table rather than, say, an index. Use more natural word order in the header boilerplate, improve some of the one-liner catalog descriptions, and fix assorted random deviations from the normal boilerplate. All purely neatnik-ism, but why not. John Naylor, some additional cleanup by me Discussion:
  • Change more places to be less trusting of RestrictInfo.is_pushed_down. On further reflection, commit e5d83995e didn't go far enough: pretty much everywhere in the planner that examines a clause's is_pushed_down flag ought to be changed to use the more complicated behavior where we also check the clause's required_relids. Otherwise we could make incorrect decisions about whether, say, a clause is safe to use as a hash clause. Some (many?) of these places are safe as-is, either because they are never reached while considering a parameterized path, or because there are additional checks that would reject a pushed-down clause anyway. However, it seems smarter to just code them all the same way rather than rely on easily-broken reasoning of that sort. In support of that, invent a new macro RINFO_IS_PUSHED_DOWN that should be used in place of direct tests on the is_pushed_down flag. Like the previous patch, back-patch to all supported branches. Discussion:
  • Tweak a couple of planner APIs to save recalculating join relids. Discussion:
  • Fix race conditions when an event trigger is added concurrently with DDL. EventTriggerTableRewrite crashed if there were table_rewrite triggers present, but there had not been when the calling command started. EventTriggerDDLCommandEnd called ddl_command_end triggers if present, even if there had been no such triggers when the calling command started, which would lead to a failure in pg_event_trigger_ddl_commands. In both cases, fix by doing nothing; it's better to wait till the next command when things will be properly initialized. In passing, remove an elog(DEBUG1) call that might have seemed interesting four years ago but surely isn't today. We found this because of intermittent failures in the buildfarm. Thanks to Alvaro Herrera and Andrew Gierth for analysis. Back-patch to 9.5; some of this code exists before that, but the specific hazards we need to guard against don't. Discussion:
  • Don't run fast_default regression test in parallel with other tests. Since it sets up an event trigger that would fire on DDL done by any concurrent test script, the original scheduling is just an invitation to irreproducible test failures. (The fact that we found a bug through exactly such irreproducible test failures doesn't really change the calculus here: this script is a hazard to anything that runs in parallel with it today or might be added to that parallel group in future. No, I don't believe that the trigger is protecting itself sufficiently to avoid all possible trouble.) Discussion:
  • Test conversion of NaN between float4 and float8. Results from buildfarm member opossum suggest that this doesn't work quite right on that platform. We've seen issues with NaN support on MIPS/NetBSD before ... allegedly they fixed this stuff back in 2010, but maybe only for small values of "fixed". If, in fact, opossum fails this test then I plan to revert it; it's mainly for diagnostic purposes rather than something we'd necessarily keep long-term. I think that the failures in window.sql could be worked around with some code duplication, but I want to verify my theory about the cause first.
  • Add missing "static" marker. Per pademelon.

Tatsuo Ishii pushed:

Heikki Linnakangas pushed:

Teodor Sigaev pushed:

  • Adjust INCLUDE index truncation comments and code. Add several assertions that ensure that we're dealing with a pivot tuple without non-key attributes where that's expected. Also, remove the assertion within _bt_isequal(), restoring the v10 function signature. A similar check will be performed for the page highkey within _bt_moveright() in most cases. Also avoid dropping all objects within regression tests, to increase pg_dump test coverage for INCLUDE indexes. Rather than using infrastructure that's generally intended to be used with reference counted heap tuple descriptors during truncation, use the same function that was introduced to store flat TupleDescs in shared memory (we use a temp palloc'd buffer). This isn't strictly necessary, but seems more future-proof than the old approach. It also lets us avoid including rel.h within indextuple.c, which was arguably a modularity violation. Also, we now call index_deform_tuple() with the truncated TupleDesc, not the source TupleDesc, since that's more robust, and saves a few cycles. In passing, fix a memory leak by pfree'ing truncated pivot tuple memory during CREATE INDEX. Also pfree during a page split, just to be consistent. Refactor _bt_check_natts() to be more readable. Author: Peter Geoghegan with some editorization by me Reviewed by: Alexander Korotkov, Teodor Sigaev Discussion:
  • Handle XLOG_BTREE_META_CLEANUP in btree_desc() and btree_identify(). New WAL record XLOG_BTREE_META_CLEANUP introduced in 857f9c36 has no handling in btree_desc() and btree_identify(). This patch implements corresponding handling. Alexander Korotkov
  • Adjust _bt_insertonpg() comments. Remove an obsolete reference to the 'afteritem' argument, which was removed by commit bc292937. Add a comment that clarifies how _bt_insertonpg() indirectly handles the insertion of high key items. Author: Peter Geoghegan
  • Fix datatype for number of heap tuples during last cleanup. It appears that new fields introduced in 857f9c36 have inconsistent datatypes: BTMetaPageData.btm_last_cleanup_num_heap_tuples is of float4 type, while xl_btree_metadata.last_cleanup_num_heap_tuples is of double type. IndexVacuumInfo.num_heap_tuples, which is a source of values for both former fields is of double type. So, make both those fields in BTMetaPageData and xl_btree_metadata use float8 type in order to match the precision of the source. That shouldn't be double type, because we always use types with explicit width in WAL. Patch introduces incompatibility of on-disk format since 857f9c36 commit, but that versions never was released, so just bump catalog version to avoid possible confusion. Author: Alexander Korortkov

Stephen Frost pushed:

Peter Eisentraut pushed:

Magnus Hagander pushed:

Correctifs en attente

Liudmila Mantrova sent in a patch to fix an inaccuracy in the documentation for pg_trgrm.

Ashutosh Bapat sent in two revisions of a patch to add tests to show problem when foreign table points to a partitioned table or inheritance table on the foreign server, Error out if one iteration of non-direct DML affects more than one row on the foreign server, and implement a partial fix for the problem in non-direct UPDATE of a foreign table pointing to a partitioned table or an inheritance parent on the foreign server.

Amit Langote sent in another revision of a patch to reorganize the partitioning code.

Etsuro Fujita sent in three more revisions of a patch to fix an oddity in tuple routing to foreign partitions.

Kyotaro HORIGUCHI and Alexander Kuzmenkov traded patches to make logrotate work better with pg_ctl.

Kyotaro HORIGUCHI, Amit Langote, and Tom Lane traded patches to expand the syntax for defining partitions.

Ashutosh Bapat sent in another revision of a patch to fix an issue which causes expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.

Euler Taveira de Oliveira sent in a patch to fix an issue in pg_recvlogical.

Vik Fearing sent in another revision of a patch to add some features to pg_stat_statements.

David Rowley sent in two more revisions of a patch to initialize expr states once in run-time partition pruning.

Fujii Masao sent in another revision of a patch to speed up relation deletes during recovery.

Ashutosh Bapat sent in a patch to refactor postgres_fdw's DML execution hook functions.

Ildar Musin sent in a patch to add a hostorder parameter to libpq.

Thomas Munro sent in two more revisions of a patch to exit by default if postmaster dies.

Álvaro Herrera and Pavan Deolasee traded patches to fix an issue which caused VM map freeze corruption.

Konstantin Knizhnik sent in another revision of a patch to fix an issue where PostgreSQL can get stuck in deadlock detection.

Pavel Stěhule sent in another revision of a patch to implement schema variables.

Thomas Munro sent in three revisions of a patch to use signals for postmaster death on Linux and FreeBSD.

Kyotaro HORIGUCHI sent in a patch to fix undefined behavior in UNION ALL with null values in subselect.

Alexander Lakhin sent in a patch to fix "make installcheck-world" breakage.

Michaël Paquier sent in a patch to fix a memory leak in _SPI_stack caused by AtEOXact_SPI failing to clean up.

Haribabu Kommi sent in another revision of a patch to implement pluggable storage.

Kyotaro HORIGUCHI sent in two more revisions of a patch to correctly attach FPI to the first record after a checkpoint.

Laurenz Albe sent in a patch to ensure that SHOW ALL honors pg_read_all_settings membership.

David Rowley sent in two revisions of a patch to add a GUC which allows partition pruning to be disabled.

Andrew Gierth sent in two revisions of a patch to add toast_validate_* functions and use same to ensure that OldestXmin doesn't go backwards.

David Rowley sent in a patch to make bms_prev_member work correctly with a 64 bit bitmapword.