Correctifs appliqués

Magnus Hagander pushed:

Robert Haas pushed:

Álvaro Herrera pushed:

  • Fix state reversal after partition tuple routing. We make some changes to ModifyTableState and the EState it uses whenever we route tuples to partitions; but we weren't restoring properly in all cases, possibly causing crashes when partitions with different tuple descriptors are targeted by tuples inserted in the same command. Refactor some code, creating ExecPrepareTupleRouting, to encapsulate the needed state changing logic, and have it invoked one level above its current place (ie. put it in ExecModifyTable instead of ExecInsert); this makes it all more readable. Add a test case to exercise this. We don't support having views as partitions; and since only views can have INSTEAD OF triggers, there is no point in testing for INSTEAD OF when processing insertions into a partitioned table. Remove code that appears to support this (but which is actually never relevant.) In passing, fix location of some very confusing comments in ModifyTableState. Reported-by: Amit Langote Author: Etsuro Fujita, Amit Langote Discussion: https://postgr/es/m/
  • Expand comment a little bit. The previous commit removed a comment that was a bit more verbose than its replacement.
  • Remove unnecessary members from ModifyTableState and ExecInsert. These values can be obtained from the ModifyTable node which is already a part of both the ModifyTableState and ExecInsert. Author: Álvaro Herrera, Amit Langote Reviewed-by: Peter Geoghegan Discussion:
  • Fix CommandCounterIncrement in partition-related DDL. It makes sense to do the CCIs in the places that do catalog updates, rather than before the places that error out because the former ones fail to do it. In particular, it looks like StorePartitionBound() and IndexSetParentIndex() ought to make their own CCIs. Per review comments from Peter Eisentraut for row-level triggers on partitioned tables. Discussion:
  • Fix relcache handling of the 'default' partition. My commit 4dba331cb3dc that moved around CommandCounterIncrement calls in partitioning DDL code unearthed a problem with the relcache handling for the 'default' partition: the construction of a correct relcache entry for the partitioned table was at the mercy of lack of CCI calls in non-trivial amounts of code. This was prone to creating problems later on, as the code develops. This was visible as a test failure in a compile with RELCACHE_FORCE_RELASE (buildfarm member prion). The problem is that after the mentioned commit it was possible to create a relcache entry that had incomplete information regarding the default partition because I introduced a CCI between adding the catalog entries for the default partition (StorePartitionBound) and the update of pg_partitioned_table entry for its parent partitioned table (update_default_partition_oid). It seems the best fix is to move the latter so that it occurs inside the former; the purposeful lack of intervening CCI should be more obvious, and harder to break. I also remove a check in RelationBuildPartitionDesc that returns NULL if the key is not set. I couldn't find any place that needs this hack anymore; probably it was required because of bugs that have since been fixed. Fix a few typos I noticed while reviewing the code involved. Discussion:
  • Allow FOR EACH ROW triggers on partitioned tables. Previously, FOR EACH ROW triggers were not allowed in partitioned tables. Now we allow AFTER triggers on them, and on trigger creation we cascade to create an identical trigger in each partition. We also clone the triggers to each partition that is created or attached later. This means that deferred unique keys are allowed on partitioned tables, too. Author: Álvaro Herrera Reviewed-by: Peter Eisentraut, Simon Riggs, Amit Langote, Robert Haas, Thomas Munro Discussion:

