Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 28 Oct, 2019 1 commit
    • Michael Paquier's avatar
      Fix dependency handling at swap phase of REINDEX CONCURRENTLY · 68ac9cf2
      Michael Paquier authored 
      When swapping the dependencies of the old and new indexes, the code has
been correctly switching all links in pg_depend from the old to the new
index for both referencing and referenced entries.  However it forgot
the fact that the new index may itself have existing entries in
pg_depend, like references to the parent table attributes.  This
resulted in duplicated entries in pg_depend after running REINDEX
CONCURRENTLY.

Fix this problem by removing any existing entries in pg_depend on the
new index before switching the dependencies of the old index to the new
one.  More regression tests are added to check the consistency of
entries in pg_depend for indexes, including partition indexes.

Author: Michael Paquier
Discussion: https://postgr.es/m/20191025064318.GF8671@paquier.xyz
Backpatch-through: 12
      68ac9cf2
  3. 27 Oct, 2019 1 commit
  5. 26 Oct, 2019 3 commits
    • Noah Misch's avatar
      Fix copy-paste defect in comment. · b8045213
      Noah Misch authored 
      Commit a7471bd8 introduced it.
      b8045213
    • Noah Misch's avatar
      Update comment about __sync_lock_test_and_set() bug. · e653c714
      Noah Misch authored 
      State the earliest known fixed version, so we can someday judge the
workaround to be obsolete.
      e653c714
    • Tom Lane's avatar
      Doc: improve documentation of configuration settings that have units. · cfb75590
      Tom Lane authored 
      When we added the GUC units feature, we didn't make any great effort
to adjust the documentation of individual GUCs; they tended to still
say things like "this is the number of milliseconds that ...", even
though users might prefer to write some other units, and SHOW might
even show the value in other units.  Commit 6c9fb69f made an effort
to improve this situation, but I thought it made things less readable
by injecting units information in mid-sentence.  It also wasn't very
consistent, and did not touch all the GUCs that have units.

To improve matters, standardize on the phrasing "If this value is
specified without units, it is taken as <units>".  Also, try to
standardize where this is mentioned, right before the specification
of the default.  (In a couple of places, doing that would've required
more rewriting than seemed justified, so I wasn't 100% consistent
about that.)  I also tried to use the phrases "amount of time",
"amount of memory", etc rather than describing the contents of GUCs
in other ways, as those were the majority usage in places that weren't
overcommitting to a particular unit.  (I left "length of time" alone
in a couple of places, though.)

I failed to resist the temptation to copy-edit some awkward text, too.

Backpatch to v12, like 6c9fb69f, mainly because v12 hasn't diverged
much from HEAD yet.

Discussion: https://postgr.es/m/15882.1571942223@sss.pgh.pa.us
      cfb75590
  7. 25 Oct, 2019 10 commits
    • Peter Eisentraut's avatar
      Remove obsolete information schema tables · 2fc2a88e
      Peter Eisentraut authored 
      Remove SQL_LANGUAGES, which was eliminated in SQL:2008, and
SQL_PACKAGES and SQL_SIZING_PROFILES, which were eliminated in
SQL:2011.  Since they were dropped by the SQL standard, the
information in them was no longer updated and therefore no longer
useful.

This also removes the feature-package association information in
sql_feature_packages.txt, but for the time begin we are keeping the
information which features are in the Core package (that is, mandatory
SQL features).  Maybe at some point someone wants to invent a way to
store that that does not involve using the "package" mechanism
anymore.

Discussion https://www.postgresql.org/message-id/flat/91334220-7900-071b-9327-0c6ecd012017%402ndquadrant.com
      2fc2a88e
    • Tom Lane's avatar
      Avoid failure when selecting a namespace node in XMLTABLE. · 592a1632
      Tom Lane authored 
      It appears that libxml2 doesn't bother to set the "children" field of
an XML_NAMESPACE_DECL node to null; that field just contains garbage.
In v10 and v11, this can result in a crash in XMLTABLE().  The rewrite
done in commit 251cf2e2 fixed this, somewhat accidentally, in v12.
We're not going to back-patch 251cf2e2, however.  The case apparently
doesn't have wide use, so rather than risk introducing other problems,
just add a safety check to throw an error.

Even though no bug manifests in v12/HEAD, add the relevant test case
there too, to prevent future regressions.

Chapman Flack (per private report)
      592a1632
    • Peter Eisentraut's avatar
      doc: Use proper em and en dashes · cbe63d02
      Peter Eisentraut authored
      cbe63d02
    • Tom Lane's avatar
      Revert "Revert part of commit dddf4cdc." · ee201520
      Tom Lane authored 
      This reverts commit c114229c.
