Correctifs appliqués

Tatsuo Ishii pushed:

Peter Eisentraut pushed:

Tom Lane pushed:

  • Fix ALTER SEQUENCE OWNED BY to not rewrite the sequence relation. It's not necessary for it to do that, since OWNED BY requires only ordinary catalog updates and doesn't affect future sequence values. And pg_upgrade needs to use OWNED BY without having it change the sequence's relfilenode. Commit 3d79013b9 broke this by making all forms of ALTER SEQUENCE change the relfilenode; that seems to be the explanation for the hard-to-reproduce buildfarm failures we've been seeing since then. Discussion:
  • Assert that we don't invent relfilenodes or type OIDs in binary upgrade. During pg_upgrade's restore run, all relfilenode choices should be overridden by commands in the dump script. If we ever find ourselves choosing a relfilenode in the ordinary way, someone blew it. Likewise for pg_type OIDs. Since pg_upgrade might well succeed anyway, if there happens not to be a conflict during the regression test run, we need assertions here to keep us on the straight and narrow. We might someday be able to remove the assertion in GetNewRelFileNode, if pg_upgrade is rewritten to remove its assumption that old and new relfilenodes always match. But it's hard to see how to get rid of the pg_type OID constraint, since those OIDs are embedded in user tables in some cases. Back-patch as far as 9.5, because of the risk of back-patches breaking something here even if it works in HEAD. I'd prefer to go back further, but 9.4 fails both assertions due to get_rel_infos()'s use of a temporary table. We can't use the later-branch solution of a CTE for compatibility reasons (cf commit 5d16332e9), and it doesn't seem worth inventing some other way to do the query. (I did check, by dint of changing the Asserts to elog(WARNING), that there are no other cases of unwanted OID assignments during 9.4's regression test run.) Discussion:
  • Fix confusion about number of subplans in partitioned INSERT setup. ExecInitModifyTable() thought there was a plan per partition, but no, there's only one. The problem had escaped detection so far because there would only be visible misbehavior if there were a SubPlan (not an InitPlan) in the quals being duplicated for each partition. However, valgrind detected a bogus memory access in test cases added by commit 4f7a95be2, and investigation of that led to discovery of the bug. The additional test case added here crashes without the patch. Patch by Amit Langote, test case by me. Discussion:
  • In initdb, defend against assignment of NULL values to not-null columns. Previously, you could write _null_ in a BKI DATA line for a column that's supposed to be NOT NULL and initdb would let it pass, probably breaking subsequent accesses to the row. No doubt the original coding overlooked this simple sanity check because in the beginning we didn't have any way to mark catalog columns NOT NULL at initdb time.
  • Re-run pgindent. This is just to have a clean base state for testing of Piotr Stefaniak's latest version of FreeBSD indent. I fixed up a couple of places where pgindent would have changed format not-nicely. perltidy not included. Discussion:
  • Disallow set-returning functions inside CASE or COALESCE. When we reimplemented SRFs in commit 69f4b9c85, our initial choice was to allow the behavior to vary from historical practice in cases where a SRF call appeared within a conditional-execution construct (currently, only CASE or COALESCE). But that was controversial to begin with, and subsequent discussion has resulted in a consensus that it's better to throw an error instead of executing the query differently from before, so long as we can provide a reasonably clear error message and a way to rewrite the query. Hence, add a parser mechanism to allow detection of such cases during parse analysis. The mechanism just requires storing, in the ParseState, a pointer to the set-returning FuncExpr or OpExpr most recently emitted by parse analysis. Then the parsing functions for CASE and COALESCE can detect the presence of a SRF in their arguments by noting whether this pointer changes while analyzing their arguments. Furthermore, if it does, it provides a suitable error cursor location for the complaint. (This means that if there's more than one SRF in the arguments, the error will point at the last one to be analyzed not the first. While connoisseurs of parsing behavior might find that odd, it's unlikely the average user would ever notice.) While at it, we can also provide more specific error messages than before about some pre-existing restrictions, such as no-SRFs-within-aggregates. Also, reject at parse time cases where a NULLIF or IS DISTINCT FROM construct would need to return a set. We've never supported that, but the restriction is depended on in more subtle ways now, so it seems wise to detect it at the start. Also, provide some documentation about how to rewrite a SRF-within-CASE query using a custom wrapper SRF. It turns out that the information_schema.user_mapping_options view contained an instance of exactly the behavior we're now forbidding; but rewriting it makes it more clear and safer too. initdb forced because of user_mapping_options change. Patch by me, with error message suggestions from Alvaro Herrera and Andres Freund, pursuant to a complaint from Regina Obe. Discussion:$d8d66170$8a832450$
  • Fix violations of CatalogTupleInsert/Update/Delete abstraction. In commits 2f5c9d9c9 and ab0289651 we invented an abstraction layer to insulate catalog manipulations from direct heap update calls. But evidently some patches that hadn't landed in-tree at that point didn't get the memo completely. Fix a couple of direct calls to simple_heap_delete to use CatalogTupleDelete instead; these appear to have been added in commits 7c4f52409 and 7b504eb28. This change is purely cosmetic ATM, but there's no point in having an abstraction layer if we allow random code to break it. Masahiko Sawada and Tom Lane Discussion:
  • Fix no-longer-valid shortcuts in expression_returns_set(). expression_returns_set() used to short-circuit its recursion upon seeing certain node types, such as DistinctExpr, that it knew the executor did not support set-valued arguments for. That was never inherent, though, just a reflection of laziness in execQual.c. With the new implementation of SRFs there is no reason to think that any scalar-valued expression node could not have a set-valued subexpression, except for AggRefs and WindowFuncs where we know there is a parser check rejecting it. And indeed, the shortcut causes unexpected failures for cases such as a SRF underneath DistinctExpr, because the planner stops looking for SRFs too soon. Discussion:
  • Improve release note text about set-returning-function changes. Paul Ramsey griped about this awhile ago, but I'd been holding fire on changing it until we settled what to do about the CASE/COALESCE issue. Discussion:
  • Fix low-probability leaks of PGresult objects in the backend. We had three occurrences of essentially the same coding pattern wherein we tried to retrieve a query result from a libpq connection without blocking. In the case where PQconsumeInput failed (typically indicating a lost connection), all three loops simply gave up and returned, forgetting to clear any previously-collected PGresult object. Since those are malloc'd not palloc'd, the oversight results in a process-lifespan memory leak. One instance, in libpqwalreceiver, is of little significance because the walreceiver process would just quit anyway if its connection fails. But we might as well fix it. The other two instances, in postgres_fdw, are somewhat more worrisome because at least in principle the scenario could be repeated, allowing the amount of memory leaked to build up to something worth worrying about. Moreover, in these cases the loops contain CHECK_FOR_INTERRUPTS calls, as well as other calls that could potentially elog(ERROR), providing another way to exit without having cleared the PGresult. Here we need to add PG_TRY logic similar to what exists in quite a few other places in postgres_fdw. Coverity noted the libpqwalreceiver bug; I found the other two cases by checking all calls of PQconsumeInput. Back-patch to all supported versions as appropriate (9.2 lacks postgres_fdw, so this is really quite unexciting for that branch). Discussion:
  • Make configure check for IPC::Run when --enable-tap-tests is specified. The TAP tests mostly don't work without IPC::Run, and the reason for the failure is not immediately obvious from the error messages you get. So teach configure to reject --enable-tap-tests unless IPC::Run exists. Mostly this just involves adding ax_prog_perl_modules.m4 from the GNU autoconf archives. This was discussed last year, but we held off on the theory that we might be switching to CMake soon. That's evidently not happening for v10, so let's absorb this now. Eugene Kazakov and Michael Paquier Discussion: Discussion:
  • Teach pgindent to skip files generated by bison or flex automatically. If a .c or .h file corresponds to a .y or .l file, skip indenting it. There's no point in reindenting derived files, and these files tend to confuse pgindent. (Which probably indicates a bug in BSD indent, but I can't get excited about trying to fix it.) For the same reasons, add src/backend/utils/fmgrtab.c to the set of files excluded by src/tools/pgindent/exclude_file_patterns. The point of doing this is that it makes it safe to run pgindent over the tree without doing "make maintainer-clean" first. While these are not the only derived .c/.h files in the tree, they are the only ones pgindent fails on. Removing that prerequisite step results in one less way to mess up a pgindent run, and it's necessary if we ever hope to get to the ease of running pgindent via "make indent".