Tom Lane pushed:

  • Fix performance hazard in REFRESH MATERIALIZED VIEW CONCURRENTLY. Jeff Janes discovered that commit 7ca25b7de made one of the queries run by REFRESH MATERIALIZED VIEW CONCURRENTLY perform badly. The root cause is bad cardinality estimation for correlated quals, but a principled solution to that problem is some way off, especially since the planner lacks any statistics about whole-row variables. Moreover, in non-error cases this query produces no rows, meaning it must be run to completion; but use of LIMIT 1 encourages the planner to pick a fast-start, slow-completion plan, exactly not what we want. Remove the LIMIT clause, and instead rely on the count parameter we pass to SPI_execute() to prevent excess work if the query does return some rows. While we've heard no field reports of planner misbehavior with this query, it could be that people are having performance issues that haven't reached the level of pain needed to cause a bug report. In any case, that LIMIT clause can't possibly do anything helpful with any existing version of the planner, and it demonstrably can cause bad choices in some cases, so back-patch to 9.4 where the code was introduced. Thomas Munro Discussion:
  • Fix some corner-case issues in REFRESH MATERIALIZED VIEW CONCURRENTLY. refresh_by_match_merge() has some issues in the way it builds a SQL query to construct the "diff" table: 1. It doesn't require the selected unique index(es) to be indimmediate. 2. It doesn't pay attention to the particular equality semantics enforced by a given index, but just assumes that they must be those of the column datatype's default btree opclass. 3. It doesn't check that the indexes are btrees. 4. It's insufficiently careful to ensure that the parser will pick the intended operator when parsing the query. (This would have been a security bug before CVE-2018-1058.) 5. It's not careful about indexes on system columns. The way to fix #4 is to make use of the existing code in ri_triggers.c for generating an arbitrary binary operator clause. I chose to move that to ruleutils.c, since that seems a more reasonable place to be exporting such functionality from than ri_triggers.c. While #1, #3, and #5 are just latent given existing feature restrictions, and #2 doesn't arise in the core system for lack of alternate opclasses with different equality behaviors, #4 seems like an issue worth back-patching. That's the bulk of the change anyway, so just back-patch the whole thing to 9.4 where this code was introduced. Discussion:
  • Prevent query-lifespan memory leakage of SP-GiST traversal values. The original coding of the SP-GiST scan traversalValue feature (commit ccd6eb49a) arranged for traversal values to be stored in the query's main executor context. That's fine if there's only one index scan per query, but if there are many, we have a memory leak as successive scans create new traversal values. Fix it by creating a separate memory context for traversal values, which we can reset during spgrescan(). Back-patch to 9.6 where this code was introduced. In principle, adding the traversalCxt field to SpGistScanOpaqueData creates an ABI break in the back branches. But I (tgl) have little sympathy for extensions including spgist_private.h, so I'm not very worried about that. Alternatively we could stick the new field at the end of the struct in back branches, but that has its own downsides. Anton Dignös, reviewed by Alexander Kuzmenkov Discussion:
  • Doc: typo fix, "PG_" should be "TG_" here. Too much PG on the brain in commit 769159fd3, evidently. Noted by Discussion:
  • Make configure check for a couple more Perl modules for --enable-tap-tests. Red Hat's notion of a basic Perl installation doesn't include Test::More or Time::HiRes, and reportedly some Debian installs also omit Time::HiRes. Check for those during configure to spare the user the pain of digging through check-world output to find out what went wrong. While we're at it, we should also check the version of Test::More, since requires at least 0.87. In principle this could be back-patched, but it's probably not necessary. Discussion:
  • Change oddly-chosen OID allocation. I noticed while fooling with John Naylor's bootstrap-data patch that we had one high-numbered manually assigned OID, 8888, which evidently came from a submission that the committer didn't bother to bring into line with usual OID allocation practices before committing. That's a bad idea, because it creates a hazard for other patches that may be temporarily using high OID numbers. Change it to something more in line with what we usually do. This evidently dates to commit abb173392. It's too late to change it in released branches, but we can fix it in HEAD.
  • Improve predtest.c's handling of cases with NULL-constant inputs. Currently, if operator_predicate_proof() is given an operator clause like "something op NULL", it just throws up its hands and reports it can't prove anything. But we can often do better than that, if the operator is strict, because then we know that the clause returns NULL overall. Depending on whether we're trying to prove or refute something, and whether we need weak or strong semantics for NULL, this may be enough to prove the implication, especially when we rely on the standard rule that "false implies anything". In particular, this lets us do something useful with questions like "does X IN (1,3,5,NULL) imply X <= 5?" The null entry in the IN list can effectively be ignored for this purpose, but the proof rules were not previously smart enough to deduce that. This patch is by me, but it owes something to previous work by Amit Langote to try to solve problems of the form mentioned. Thanks also to Emre Hasegeli and Ashutosh Bapat for review. Discussion:
  • Fix mishandling of quoted-list GUC values in pg_dump and ruleutils.c. Code that prints out the contents of setconfig or proconfig arrays in SQL format needs to handle GUC_LIST_QUOTE variables differently from other ones, because for those variables, flatten_set_variable_args() already applied a layer of quoting. The value can therefore safely be printed as-is, and indeed must be, or flatten_set_variable_args() will muck it up completely on reload. For all other GUC variables, it's necessary and sufficient to quote the value as a SQL literal. We'd recognized the need for this long ago, but mis-analyzed the need slightly, thinking that all GUC_LIST_INPUT variables needed the special treatment. That's actually wrong, since a valid value of a LIST variable might include characters that need quoting, although no existing variables accept such values. More to the point, we hadn't made any particular effort to keep the various places that deal with this up-to-date with the set of variables that actually need special treatment, meaning that we'd do the wrong thing with, for example, temp_tablespaces values. This affects dumping of SET clauses attached to functions, as well as ALTER DATABASE/ROLE SET commands. In ruleutils.c we can fix it reasonably honestly by exporting a guc.c function that allows discovering the flags for a given GUC variable. But pg_dump doesn't have easy access to that, so continue the old method of having a hard-wired list of affected variable names. At least we can fix it to have just one list not two, and update the list to match current reality. A remaining problem with this is that it only works for built-in GUC variables. pg_dump's list obvious knows nothing of third-party extensions, and even the "ask guc.c" method isn't bulletproof since the relevant extension might not be loaded. There's no obvious solution to that, so for now, we'll just have to discourage extension authors from inventing custom GUCs that need GUC_LIST_QUOTE. This has been busted for a long time, so back-patch to all supported branches. Michael Paquier and Tom Lane, reviewed by Kyotaro Horiguchi and Pavel Stehule Discussion:
  • Prevent extensions from creating custom GUCs that are GUC_LIST_QUOTE. Pending some solution for the problems noted in commit 742869946, disallow dynamic creation of GUC_LIST_QUOTE variables. If there are any extensions out there using this feature, they'd not be happy for us to start enforcing this rule in minor releases, so this is a HEAD-only change. The previous commit didn't make things any worse than they already were for such cases. Discussion:
  • Fix errors in contrib/bloom index build. Count the number of tuples in the index honestly, instead of assuming that it's the same as the number of tuples in the heap. (It might be different if the index is partial.) Fix counting of tuples in current index page, too. This error would have led to failing to write out the final page of the index if it contained exactly one tuple, so that the last tuple of the relation would not get indexed. Back-patch to 9.6 where contrib/bloom was added. Tomas Vondra and Tom Lane Discussion:
  • Fix tuple counting in SP-GiST index build. Count the number of tuples in the index honestly, instead of assuming that it's the same as the number of tuples in the heap. (It might be different if the index is partial.) Back-patch to all supported versions. Tomas Vondra Discussion:
  • Sync up our various ways of estimating pg_class.reltuples. VACUUM thought that reltuples represents the total number of tuples in the relation, while ANALYZE counted only live tuples. This can cause "flapping" in the value when background vacuums and analyzes happen separately. The planner's use of reltuples essentially assumes that it's the count of live (visible) tuples, so let's standardize on having it mean live tuples. Another issue is that the definition of "live tuple" isn't totally clear; what should be done with INSERT_IN_PROGRESS or DELETE_IN_PROGRESS tuples? ANALYZE's choices in this regard are made on the assumption that if the originating transaction commits at all, it will happen after ANALYZE finishes, so we should ignore the effects of the in-progress transaction --- unless it is our own transaction, and then we should count it. Let's propagate this definition into VACUUM, too. Likewise propagate this definition into CREATE INDEX, and into contrib/pgstattuple's pgstattuple_approx() function. Tomas Vondra, reviewed by Haribabu Kommi, some corrections by me Discussion:
  • Improve style guideline compliance of assorted error-report messages. Per the project style guide, details and hints should have leading capitalization and end with a period. On the other hand, errcontext should not be capitalized and should not end with a period. To support well formatted error contexts in dblink, extend dblink_res_error() to take a format+arguments rather than a hardcoded string. Daniel Gustafsson Discussion:
  • Fix make rules that generate multiple output files. For years, our makefiles have correctly observed that "there is no correct way to write a rule that generates two files". However, what we did is to provide empty rules that "generate" the secondary output files from the primary one, and that's not right either. Depending on the details of the creating process, the primary file might end up timestamped later than one or more secondary files, causing subsequent make runs to consider the secondary file(s) out of date. That's harmless in a plain build, since make will just re-execute the empty rule and nothing happens. But it's fatal in a VPATH build, since make will expect the secondary file to be rebuilt in the build directory. This would manifest as "file not found" failures during VPATH builds from tarballs, if we were ever unlucky enough to ship a tarball with apparently out-of-date secondary files. (It's not clear whether that has ever actually happened, but it definitely could.) To ensure that secondary output files have timestamps >= their primary's, change our makefile convention to be that we provide a "touch $@" action not an empty rule. Also, make sure that this rule actually gets invoked during a distprep run, else the hazard remains. It's been like this a long time, so back-patch to all supported branches. In HEAD, I skipped the changes in src/backend/catalog/Makefile, because those rules are due to get replaced soon in the bootstrap data format patch, and there seems no need to create a merge issue for that patch. If for some reason we fail to land that patch in v11, we'll need to back-fill the changes in that one makefile from v10. Discussion:
  • Mop-up for commit feb8254518752b2cb4a8964c374dd82d49ef0e0d. Missed these occurrences of some of the adjusted error messages. Per buildfarm member pademelon.
  • Stabilize regression test result. If random() returns a result sufficiently close to zero, float8out switches to scientific notation, breaking this test case's expectation that the output should look like '0.xxxxxxxxx'. Casting to numeric should fix that. Per buildfarm member pogona. Discussion:
  • Add #includes missed in commit e22b27f0cb3ee03ee300d431997f5944ccf2d7b3. Leaving out getopt_long.h works on some platforms, but not all. Per buildfarm. Discussion:
  • Doc: remove extra comma in syntax summary for array_fill(). Noted by Scott Ure. Back-patch to all supported branches. Discussion:
  • Remove useless if-test. Coverity complained that this check is pointless, and it's right. There is no case where we'd call ExecutorStart with a null plannedstmt, and if we did, it'd have crashed before here. Thinko in commit cc415a56d.

