Correctifs appliqués

Bruce Momjian pushed:

�lvaro Herrera pushed:

Robert Haas pushed:

Peter Eisentraut pushed:

Tom Lane pushed:

  • Make plpgsql use its DTYPE_REC code paths for composite-type variables. Formerly, DTYPE_REC was used only for variables declared as "record"; variables of named composite types used DTYPE_ROW, which is faster for some purposes but much less flexible. In particular, the ROW code paths are entirely incapable of dealing with DDL-caused changes to the number or data types of the columns of a row variable, once a particular plpgsql function has been parsed for the first time in a session. And, since the stored representation of a ROW isn't a tuple, there wasn't any easy way to deal with variables of domain-over-composite types, since the domain constraint checking code would expect the value to be checked to be a tuple. A lesser, but still real, annoyance is that ROW format cannot represent a true NULL composite value, only a row of per-field NULL values, which is not exactly the same thing. Hence, switch to using DTYPE_REC for all composite-typed variables, whether "record", named composite type, or domain over named composite type. DTYPE_ROW remains but is used only for its native purpose, to represent a fixed-at-compile-time list of variables, for instance the targets of an INTO clause. To accomplish this without taking significant performance losses, introduce infrastructure that allows storing composite-type variables as "expanded objects", similar to the "expanded array" infrastructure introduced in commit 1dc5ebc90. A composite variable's value is thereby kept (most of the time) in the form of separate Datums, so that field accesses and updates are not much more expensive than they were in the ROW format. This holds the line, more or less, on performance of variables of named composite types in field-access-intensive microbenchmarks, and makes variables declared "record" perform much better than before in similar tests. In addition, the logic involved with enforcing composite-domain constraints against updates of individual fields is in the expanded record infrastructure not plpgsql proper, so that it might be reusable for other purposes. In further support of this, introduce a typcache feature for assigning a unique-within-process identifier to each distinct tuple descriptor of interest; in particular, DDL alterations on composite types result in a new identifier for that type. This allows very cheap detection of the need to refresh tupdesc-dependent data. This improves on the "tupDescSeqNo" idea I had in commit 687f096ea: that assigned identifying sequence numbers to successive versions of individual composite types, but the numbers were not unique across different types, nor was there support for assigning numbers to registered record types. In passing, allow plpgsql functions to accept as well as return type "record". There was no good reason for the old restriction, and it was out of step with most of the other PLs. Tom Lane, reviewed by Pavel Stehule Discussion:
  • Speed up plpgsql function startup by doing fewer pallocs. Previously, copy_plpgsql_datum did a separate palloc for each variable needing instance-local storage. In simple benchmarks this made for a noticeable fraction of the total runtime. Improve it by precalculating the space needed for all of a function's variables and doing just one palloc for all of them. In passing, remove PLPGSQL_DTYPE_EXPR from the list of plpgsql "datum" types, since in fact it has nothing in common with the others, and there is noplace that needs to discriminate on the basis of dtype between an expression and any type of datum. And add comments clarifying which datum struct fields are generic and which aren't. Tom Lane, reviewed by Pavel Stehule Discussion:
  • Speed up plpgsql trigger startup by introducing "promises". Over the years we've accreted quite a few special variables that are predefined in plpgsql trigger functions. The cost of initializing these variables to their defined values turns out to be a significant part of the runtime of simple triggers; but, undoubtedly, most real-world triggers never examine the values of most of these variables. To improve matters, invent the notion of a variable that has a "promise" attached to it, specifying which of the predetermined values should be assigned to the variable if anything ever reads it. This eliminates all the unneeded startup overhead, in return for a small penalty on accesses to these variables. Tom Lane, reviewed by Pavel Stehule Discussion:
  • Support CONSTANT/NOT NULL/initial value for plpgsql composite variables. These features were never implemented previously for composite or record variables ... not that the documentation admitted it, so there's no doc updates here. This also fixes some issues concerning enforcing DOMAIN NOT NULL constraints against plpgsql variables, although I'm not sure that that topic is completely dealt with. I created a new plpgsql test file for these features, and moved the one relevant existing test case into that file. Tom Lane, reviewed by Daniel Gustafsson Discussion:
  • Fix broken logic for reporting PL/Python function names in errcontext. plpython_error_callback() reported the name of the function associated with the topmost PL/Python execution context. This was not merely wrong if there were nested PL/Python contexts, but it risked a core dump if the topmost one is an inline code block rather than a named function. That will have proname = NULL, and so we were passing a NULL pointer to snprintf("%s"). It seems that none of the PL/Python-testing machines in the buildfarm will dump core for that, but some platforms do, as reported by Marina Polyakova. Investigation finds that there actually is an existing regression test that used to prove that the behavior was wrong, though apparently no one had noticed that it was printing the wrong function name. It stopped showing the problem in 9.6 when we adjusted psql to not print CONTEXT by default for NOTICE messages. The problem is masked (if your platform avoids the core dump) in error cases, because PL/Python will throw away the originally generated error info in favor of a new traceback produced at the outer level. Repair by using ErrorContextCallback.arg to pass the correct context to the error callback. Add a regression test illustrating correct behavior. Back-patch to all supported branches, since they're all broken this way. Discussion:
  • Add an assertion that we don't pass NULL to snprintf("%s"). Per commit e748e902d, we appear to have little or no coverage in the buildfarm of machines that will dump core when asked to printf a null string pointer. Let's try to improve that situation by adding an assertion that will make src/port/snprintf.c behave that way. Since it's just an assertion, it won't break anything in production builds, but it will help developers find this type of oversight. Note that while our buildfarm coverage of machines that use that snprintf implementation is pretty thin on the Unix side (apparently amounting only to gaur/pademelon), all of the MSVC critters use it. Discussion:
  • Silence assorted "variable may be used uninitialized" warnings. All of these are false positives, but in each case a fair amount of analysis is needed to see that, and it's not too surprising that not all compilers are smart enough. (In particular, in the logtape.c case, a compiler lacking the knowledge provided by the Assert would almost surely complain, so that this warning will be seen in any non-assert build.) Some of these are of long standing while others are pretty recent, but it only seems worth fixing them in HEAD. Jaime Casanova, tweaked a bit by me Discussion:
  • Stabilize new plpgsql_record regression tests. The buildfarm's CLOBBER_CACHE_ALWAYS animals aren't happy with some of the test cases added in commit 4b93f5799. There are two different problems: * In two places, a different CONTEXT stack is shown because the error is detected in a different place, due to recompiling an expression from scratch rather than re-using a previously cached plan for it. I fixed these via the expedient of hiding the CONTEXT stack altogether. * In one place, a test expected to fail (because a cached plan hadn't been updated) actually succeeds (because the forced recompile makes it good). I couldn't think of a simple workaround for this, so I've just commented out that test step altogether. I have hopes of improving things enough that both of these kluges can be reverted eventually. The first one is the same kind of problem previously discussed at but there was insufficient agreement about how to fix it, so we just hacked around the output instability (commit 9edc97b71). The second issue should be fixed by allowing the plan to be rebuilt when a type conflict is detected. But for today, let's just make the buildfarm green again.
  • Revert "Stabilize output of new regression test case". This effectively reverts commit 9edc97b71 (although the test is now in a different place and has different contents). We don't need that hack anymore, because since commit 4b93f5799, this test case no longer throws an error and so there's no displayed CONTEXT that could vary depending on CLOBBER_CACHE_ALWAYS. The underlying unstable-output problem isn't really gone, of course, but it no longer manifests here.
  • Move the extern declaration for ExceptionalCondition into c.h. This is the logical conclusion of our decision to support Assert() in both frontend and backend code: it should be possible to use that after including just c.h. But as things were arranged before, if you wanted to use Assert() in code that might be compiled for either environment, you had to include postgres.h for the backend case. Let's simplify that. Per buildfarm, some of whose members started throwing warnings after commit 0c62356cc added an Assert in src/port/snprintf.c. It's possible that some other src/port files that use the stanza #ifndef FRONTEND #include "postgres.h" #else #include "postgres_fe.h" #endif could now be simplified to just say '#include "c.h"'. I have not tested for that, though, and it'd be unlikely to apply for more than a small number of them.
  • Cast to void in StaticAssertExpr, not its callers. Seems a bit silly that many (in fact all, as of today) uses of StaticAssertExpr would need to cast it to void to avoid warnings from pickier compilers. Let's just do the cast right in the macro, instead. In passing, change StaticAssertExpr to StaticAssertStmt in one place where that seems more apropos. Discussion:
  • Doc: fix minor bug in CREATE TABLE example. One example in create_table.sgml claimed to be showing table constraint syntax, but it was really column constraint syntax due to the omission of a comma. This is both wrong and confusing, so fix it in all supported branches. Per report from Discussion:
  • Fix plpgsql to enforce domain checks when returning a NULL domain value. If a plpgsql function is declared to return a domain type, and the domain's constraints forbid a null value, it was nonetheless possible to return NULL, because we didn't bother to check the constraints for a null result. I'd noticed this while fooling with domains-over-composite, but had not gotten around to fixing it immediately. Add a regression test script exercising this and various other domain cases, largely borrowed from the plpython_types test. Although this is clearly a bug fix, I'm not sure whether anyone would thank us for changing the behavior in stable branches, so I'm inclined not to back-patch.
  • Remove some inappropriate #includes. Other header files should never #include postgres.h (nor postgres_fe.h, nor c.h), per project policy. Also, there's no need for any backend .c file to explicitly include elog.h or palloc.h, because postgres.h pulls those in already. Extracted from a larger patch by Kyotaro Horiguchi. The rest of the removals he suggests require more study, but these are no-brainers. Discussion:

Andres Freund pushed:

  • Return implementation defined value if pg_$op_s$bit_overflow overflows. Some older compilers otherwise sometimes complain about undefined values, even though the return value should not be used in the overflow case. We assume that any decent compiler will optimize away the unnecessary assignment in performance critical cases. We do not want to restrain the returned value to a specific value, e.g. 0 or the wrapped-around value, because some fast ways to implement overflow detecting math do not easily allow for that (e.g. msvc intrinsics). As the function documentation already documents the returned value in case of intrinsics to be implementation defined, no documentation has to be updated. Per complaint from Tom Lane and his buildfarm member prairiedog. Author: Andres Freund Discussion:
  • Revert "Do execGrouping.c via expression eval machinery.". This reverts commit 773aec7aa98abd38d6d9435913bb8e14e392c274. There's an unresolved issue in the reverted commit: It only creates one comparator function, but in for the nodeSubplan.c case we need more (c.f. FindTupleHashEntry vs LookupTupleHashEntry calls in nodeSubplan.c). This isn't too difficult to fix, but it's not entirely trivial either. The fact that the issue only causes breakage on 32bit systems shows that the current test coverage isn't that great. To avoid turning half the buildfarm red till those two issues are addressed, revert.
  • Do execGrouping.c via expression eval machinery. This has a performance benefit on own, although not hugely so. The primary benefit is that it will allow for to JIT tuple deforming and comparator invocations. Author: Andres Freund Discussion:
  • Do execGrouping.c via expression eval machinery, take two. This has a performance benefit on own, although not hugely so. The primary benefit is that it will allow for to JIT tuple deforming and comparator invocations. Large parts of this were previously committed (773aec7aa), but the commit contained an omission around cross-type comparisons and was thus reverted. Author: Andres Freund Discussion:
  • Allow tupleslots to have a fixed tupledesc, use in executor nodes. The reason for doing so is that it will allow expression evaluation to optimize based on the underlying tupledesc. In particular it will allow to JIT tuple deforming together with the expression itself. For that expression initialization needs to be moved after the relevant slots are initialized - mostly unproblematic, except in the case of nodeWorktablescan.c. After doing so there's no need for ExecAssignResultType() and ExecAssignResultTypeFromTL() anymore, as all former callers have been converted to create a slot with a fixed descriptor. When creating a slot with a fixed descriptor, tts_values/isnull can be allocated together with the main slot, reducing allocation overhead and increasing cache density a bit. Author: Andres Freund Discussion:

Magnus Hagander pushed:

Correctifs en attente

Thomas Munro sent in a patch to minor clean-up in dshash.{c,h}.

Thomas Munro sent in a WIP patch to use explicit memory barriers when manipulating MyProc->subxids and nxids.

Artur Zakirov sent in another revision of a patch to implement a more flexible configuration for full-text search.

Thomas Munro sent in a patch to remove volatile qualifiers from shm_mq.c.

�lvaro Herrera sent in another revision of a patch to allow unique indexes on partitioned tables. Peter Eisentraut sent a follow-on patch to clarify the documentation.

Rajkumar Raghuwanshi sent in a patch to fix an error which manifested as expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.

Artur Zakirov sent in two more revisions of a patch to fix a bug in to_timestamp().

Amit Langote sent in a patch to update the docs to reflect the new capability to index partitioned tables.

Amit Kapila sent in a patch to fix a failure condition in tuple version locking.

Etsuro Fujita sent in a patch to fix a regression test failure in the PostgreSQL FDW.

Michail Nikolaev sent in another revision of a patch to get index scan offsets faster by using the visibility map.

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

Rafia Sabih sent in another revision of a patch to implement partition-wise aggregation/grouping.

Marina Polyakova sent in a patch to fix a PL/PythonU breakage on Solaris.

Peter Eisentraut sent in a patch to support Python 3.7 when it arrives.

Anthony Bykov sent in two more revisions of a patch to make TRANSFORMs for JSONB in PL/Perl.

Amit Langote and Etsuro Fujita traded patches to initialize per-partition objects during tuple routing.

Micha�l Paquier sent in a patch to fix some cache lookup errors with functions manipulation object addresses.

Amit Langote sent in three more revisions of a patch to speed up partition pruning.

Andrew Dunstan sent in two more revisions of a patch to speed up ALTER TABLE ... ADD COLUMN with a non-NULL DEFAULT.

Nikita Glukhov sent in another revision of a patch to implement SQL/JSON functions.

Nikita Glukhov sent in another revision of a patch to implement SQL/JSON's JSON_TABLE.

Kyotaro HORIGUCHI sent in a patch to fix the fact that the comment on formdesc in relcache.c's LookupOpclassInfo() omits pg_subscription.

Nikita Glukhov sent in another revision of a patch to implement SQL/JSON's jsonpath.

Peter Eisentraut sent in a patch to implement a Kerberos test suite.

Nikolay Shaplov sent in another revision of a patch to add an enum reloption type.

Pierre Ducroquet sent in another revision of a patch atop the existing JIT patch to enable JIT compiling with LLVM v10.1.

Takayuki Tsunakawa sent in a patch to fix an issue where a cascaded standby fails to start after a clean shutdown by zero-filling WAL blocks on standbys.

Amit Langote sent in four more revisions of a patch to reorganize the partitioning code.

�lvaro Herrera sent in another revision of a patch to implement FOR EACH ROW triggers on partitioned tables.

Amit Kapila sent in another revision of a patch to ensure that parallel paths include tlist cost.

Kyotaro HORIGUCHI sent in a patch to ensure that extra shared memory isn't requested when HAVE_SPINLOCKS is enabled.

Ashutosh Bapat sent in a patch to change baserel to foreignrel in some postgresql_fdw functions.

Claudio Freire sent in another revision of a patch to vacuum to update the FSM more frequently.

Kyotaro HORIGUCHI sent in a patch to ensure that spin.c includes pg_sema.h only when necessary.

Kyotaro HORIGUCHI sent in a patch to remove DSM_INPL_NONE.

Konstantin Knizhnik sent in a patch to change the transaction lock behavior from long chains to pairs, which eliminates a quadratic behavior.

Pavan Deolasee sent in another revision of a patch to implement MERGE.

Ashutosh Bapat sent in another revision of a patch to improve the partition matching algorithm for partition-wise joins.

Takayuki Tsunakawa sent in a patch to produce a crash dump before main() on Windows.

Amul Sul sent in a patch to fix a server crash in the pg_replication_slot_advance function.

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

Anna Akenteva sent in a patch to fix an issue where larger strings can be loaded into a table than selected from it.

Tom Lane and Marina Polyakova traded patches to stabilize some regression test outputs.

Joe Conway sent in two revisions of a patch to ensure that all catalog tables that need them have appropriate TOAST tables.

Arseny Sher sent in a patch to start decoding from a restart_lsn which definitely exists.

Oleg Ivanov sent in another revision of a patch to implement generic WAL compression.

David Rowley sent in another revision of a patch to improve runtime partition pruning.

Micha�l Paquier sent in another revision of a patch to prevent SSL and LDAP tests from running without support in build and add PROVE_EXTRA_ALLOWED to control optional test suites.

Micha�l Paquier sent in a patch to documenting PROVE_TESTS in section of TAP tests.

Fabien COELHO sent in two revisions of a patch to pgbench to enable specifying scale as a size.

David Rowley sent in two revisions of a patch to make PartitionClauseInfo a nonnode type.