Robert Haas pushed:

Dean Rasheed pushed:

�lvaro Herrera pushed:

Andres Freund pushed:

  • Don't force-assign transaction id when exporting a snapshot. Previously we required every exported transaction to have an xid assigned. That was used to check that the exporting transaction is still running, which in turn is needed to guarantee that that necessary rows haven't been removed in between exporting and importing the snapshot. The exported xid caused unnecessary problems with logical decoding, because slot creation has to wait for all concurrent xid to finish, which in turn serializes concurrent slot creation. It also prohibited snapshots to be exported on hot-standby replicas. Instead export the virtual transactionid, which avoids the unnecessary serialization and the inability to export snapshots on standbys. This changes the file name of the exported snapshot, but since we never documented what that one means, that seems ok. Author: Petr Jelinek, slightly editorialized by me Reviewed-By: Andres Freund Discussion:

Bruce Momjian pushed:

Noah Misch pushed:

Heikki Linnakangas pushed:

Magnus Hagander pushed:

Correctifs en attente

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

Thomas Munro sent in another revision of a patch to ensure that there are two transition table tuplestores for the ON CONFLICT DO UPDATE case, one for updated tuples, and the other for inserted tuples.

Haribabu Kommi sent in another revision of a patch to put in the infrastructure for pluggable storage.