Andrew Dunstan pushed:

  • Don't use an Msys virtual path to create a tablespace. The new unlogged_reinit recovery tests create a new tablespace using's tempdir. However, on msys that function returns a virtual path that isn't understood by Postgres. Here we add a new function to to turn such a path into a real path on the underlying file system, and use it in the new test to create the tablespace. The new function is essentially a NOOP everywhere but msys.

Peter Eisentraut pushed:

Andres Freund pushed:

Teodor Sigaev pushed:

Andrew Gierth pushed:

  • Repair crash with unsortable grouping sets. If there were multiple grouping sets, none of them empty, all of which were unsortable, then an oversight in consider_groupingsets_paths led to a null pointer dereference. Fix, and add a regression test for this case. Per report from Dang Minh Huong, though I didn't use their patch. Backpatch to 10.x where hashed grouping sets were added.

Tatsuo Ishii pushed:

Dean Rasheed pushed:

  • Improve ANALYZE's strategy for finding MCVs. Previously, a value was included in the MCV list if its frequency was 25% larger than the estimated average frequency of all nonnull values in the table. For uniform distributions, that can lead to values being included in the MCV list and significantly overestimated on the basis of relatively few (sometimes just 2) instances being seen in the sample. For non-uniform distributions, it can lead to too few values being included in the MCV list, since the overall average frequency may be dominated by a small number of very common values, while the remaining values may still have a large spread of frequencies, causing both substantial overestimation and underestimation of the remaining values. Furthermore, increasing the statistics target may have little effect because the overall average frequency will remain relatively unchanged. Instead, populate the MCV list with the largest set of common values that are statistically significantly more common than the average frequency of the remaining values. This takes into account the variance of the sample counts, which depends on the counts themselves and on the proportion of the table that was sampled. As a result, it constrains the relative standard error of estimates based on the frequencies of values in the list, reducing the chances of too many values being included. At the same time, it allows more values to be included, since the MCVs need only be more common than the remaining non-MCVs, rather than the overall average. Thus it tends to produce fewer MCVs than the previous code for uniform distributions, and more for non-uniform distributions, reducing estimation errors in both cases. In addition, the algorithm responds better to increasing the statistics target, allowing more values to be included in the MCV list when more of the table is sampled. Jeff Janes, substantially modified by me. Reviewed by John Naylor and Tomas Vondra. Discussion:

Noah Misch pushed:

  • Don't qualify type pg_catalog.text in extend-extensions-example. Extension scripts begin execution with pg_catalog at the front of the search path, so type names reliably refer to pg_catalog. Remove these superfluous qualifications. Earlier <programlisting> of this <sect1> already omitted them. Back-patch to 9.3 (all supported versions).

Correctifs en attente

Tom Lane sent in a patch to fix some issues in matview.c's refresh-query construction.

Daniel Gustafsson and Magnus Hagander traded patches to make it possible to enable checksums online.

Tomas Vondra sent in another revision of a patch to implement multivariate histograms and MCV lists.

Chapman Flack sent in another revision of a patch to zero headers of unused pages after WAL switch and add a test for ensuring WAL segment is zeroed out.

Amul Sul sent in another revision of a patch to restrict concurrent update/delete with UPDATE of partition key.

Kyotaro HORIGUCHI sent in another revision of a patch to restrict maximum keep segments by repslots.

Andrey Borodin sent in another revision of a patch to add SLRU checksums.

Masahiko Sawada sent in another revision of a patch to qualify datatype name in log of data type conversion on subscriber and add a test module for same.

Masahiko Sawada sent in another revision of a patch to add a vacuum_cleanup_index_scale_factor GUC.

Etsuro Fujita, Amit Langote, and Álvaro Herrera traded patches to make ON CONFLICT .. DO UPDATE work on partitioned tables.