Commit 1408d5d8 should make it
safe to include these headers in the natural order.
      ee201520
    • Tom Lane's avatar
      Get rid of useless/dangerous redefinition of bool in ECPG. · 1408d5d8
      Tom Lane authored 
      pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE".  The latter two are just plain unused, and have been for
awhile.  The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using <stdbool.h>.
However, it then defined bool as "char"; since commit d26a810e that
conflicts with c.h's desire to use "unsigned char".  We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but dddf4cdc exposed that it was problematic.

To fix, let's just get rid of these definitions.  They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool.  (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)

This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.

Back-patch to v12 where d26a810e came in, to forestall any unpleasant
surprises from future back-patched bug fixes.

Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
      1408d5d8
    • Tom Lane's avatar
      Improve management of statement timeouts. · 22f6f2c1
      Tom Lane authored 
      Commit f8e5f156 added private state in postgres.c to track whether
a statement timeout is running.  This seems like bad design to me;
timeout.c's private state should be the single source of truth about
that.  We already fixed one bug associated with failure to keep those
states in sync (cf. be42015f), and I've got little faith that we
won't find more in future.  So get rid of postgres.c's local variable
by exposing a way to ask timeout.c whether a timeout is running.
(Obviously, such an inquiry is subject to race conditions, but it
seems fine for the purpose at hand.)

To make get_timeout_active() as cheap as possible, add a flag in
the per-timeout struct showing whether that timeout is active.
This allows some small savings elsewhere in timeout.c, mainly
elimination of unnecessary searches of the active_timeouts array.

While at it, fix enable_statement_timeout to not call disable_timeout
when statement_timeout is 0 and the timeout is not running.  This
avoids a useless deschedule-and-reschedule-timeouts cycle, which
represents a significant savings (at least one kernel call) when
there is any other active timeout.  Right now, there usually isn't,
but there are proposals around to change that.

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
      22f6f2c1
    • Tom Lane's avatar
      Reset statement_timeout between queries of a multi-query string. · 2b2bacdc
      Tom Lane authored 
      Historically, we started the timer (if StatementTimeout > 0) at the
beginning of a simple-Query message and usually let it run until the
end, so that the timeout limit applied to the entire query string,
and intra-string changes of the statement_timeout GUC had no effect.
But, confusingly, a COMMIT within the string would reset the state
and allow a fresh timeout cycle to start with the current setting.

Commit f8e5f156 changed the behavior of statement_timeout for extended
query protocol, and as an apparently-unintended side effect, a change in
the statement_timeout GUC during a multi-statement simple-Query message
might have an effect immediately --- but only if it was going from
"disabled" to "enabled".

This is all pretty confusing, not to mention completely undocumented.
Let's change things so that the timeout is always reset between queries
of a multi-query string, whether they're transaction control commands
or not.  Thus the active timeout setting is applied to each query in
the string, separately.  This costs a few more cycles if statement_timeout
is active, but it provides much more intuitive behavior, especially if one
changes statement_timeout in one of the queries of the string.

Also, add something to the documentation to explain all this.

Per bug #16035 from Raj Mohite.  Although this is a bug fix, I'm hesitant
to back-patch it; conceivably somebody has worked out the old behavior
and is depending on it.  (But note that this change should make the
behavior less restrictive in most cases, since the timeout will now
be applied to shorter segments of code.)

Discussion: https://postgr.es/m/16035-456e6e69ebfd4374@postgresql.org
      2b2bacdc
    • Amit Kapila's avatar
      Revert part of commit dddf4cdc. · c114229c
      Amit Kapila authored 
      The commit dddf4cdc tries to ensure that the Postgres header file
inclusions are in order based on their ASCII value.  However, in one of
the case there is a header file dependency due to which we can't maintain
such order.

Author: Amit Kapila
Discussion: https://postgr.es/m/E1iNpHW-000855-1u@gemulon.postgresql.org
      c114229c
    • Amit Kapila's avatar
      Make the order of the header file includes consistent in non-backend modules. · dddf4cdc
      Amit Kapila authored 
      Similar to commit 7e735035, this commit makes the order of header file
inclusion consistent for non-backend modules.

In passing, fix the case where we were using angle brackets (<>) for the
local module includes instead of quotes ("").

Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
      dddf4cdc
    • Michael Paquier's avatar
      Handle interrupts within a transaction context in REINDEX CONCURRENTLY · 8270a0d9
      Michael Paquier authored 
      Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed.  This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.

This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly.  Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com
Backpatch-through: 12
      8270a0d9
  9. 24 Oct, 2019 3 commits
  11. 23 Oct, 2019 6 commits
  13. 22 Oct, 2019 2 commits
  15. 21 Oct, 2019 9 commits
  17. 20 Oct, 2019 1 commit
  19. 19 Oct, 2019 4 commits