Tom Lane sent in a patch to check that pg_upgrade never generates a new pg_type OID and ensures that relfilenodes are all assigned.

Amit Langote sent in a patch to teach pgrowlocks to check relkind before scanning.

Jeevan Ladhe sent in a patch to fix possible optimizations in ATExecAttachPartition().

Micha�l Paquier sent in a patch to ensure that pg_receivewal and messages are printed only in verbose mode.

Masahiko Sawada sent in a patch to make RemoveSubscriptionRel use CatalogTupleDelete() rather than the curren simple_heap_delete.

Masahiko Sawada sent in a patch to disallow ALTER SUBSCRIPTION SET PUBLICATION WITH (refresh = true) and ALTER SUBSCRIPTION REFRESH PUBLICATION inside a transaction block.

Bruce Momjian sent in two revisions of a patch to clarify the documentation around hint bits as it relates to pg_upgrade.

Peter Eisentraut sent in a patch to fix the output of char node fields.

Christian Ullrich sent in a patch to make setlocale() aware of multithreading to avoid crash.

Etsuro Fujita sent in a patch to update comments in nodeModifyTable.c.

Mithun Cy sent in another revision of a patch to implement auto_prewarm.

Beena Emerson sent in another revision of a patch to implement default partitions for range partitions.

Surafel Temesgen sent in another revision of a patch to disallow multiple queries per PQexec().

Marina Polyakova sent in a patch to improve pgbench by removing serialization and deadlock errors, setting the correct default transaction isolation level, reporting per-statement serialization and deadlock failures, and fixing the documentation.

Etsuro Fujita sent in a patch to add a missing comment for create_modifytable_path.

Justin Pryzby sent in a patch to make showusage() get memory fields from getrusage().

Amit Khandekar sent in another revision of a patch to make UPDATEs on declaratively partitioned tables which change the partition key to actually move the affected rows to the appropriate partitions.

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

Jeff Janes sent in another revision of a patch to prevent subscription workers from signalling the WAL writer too much.

Peter Eisentraut sent in a WIP patch to tweak publication fetching in psql.

QL Zhuo sent in two revisions of a patch to ensure that filepath/filename are not downcased while loading libraries.

Fabien COELHO sent in another revision of a patch to implement \gdesc in psql.

Ashutosh Sharma sent in three more revisions of a patch to fix ICU on Windows.

Etsuro Fujita sent in another revision of a patch to adjust inherited update tlists.

Konstantin Knizhnik sent in a patch to implement AES encryption in the backend.

Fabien COELHO sent in another revision of a patch to add a special variable to psql to reflect the last query status.

Shubham Barai sent in two revisions of a patch to implement predicate locking in GiST indexes.

Mark Rofail sent in a patch to add a record to pg_proc (src/include/catalog/pg_proc.h), modify opr_sanity regression check expected results to account for it, implement a low-level function called `array_contains_elem` as an equivalent to `array_contain_compare` but accepts anyelement instead of anyarray as the right operand.

Julien Rouhaud sent in a patch to fix a typo in insert.sgml.