Amit Langote and David Rowley traded patches to lay infrastructure for speeding up partition pruning.

Artur Zakirov and Tomas Vondra traded patches to implement shared ISpell dictionaries.

Pavel Stěhule sent in two more revisions of a patch to add extra checks to PL/pgsql.

Thomas Munro sent in a patch to add docs to the top-level Makefile for non-GNU make.

Pavel Stěhule sent in a patch to enable procedures to be called with default arguments in PL/pgsql.

Alexander Korotkov sent in two more revisions of a patch to implement incremental sort.

Julian Markwort sent in two more revisions of a patch to add a plan option to pg_stat_statements.

Pavel Stěhule sent in two more revisions of a patch to implement schema variables.

Dilip Kumar sent in two more revisions of a patch to ensure that InitXLogInsert is never called in a critical section.

Peter Eisentraut sent in another revision of a patch to use file cloning in pg_upgrade and CREATE DATABASE.

Pavan Deolasee, Peter Geoghegan, and Amit Langote traded patches to implement MERGE.

Daniel Gustafsson sent in another revision of a patch to support sending an optional message in backend cancel/terminate.

Tomas Vondra sent in another revision of a patch to implement BRIN multi-range indexes and BRIN Bloom indexes.

Michael Banck sent in a patch to allow setting replication slots in recovery.conf even if wal-method is none.

Daniel Gustafsson sent in a patch to pg_basebackup to add missing newlines in some error messages.

Konstantin Knizhnik sent in another revision of a patch to optimize secondary indexes.

Thomas Munro sent in a patch to tweak the JIT docs.

Doug Rady sent in another revision of a patch to enable building pgbench using ppoll() for larger connection counts.

Tom Lane and John Naylor traded patches to rationalize the way bootstrap data is handled.

Pavel Stěhule sent in a patch to enable CALL with named default arguments in PL/pgsql.

Nathan Bossart sent in a patch to combine options for RangeVarGetRelidExtended() into a flags argument and add a skip-locked option to same.

Fabien COELHO sent in a patch to fix some constants in the new general-purpose hashing functions for pgbench.

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

Dmitry Dolgov sent in another revision of a patch to implement generic type subscripting and use same for arrays and JSONB.

Teodor Sigaev sent in another revision of a patch to add a prefix operator for text with SP-GiST support.

David Rowley sent in two more revisions of a patch to remove useless DISTINCT clauses.

Fabien COELHO sent in two more revisions of a patch to implement --random-seed for pgbench.

David Steele and Michael Banck traded patches to verify checksums during basebackups.

Pavan Deolasee sent in two more revisions of a patch to speed up inserts with mostly-monotonically increasing values.

Daniel Vérité and Pavel Stěhule traded patches to add a csv format to psql.

Tomas Vondra sent in a patch to fix a a minor mistake in brin_inclusion.c comment.

Ashutosh Bapat sent in a patch to fix a comment in BuildTupleFromCStrings().

Teodor Sigaev sent in another revision of a patch to implement predicate locking on GIN indexes.

Ashutosh Bapat sent in a patch to fix an issue with procedure name resolution.

Julian Markwort sent in another revision of a patch to add a new auth option to pg_hba.conf: clientcert=verify-full.

Simon Riggs sent in another revision of a patch to implement logical decoding of two-phase transactions.

David Steele sent in a patch to add more TAP tests to pgrewind.

Pavan Deolasee sent in another revision of a patch to change the WAL header to reduce contention during ReserveXLogInsertLocation().

Haribabu Kommi sent in two more revisions of a patch to make PQhost return connected host and hostaddr details.

Fabien COELHO sent in a patch to pgbench to test whether a variable exists.

Robert Haas sent in another revision of a patch to enable parallel seq scan for slow functions.

Peter Eisentraut sent in another revision of a patch to enable nested CALL with transactions in PL/pgSQL.

Fabien COELHO sent in another revision of a patch to pgbench to enable it to store SELECT results into variables.

Tom Lane sent in a patch to help with backend memory dump analysis by adding context identifiers.

David Rowley sent in two more revisions of a patch to speed up execution of ALTER TABLE ... ADD COLUMN ... DEFAULT [not null].

Michaël Paquier sent in a patch to simplify the final sync in pg_rewind's target folder and add --no-sync.

Takayuki Tsunakawa sent in another revision of a patch to fix a problem in ECPG where freeing memory for pgtypes crashes on Windows.