1. 02 Feb, 2021 7 commits
    • Tom Lane's avatar
      Retire findoidjoins. · ef3d4613
      Tom Lane authored
      In the wake of commit 62f34097, we no longer need this tool.
      
      Discussion: https://postgr.es/m/3240355.1612129197@sss.pgh.pa.us
      ef3d4613
    • Tom Lane's avatar
      Build in some knowledge about foreign-key relationships in the catalogs. · 62f34097
      Tom Lane authored
      This follows in the spirit of commit dfb75e47, which created primary
      key and uniqueness constraints to improve the visibility of constraints
      imposed on the system catalogs.  While our catalogs contain many
      foreign-key-like relationships, they don't quite follow SQL semantics,
      in that the convention for an omitted reference is to write zero not
      NULL.  Plus, we have some cases in which there are arrays each of whose
      elements is supposed to be an FK reference; SQL has no way to model that.
      So we can't create actual foreign key constraints to describe the
      situation.  Nonetheless, we can collect and use knowledge about these
      relationships.
      
      This patch therefore adds annotations to the catalog header files to
      declare foreign-key relationships.  (The BKI_LOOKUP annotations cover
      simple cases, but we weren't previously distinguishing which such
      columns are allowed to contain zeroes; we also need new markings for
      multi-column FK references.)  Then, Catalog.pm and genbki.pl are
      taught to collect this information into a table in a new generated
      header "system_fk_info.h".  The only user of that at the moment is
      a new SQL function pg_get_catalog_foreign_keys(), which exposes the
      table to SQL.  The oidjoins regression test is rewritten to use
      pg_get_catalog_foreign_keys() to find out which columns to check.
      Aside from removing the need for manual maintenance of that test
      script, this allows it to cover numerous relationships that were not
      checked by the old implementation based on findoidjoins.  (As of this
      commit, 217 relationships are checked by the test, versus 181 before.)
      
      Discussion: https://postgr.es/m/3240355.1612129197@sss.pgh.pa.us
      62f34097
    • Tom Lane's avatar
      Doc: consistently identify OID catalog columns that can be zero. · 47933140
      Tom Lane authored
      Not all OID-reference columns that can contain zeroes (indicating
      "no reference") were explicitly marked in catalogs.sgml.  Fix that,
      and try to make the style a little more consistent while at it ---
      for example, consistently write "zero" not "0" for these cases.
      
      Joel Jacobson and Tom Lane
      
      Discussion: https://postgr.es/m/4ed9a372-7bf9-479a-926c-ae8e774717a8@www.fastmail.com
      47933140
    • Tom Lane's avatar
      Remove extra increment of plpgsql's statement counter for FOR loops. · dfcc46fe
      Tom Lane authored
      This left gaps in the internal statement numbering, which is not
      terribly harmful (else we'd have noticed sooner), but it's not
      great either.
      
      Oversight in bbd5c207; backpatch to v12 where that came in.
      
      Pavel Stehule
      
      Discussion: https://postgr.es/m/CAFj8pRDXyQaJmpotNTQVc-t-WxdWZC35V2PnmwOaV1-taidFWA@mail.gmail.com
      dfcc46fe
    • Tom Lane's avatar
      Fix ancient memory leak in contrib/auto_explain. · 5c0f7cc5
      Tom Lane authored
      The ExecutorEnd hook is invoked in a context that could be quite
      long-lived, not the executor's own per-query context as I think
      we were sort of assuming.  Thus, any cruft generated while producing
      the EXPLAIN output could accumulate over multiple queries.  This can
      result in spectacular leakage if log_nested_statements is on, and
      even without that I'm surprised nobody complained before.
      
      To fix, just switch into the executor's context so that anything we
      allocate will be released when standard_ExecutorEnd frees the executor
      state.  We might as well nuke the code's retail pfree of the explain
      output string, too; that's laughably inadequate to the need.
      
      Japin Li, per report from Jeff Janes.  This bug is old, so
      back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com
      5c0f7cc5
    • Peter Eisentraut's avatar
      Improve confusing variable names · 1d71f3c8
      Peter Eisentraut authored
      The prototype calls the second argument of
      pgstat_progress_update_multi_param() "index", and some callers name
      their local variable that way.  But when the surrounding code deals
      with index relations, this is confusing, and in at least one case
      shadowed another variable that is referring to an index relation.
      Adjust those call sites to have clearer local variable naming, similar
      to existing callers in indexcmds.c.
      1d71f3c8
    • Michael Paquier's avatar
      Remove unused column atttypmod from initial tablesync query · 4ad31bb2
      Michael Paquier authored
      The initial tablesync done by logical replication used a query to fetch
      the information of a relation's columns that included atttypmod, but it
      was left unused.  This was added by 7c4f5240.
      
      Author: Euler Taveira
      Reviewed-by: Önder Kalacı, Amit Langote, Japin Li
      Discussion: https://postgr.es/m/CAHE3wggb715X+mK_DitLXF25B=jE6xyNCH4YOwM860JR7HarGQ@mail.gmail.com
      4ad31bb2
  2. 01 Feb, 2021 8 commits
  3. 31 Jan, 2021 5 commits
  4. 30 Jan, 2021 7 commits
    • Peter Eisentraut's avatar
      Add primary keys and unique constraints to system catalogs · dfb75e47
      Peter Eisentraut authored
      For those system catalogs that have a unique indexes, make a primary
      key and unique constraint, using ALTER TABLE ... PRIMARY KEY/UNIQUE
      USING INDEX.
      
      This can be helpful for GUI tools that look for a primary key, and it
      might in the future allow declaring foreign keys, for making schema
      diagrams.
      
      The constraint creation statements are automatically created by
      genbki.pl from DECLARE_UNIQUE_INDEX directives.  To specify which one
      of the available unique indexes is the primary key, use the new
      directive DECLARE_UNIQUE_INDEX_PKEY instead.  By convention, we
      usually make a catalog's OID column its primary key, if it has one.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/dc5f44d9-5ec1-a596-0251-dadadcdede98@2ndquadrant.com
      dfb75e47
    • Peter Eisentraut's avatar
      doc: Clarify status of SELECT INTO on reference page · 65330622
      Peter Eisentraut authored
      The documentation as well as source code comments weren't entirely
      clear whether SELECT INTO was truly deprecated (thus in theory
      destined to be removed eventually), or just a less recommended
      variant.  After discussion, it appears that other implementations also
      use SELECT INTO in direct SQL in a way similar to PostgreSQL, so it
      seems worth keeping it for compatibility.  Update the language in the
      documentation to that effect.
      
      Discussion: https://www.postgresql.org/message-id/flat/96dc0df3-e13a-a85d-d045-d6e2c85218da%40enterprisedb.com
      65330622
    • Peter Eisentraut's avatar
      Allow GRANTED BY clause in normal GRANT and REVOKE statements · 6aaaa76b
      Peter Eisentraut authored
      The SQL standard allows a GRANTED BY clause on GRANT and
      REVOKE (privilege) statements that can specify CURRENT_USER or
      CURRENT_ROLE.  In PostgreSQL, both of these are the default behavior.
      Since we already have all the parsing support for this for the
      GRANT (role) statement, we might as well add basic support for this
      for the privilege variant as well.  This allows us to check off SQL
      feature T332.  In the future, perhaps more interesting things could be
      done with this, too.
      Reviewed-by: default avatarSimon Riggs <simon@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/f2feac44-b4c5-f38f-3699-2851d6a76dc9@2ndquadrant.com
      6aaaa76b
    • Noah Misch's avatar
      Revive "snapshot too old" with wal_level=minimal and SET TABLESPACE. · 7da83415
      Noah Misch authored
      Given a permanent relation rewritten in the current transaction, the
      old_snapshot_threshold mechanism assumed the relation had never been
      subject to early pruning.  Hence, a query could fail to report "snapshot
      too old" when the rewrite followed an early truncation.  ALTER TABLE SET
      TABLESPACE is probably the only rewrite mechanism capable of exposing
      this bug.  REINDEX sets indcheckxmin, avoiding the problem.  CLUSTER has
      zeroed page LSNs since before old_snapshot_threshold existed, so
      old_snapshot_threshold has never cooperated with it.  ALTER TABLE
      ... SET DATA TYPE makes the table look empty to every past snapshot,
      which is strictly worse.  Back-patch to v13, where commit
      c6b92041 broke this.
      
      Kyotaro Horiguchi and Noah Misch
      
      Discussion: https://postgr.es/m/20210113.160705.2225256954956139776.horikyota.ntt@gmail.com
      7da83415
    • Noah Misch's avatar
      Fix error with CREATE PUBLICATION, wal_level=minimal, and new tables. · 360bd232
      Noah Misch authored
      CREATE PUBLICATION has failed spuriously when applied to a permanent
      relation created or rewritten in the current transaction.  Make the same
      change to another site having the same semantic intent; the second
      instance has no user-visible consequences.  Back-patch to v13, where
      commit c6b92041 broke this.
      
      Kyotaro Horiguchi
      
      Discussion: https://postgr.es/m/20210113.160705.2225256954956139776.horikyota.ntt@gmail.com
      360bd232
    • Noah Misch's avatar
      Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions. · 8a54e12a
      Noah Misch authored
      In a cluster having used CREATE INDEX CONCURRENTLY while having enabled
      prepared transactions, queries that use the resulting index can silently
      fail to find rows.  Fix this for future CREATE INDEX CONCURRENTLY by
      making it wait for prepared transactions like it waits for ordinary
      transactions.  This expands the VirtualTransactionId structure domain to
      admit prepared transactions.  It may be necessary to reindex to recover
      from past occurrences.  Back-patch to 9.5 (all supported versions).
      
      Andrey Borodin, reviewed (in earlier versions) by Tom Lane and Michael
      Paquier.
      
      Discussion: https://postgr.es/m/2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru
      8a54e12a
    • Fujii Masao's avatar
      postgres_fdw: Fix tests for CLOBBER_CACHE_ALWAYS. · f77717b2
      Fujii Masao authored
      The regression tests added in commits 708d165d and 411ae649 caused
      buildfarm failures when  CLOBBER_CACHE_ALWAYS was enabled.
      This commit stabilizes those tests.
      
      The foreign server connections established by postgres_fdw behaves
      differently depending on whether CLOBBER_CACHE_ALWAYS is enabled or not.
      If it's not enabled, those connections are cached. On the other hand,
      if it's enabled, when the connections are established outside transaction
      block, they are not cached (i.e., they are immediately closed at the end of
      query that established them). So the subsequent postgres_fdw_get_connections()
      cannot list those connections and postgres_fdw_disconnect() cannot close them
      (because they are already closed).
      
      When the connections are established inside transaction block, they are
      cached whether CLOBBER_CACHE_ALWAYS was enabled or not. But if it's enabled,
      they are immediately marked as invalid, otherwise not. This causes the
      subsequent postgres_fdw_get_connections() to return different result in
      "valid" column depending on whether CLOBBER_CACHE_ALWAYS was enabled or not.
      
      This commit prevents the above differences of behavior from
      affecting the regression tests.
      
      Per buildfarm failure on trilobite.
      
      Original patch by Bharath Rupireddy. I (Fujii Masao) extracted
      the regression test fix from that and revised it a bit.
      
      Reported-by: Tom Lane
      Author: Bharath Rupireddy
      Reviewed-by: Fujii Masao
      Discussion: https://postgr.es/m/2688508.1611865371@sss.pgh.pa.us
      f77717b2
  5. 29 Jan, 2021 7 commits
  6. 28 Jan, 2021 6 commits
    • Tom Lane's avatar
      Silence another gcc 11 warning. · 1046dbed
      Tom Lane authored
      Per buildfarm and local experimentation, bleeding-edge gcc isn't
      convinced that the MemSet in reorder_function_arguments() is safe.
      Shut it up by adding an explicit check that pronargs isn't negative,
      and by changing MemSet to memset.  (It appears that either change is
      enough to quiet the warning at -O2, but let's do both to be sure.)
      1046dbed
    • Alvaro Herrera's avatar
      Remove bogus restriction from BEFORE UPDATE triggers · 6f5c8a8e
      Alvaro Herrera authored
      In trying to protect the user from inconsistent behavior, commit
      487e9861 "Enable BEFORE row-level triggers for partitioned tables"
      tried to prevent BEFORE UPDATE FOR EACH ROW triggers from moving the row
      from one partition to another.  However, it turns out that the
      restriction is wrong in two ways: first, it fails spuriously, preventing
      valid situations from working, as in bug #16794; and second, they don't
      protect from any misbehavior, because tuple routing would cope anyway.
      
      Fix by removing that restriction.
      
      We keep the same restriction on BEFORE INSERT FOR EACH ROW triggers,
      though.  It is valid and useful there.  In the future we could remove it
      by having tuple reroute work for inserts as it does for updates.
      
      Backpatch to 13.
      
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Reported-by: default avatarPhillip Menke <pg@pmenke.de>
      Discussion: https://postgr.es/m/16794-350a655580fbb9ae@postgresql.org
      6f5c8a8e
    • Tom Lane's avatar
      Fix hash partition pruning with asymmetric partition sets. · 1d9351a8
      Tom Lane authored
      perform_pruning_combine_step() was not taught about the number of
      partition indexes used in hash partitioning; more embarrassingly,
      get_matching_hash_bounds() also had it wrong.  These errors are masked
      in the common case where all the partitions have the same modulus
      and no partition is missing.  However, with missing or unequal-size
      partitions, we could erroneously prune some partitions that need
      to be scanned, leading to silently wrong query answers.
      
      While a minimal-footprint fix for this could be to export
      get_partition_bound_num_indexes and make the incorrect functions use it,
      I'm of the opinion that that function should never have existed in the
      first place.  It's not reasonable data structure design that
      PartitionBoundInfoData lacks any explicit record of the length of
      its indexes[] array.  Perhaps that was all right when it could always
      be assumed equal to ndatums, but something should have been done about
      it as soon as that stopped being true.  Putting in an explicit
      "nindexes" field makes both partition_bounds_equal() and
      partition_bounds_copy() simpler, safer, and faster than before,
      and removes explicit knowledge of the number-of-partition-indexes
      rules from some other places too.
      
      This change also makes get_hash_partition_greatest_modulus obsolete.
      I left that in place in case any external code uses it, but no core
      code does anymore.
      
      Per bug #16840 from Michał Albrycht.  Back-patch to v11 where the
      hash partitioning code came in.  (In the back branches, add the new
      field at the end of PartitionBoundInfoData to minimize ABI risks.)
      
      Discussion: https://postgr.es/m/16840-571a22976f829ad4@postgresql.org
      1d9351a8
    • Tom Lane's avatar
      Make ecpg's rjulmdy() and rmdyjul() agree with their declarations. · 1b242f42
      Tom Lane authored
      We had "short *mdy" in the extern declarations, but "short mdy[3]"
      in the actual function definitions.  Per C99 these are equivalent,
      but recent versions of gcc have started to issue warnings about
      the inconsistency.  Clean it up before the warnings get any more
      widespread.
      
      Back-patch, in case anyone wants to build older PG versions with
      bleeding-edge compilers.
      
      Discussion: https://postgr.es/m/2401575.1611764534@sss.pgh.pa.us
      1b242f42
    • Alvaro Herrera's avatar
      pgbench: Remove dead code · 6819b904
      Alvaro Herrera authored
      doConnect() never returns connections in state CONNECTION_BAD, so
      checking for that is pointless.  Remove the code that does.
      
      This code has been dead since ba708ea3, 20 years ago.
      
      Discussion: https://postgr.es/m/20210126195224.GA20361@alvherre.pgsqlReviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      6819b904
    • Peter Eisentraut's avatar
      Remove gratuitous uses of deprecated SELECT INTO · b034ef9b
      Peter Eisentraut authored
      CREATE TABLE AS has been preferred over SELECT INTO (outside of ecpg
      and PL/pgSQL) for a long time.  There were still a few uses of SELECT
      INTO in tests and documentation, some old, some more recent.  This
      changes them to CREATE TABLE AS.  Some occurrences in the tests remain
      where they are specifically testing SELECT INTO parsing or similar.
      
      Discussion: https://www.postgresql.org/message-id/flat/96dc0df3-e13a-a85d-d045-d6e2c85218da%40enterprisedb.com
      b034ef9b