1. 23 Jan, 2020 2 commits
  2. 22 Jan, 2020 4 commits
    • Alvaro Herrera's avatar
      Add BRIN test case · 611ce856
      Alvaro Herrera authored
      This test case was sketched in commit message 4c870109 to explain an
      ancient bug; it translates to a coverage increase, so add it to the BRIN
      regression tests.
      611ce856
    • Fujii Masao's avatar
      Add GUC ignore_invalid_pages. · 41c184bc
      Fujii Masao authored
      Detection of WAL records having references to invalid pages
      during recovery causes PostgreSQL to raise a PANIC-level error,
      aborting the recovery. Setting ignore_invalid_pages to on causes
      the system to ignore those WAL records (but still report a warning),
      and continue recovery. This behavior may cause crashes, data loss,
      propagate or hide corruption, or other serious problems.
      However, it may allow you to get past the PANIC-level error,
      to finish the recovery, and to cause the server to start up.
      
      Author: Fujii Masao
      Reviewed-by: Michael Paquier
      Discussion: https://www.postgresql.org/message-id/CAHGQGwHCK6f77yeZD4MHOnN+PaTf6XiJfEB+Ce7SksSHjeAWtg@mail.gmail.com
      41c184bc
    • Amit Kapila's avatar
      Fix the computation of max dead tuples during the vacuum. · 79a3efb8
      Amit Kapila authored
      In commit 40d964ec, we changed the way memory is allocated for dead
      tuples but forgot to update the place where we compute the maximum
      number of dead tuples.  This could lead to invalid memory requests.
      
      Reported-by: Andres Freund
      Diagnosed-by: Andres Freund
      Author: Masahiko Sawada
      Reviewed-by: Amit Kapila and Dilip Kumar
      Discussion: https://postgr.es/m/20200121060020.e3cr7s7fj5rw4lok@alap3.anarazel.de
      79a3efb8
    • Michael Paquier's avatar
      Fix concurrent indexing operations with temporary tables · a904abe2
      Michael Paquier authored
      Attempting to use CREATE INDEX, DROP INDEX or REINDEX with CONCURRENTLY
      on a temporary relation with ON COMMIT actions triggered unexpected
      errors because those operations use multiple transactions internally to
      complete their work.  Here is for example one confusing error when using
      ON COMMIT DELETE ROWS:
      ERROR:  index "foo" already contains data
      
      Issues related to temporary relations and concurrent indexing are fixed
      in this commit by enforcing the non-concurrent path to be taken for
      temporary relations even if using CONCURRENTLY, transparently to the
      user.  Using a non-concurrent path does not matter in practice as locks
      cannot be taken on a temporary relation by a session different than the
      one owning the relation, and the non-concurrent operation is more
      effective.
      
      The problem exists with REINDEX since v12 with the introduction of
      CONCURRENTLY, and with CREATE/DROP INDEX since CONCURRENTLY exists for
      those commands.  In all supported versions, this caused only confusing
      error messages to be generated.  Note that with REINDEX, it was also
      possible to issue a REINDEX CONCURRENTLY for a temporary relation owned
      by a different session, leading to a server crash.
      
      The idea to enforce transparently the non-concurrent code path for
      temporary relations comes originally from Andres Freund.
      
      Reported-by: Manuel Rigger
      Author: Michael Paquier, Heikki Linnakangas
      Reviewed-by: Andres Freund, Álvaro Herrera, Heikki Linnakangas
      Discussion: https://postgr.es/m/CA+u7OA6gP7YAeCguyseusYcc=uR8+ypjCcgDDCTzjQ+k6S9ksQ@mail.gmail.com
      Backpatch-through: 9.4
      a904abe2
  3. 21 Jan, 2020 3 commits
    • Tom Lane's avatar
      Clarify behavior of adding and altering a column in same ALTER command. · 9b9c5f27
      Tom Lane authored
      The behavior of something like
      
      ALTER TABLE transactions
        ADD COLUMN status varchar(30) DEFAULT 'old',
        ALTER COLUMN status SET default 'current';
      
      is to fill existing table rows with 'old', not 'current'.  That's
      intentional and desirable for a couple of reasons:
      
      * It makes the behavior the same whether you merge the sub-commands
      into one ALTER command or give them separately;
      
      * If we applied the new default while filling the table, there would
      be no way to get the existing behavior in one SQL command.
      
      The same reasoning applies in cases that add a column and then
      manipulate its GENERATED/IDENTITY status in a second sub-command,
      since the generation expression is really just a kind of default.
      However, that wasn't very obvious (at least not to me; earlier in
      the referenced discussion thread I'd thought it was a bug to be
      fixed).  And it certainly wasn't documented.
      
      Hence, add documentation, code comments, and a test case to clarify
      that this behavior is all intentional.
      
      In passing, adjust ATExecAddColumn's defaults-related relkind check
      so that it matches up exactly with ATRewriteTables, instead of being
      effectively (though not literally) the negated inverse condition.
      The reasoning can be explained a lot more concisely that way, too
      (not to mention that the comment now matches the code, which it
      did not before).
      
      Discussion: https://postgr.es/m/10365.1558909428@sss.pgh.pa.us
      9b9c5f27
    • Andres Freund's avatar
      Fix edge case leading to agg transitions skipping ExecAggTransReparent() calls. · affdde2e
      Andres Freund authored
      The code checking whether an aggregate transition value needs to be
      reparented into the current context has always only compared the
      transition return value with the previous transition value by datum,
      i.e. without regard for NULLness.  This normally works, because when
      the transition function returns NULL (via fcinfo->isnull), it'll
      return a value that won't be the same as its input value.
      
      But there's no hard requirement that that's the case. And it turns
      out, it's possible to hit this case (see discussion or reproducers),
      leading to a non-null transition value not being reparented, followed
      by a crash caused by that.
      
      Instead of adding another comparison of NULLness, instead have
      ExecAggTransReparent() ensure that pergroup->transValue ends up as 0
      when the new transition value is NULL. That avoids having to add an
      additional branch to the much more common cases of the transition
      function returning the old transition value (which is a pointer in
      this case), and when the new value is different, but not NULL.
      
      In branches since 69c3936a, also deduplicate the reparenting code
      between the expression evaluation based transitions, and the path for
      ordered aggregates.
      
      Reported-By: Teodor Sigaev, Nikita Glukhov
      Author: Andres Freund
      Discussion: https://postgr.es/m/bd34e930-cfec-ea9b-3827-a8bc50891393@sigaev.ru
      Backpatch: 9.4-, this issue has existed since at least 7.4
      affdde2e
    • Michael Paquier's avatar
      Add GUC variables for stat tracking and timeout as PGDLLIMPORT · 62c9b522
      Michael Paquier authored
      This helps integration of extensions with Windows.  The following
      parameters are changed:
      - idle_in_transaction_session_timeout (9.6 and newer versions)
      - lock_timeout
      - statement_timeout
      - track_activities
      - track_counts
      - track_functions
      
      Author: Pascal Legrand
      Reviewed-by: Amit Kamila, Julien Rouhaud, Michael Paquier
      Discussion: https://postgr.es/m/1579298868581-0.post@n3.nabble.com
      Backpatch-through: 9.4
      62c9b522
  4. 20 Jan, 2020 5 commits
    • Tom Lane's avatar
      Further tweaking of jsonb_set_lax(). · 31f403e9
      Tom Lane authored
      Some buildfarm members were still warning about this, because in
      9c679a08 I'd missed decorating one of the ereport() code paths
      with a dummy return.
      
      Also, adjust the error messages to be more in line with project
      style guide.
      31f403e9
    • Tom Lane's avatar
      Fix pg_dump's sigTermHandler() to use _exit() not exit(). · cd23a201
      Tom Lane authored
      sigTermHandler() tried to be careful to invoke only operations that
      are safe to do in a signal handler.  But for some reason we forgot
      that exit(3) is not among those, because it calls atexit handlers
      that might do various random things.  (pg_dump itself installs no
      atexit handlers, but e.g. OpenSSL does.)  That led to crashes or
      lockups when attempting to terminate a parallel dump or restore
      via a signal.
      
      Fix by calling _exit() instead.
      
      Per bug #16199 from Raúl Marín.  Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/16199-cb2f121146a96f9b@postgresql.org
      cd23a201
    • Heikki Linnakangas's avatar
      Fix crash in BRIN inclusion op functions, due to missing datum copy. · 4c870109
      Heikki Linnakangas authored
      The BRIN add_value() and union() functions need to make a longer-lived
      copy of the argument, if they want to store it in the BrinValues struct
      also passed as argument. The functions for the "inclusion operator
      classes" used with box, range and inet types didn't take into account
      that the union helper function might return its argument as is, without
      making a copy. Check for that case, and make a copy if necessary. That
      case arises at least with the range_union() function, when one of the
      arguments is an 'empty' range:
      
      CREATE TABLE brintest (n numrange);
      CREATE INDEX brinidx ON brintest USING brin (n);
      INSERT INTO brintest VALUES ('empty');
      INSERT INTO brintest VALUES (numrange(0, 2^1000::numeric));
      INSERT INTO brintest VALUES ('(-1, 0)');
      
      SELECT brin_desummarize_range('brinidx', 0);
      SELECT brin_summarize_range('brinidx', 0);
      
      Backpatch down to 9.5, where BRIN was introduced.
      
      Discussion: https://www.postgresql.org/message-id/e6e1d6eb-0a67-36aa-e779-bcca59167c14%40iki.fi
      Reviewed-by: Emre Hasegeli, Tom Lane, Alvaro Herrera
      4c870109
    • Amit Kapila's avatar
      Allow vacuum command to process indexes in parallel. · 40d964ec
      Amit Kapila authored
      This feature allows the vacuum to leverage multiple CPUs in order to
      process indexes.  This enables us to perform index vacuuming and index
      cleanup with background workers.  This adds a PARALLEL option to VACUUM
      command where the user can specify the number of workers that can be used
      to perform the command which is limited by the number of indexes on a
      table.  Specifying zero as a number of workers will disable parallelism.
      This option can't be used with the FULL option.
      
      Each index is processed by at most one vacuum process.  Therefore parallel
      vacuum can be used when the table has at least two indexes.
      
      The parallel degree is either specified by the user or determined based on
      the number of indexes that the table has, and further limited by
      max_parallel_maintenance_workers.  The index can participate in parallel
      vacuum iff it's size is greater than min_parallel_index_scan_size.
      
      Author: Masahiko Sawada and Amit Kapila
      Reviewed-by: Dilip Kumar, Amit Kapila, Robert Haas, Tomas Vondra,
      Mahendra Singh and Sergei Kornilov
      Tested-by: Mahendra Singh and Prabhat Sahu
      Discussion:
      https://postgr.es/m/CAD21AoDTPMgzSkV4E3SFo1CH_x50bf5PqZFQf4jmqjk-C03BWg@mail.gmail.com
      https://postgr.es/m/CAA4eK1J-VoR9gzS5E75pcD-OH0mEyCdp8RihcwKrcuw7J-Q0+w@mail.gmail.com
      40d964ec
    • Tom Lane's avatar
      Fix out-of-memory handling in ecpglib. · 44f1fc8d
      Tom Lane authored
      ecpg_build_params() would crash on a null pointer dereference if
      realloc() failed, due to updating the persistent "stmt" struct
      too aggressively.  (Even without the crash, this would've leaked
      the old storage that we were trying to realloc.)
      
      Per Coverity.  This seems to have been broken in commit 0cc05079,
      so back-patch into v12.
      44f1fc8d
  5. 19 Jan, 2020 3 commits
  6. 18 Jan, 2020 2 commits
    • Tom Lane's avatar
      Doc: rearrange the documentation of binary-string functions. · 34a0a81b
      Tom Lane authored
      Rather than intermixing the discussion of text-string and binary-string
      functions, make a clean break, moving all discussion of binary-string
      operations into section 9.5.  This involves some duplication of
      function descriptions between 9.4 and 9.5, but it seems cleaner on the
      whole since the individual descriptions are clearer (and on the other
      side of the coin, it gets rid of some duplicated descriptions, too).
      
      Move the convert*/encode/decode functions to a separate table, because
      they don't quite seem to fit under the heading of "binary string
      functions".
      
      Also provide full documentation of the textual formats supported by
      encode() and decode() (which was the original goal of this patch
      series, many moons ago).
      
      Also move the table of built-in encoding conversions out of section 9.4,
      where it no longer had any relevance whatsoever, and put it into section
      23.3 about character sets.  I chose to put both that and table 23.2
      (multibyte-translation-table) into a new <sect2> so as not to break up
      the flow of discussion in 23.3.3.
      
      Also do a bunch of minor copy-editing on the function descriptions
      in 9.4 and 9.5.
      
      Karl Pinc, reviewed by Fabien Coelho, further hacking by me
      
      Discussion: https://postgr.es/m/20190304163347.7bca4897@slate.meme.com
      34a0a81b
    • Michael Paquier's avatar
      Add GUC checks for ssl_min_protocol_version and ssl_max_protocol_version · 41aadeeb
      Michael Paquier authored
      Mixing incorrect bounds set in the SSL context leads to confusing error
      messages generated by OpenSSL which are hard to act on.  New checks are
      added within the GUC machinery to improve the user experience as they
      apply to any SSL implementation, not only OpenSSL, and doing the checks
      beforehand avoids the creation of a SSL during a reload (or startup)
      which we know will never be used anyway.
      
      Backpatch down to 12, as those parameters have been introduced by
      e73e67c7.
      
      Author: Michael Paquier
      Reviewed-by: Daniel Gustafsson
      Discussion: https://postgr.es/m/20200114035420.GE1515@paquier.xyz
      Backpatch-through: 12
      41aadeeb
  7. 17 Jan, 2020 7 commits
    • Alexander Korotkov's avatar
      Avoid full scan of GIN indexes when possible · 4b754d6c
      Alexander Korotkov authored
      The strategy of GIN index scan is driven by opclass-specific extract_query
      method.  This method that needed search mode is GIN_SEARCH_MODE_ALL.  This
      mode means that matching tuple may contain none of extracted entries.  Simple
      example is '!term' tsquery, which doesn't need any term to exist in matching
      tsvector.
      
      In order to handle such scan key GIN calculates virtual entry, which contains
      all TIDs of all entries of attribute.  In fact this is full scan of index
      attribute.  And typically this is very slow, but allows to handle some queries
      correctly in GIN.  However, current algorithm calculate such virtual entry for
      each GIN_SEARCH_MODE_ALL scan key even if they are multiple for the same
      attribute.  This is clearly not optimal.
      
      This commit improves the situation by introduction of "exclude only" scan keys.
      Such scan keys are not capable to return set of matching TIDs.  Instead, they
      are capable only to filter TIDs produced by normal scan keys.  Therefore,
      each attribute should contain at least one normal scan key, while rest of them
      may be "exclude only" if search mode is GIN_SEARCH_MODE_ALL.
      
      The same optimization might be applied to the whole scan, not per-attribute.
      But that leads to NULL values elimination problem.  There is trade-off between
      multiple possible ways to do this.  We probably want to do this later using
      some cost-based decision algorithm.
      
      Discussion: https://postgr.es/m/CAOBaU_YGP5-BEt5Cc0%3DzMve92vocPzD%2BXiZgiZs1kjY0cj%3DXBg%40mail.gmail.com
      Author: Nikita Glukhov, Alexander Korotkov, Tom Lane, Julien Rouhaud
      Reviewed-by: Julien Rouhaud, Tomas Vondra, Tom Lane
      4b754d6c
    • Tom Lane's avatar
      Repair more failures with SubPlans in multi-row VALUES lists. · 41c6f9db
      Tom Lane authored
      Commit 9b63c13f turns out to have been fundamentally misguided:
      the parent node's subPlan list is by no means the only way in which
      a child SubPlan node can be hooked into the outer execution state.
      As shown in bug #16213 from Matt Jibson, we can also get short-lived
      tuple table slots added to the outer es_tupleTable list.  At this point
      I have little faith that there aren't other possible connections as
      well; the long time it took to notice this problem shows that this
      isn't a heavily-exercised situation.
      
      Therefore, revert that fix, returning to the coding that passed a
      NULL parent plan pointer down to the transiently-built subexpressions.
      That gives us a pretty good guarantee that they won't hook into the
      outer executor state in any way.  But then we need some other solution
      to make SubPlans work.  Adopt the solution speculated about in the
      previous commit's log message: do expression initialization at plan
      startup for just those VALUES rows containing SubPlans, abandoning the
      goal of reclaiming memory intra-query for those rows.  In practice it
      seems unlikely that queries containing a vast number of VALUES rows
      would be using SubPlans in them, so this should not give up much.
      
      (BTW, this test case also refutes my claim in connection with the prior
      commit that the issue only arises with use of LATERAL.  That was just
      wrong: some variants of SubLink always produce SubPlans.)
      
      As with previous patch, back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/16213-871ac3bc208ecf23@postgresql.org
      41c6f9db
    • Alvaro Herrera's avatar
      Set ReorderBufferTXN->final_lsn more eagerly · 15cac3a5
      Alvaro Herrera authored
      ... specifically, set it incrementally as each individual change is
      spilled down to disk.  This way, it is set correctly when the
      transaction disappears without trace, ie. without leaving an XACT_ABORT
      wal record.  (This happens when the server crashes midway through a
      transaction.)
      
      Failing to have final_lsn prevents ReorderBufferRestoreCleanup() from
      working, since it needs the final_lsn in order to know the endpoint of
      its iteration through spilled files.
      
      Commit df9f682c already tried to fix the problem, but it didn't set
      the final_lsn in all cases.  Revert that, since it's no longer needed.
      
      Author: Vignesh C
      Reviewed-by: Amit Kapila, Dilip Kumar
      Discussion: https://postgr.es/m/CALDaNm2CLk+K9JDwjYST0sPbGg5AQdvhUt0jbKyX_HdAE0jk3A@mail.gmail.com
      15cac3a5
    • Tomas Vondra's avatar
      Allocate freechunks bitmap as part of SlabContext · 543852fd
      Tomas Vondra authored
      The bitmap used by SlabCheck to cross-check free chunks in a block used
      to be allocated for each SlabCheck call, and was never freed. The memory
      leak could be fixed by simply adding a pfree call, but it's actually a
      bad idea to do any allocations in SlabCheck at all as it assumes the
      state of the memory management as a whole is sane.
      
      So instead we allocate the bitmap as part of SlabContext, which means
      we don't need to do any allocations in SlabCheck and the bitmap goes
      away together with the SlabContext.
      
      Backpatch to 10, where the Slab context was introduced.
      
      Author: Tomas Vondra
      Reported-by: Andres Freund
      Reviewed-by: Tom Lane
      Backpatch-through: 10
      Discussion: https://www.postgresql.org/message-id/20200116044119.g45f7pmgz4jmodxj%40alap3.anarazel.de
      543852fd
    • Andrew Dunstan's avatar
    • Andrew Dunstan's avatar
      Add a non-strict version of jsonb_set · a83586b5
      Andrew Dunstan authored
      jsonb_set_lax() is the same as jsonb_set, except that it takes and extra
      argument that specifies what to do if the value argument is NULL. The
      default is 'use_json_null'. Other possibilities are 'raise_exception',
      'return_target' and 'delete_key', all these behaviours having been
      suggested as reasonable by various users.
      
      Discussion: https://postgr.es/m/375873e2-c957-3a8d-64f9-26c43c2b16e7@2ndQuadrant.com
      
      Reviewed by: Pavel Stehule
      a83586b5
    • Michael Paquier's avatar
      Move OpenSSL routines for min/max protocol setting to src/common/ · f7cd5896
      Michael Paquier authored
      Two routines have been added in OpenSSL 1.1.0 to set the protocol bounds
      allowed within a given SSL context:
      - SSL_CTX_set_min_proto_version
      - SSL_CTX_set_max_proto_version
      
      As Postgres supports OpenSSL down to 1.0.1 (as of HEAD), equivalent
      replacements exist in the tree, which are only available for the
      backend.  A follow-up patch is planned to add control of the SSL
      protocol bounds for libpq, so move those routines to src/common/ so as
      libpq can use them.
      
      Author: Daniel Gustafsson
      Discussion: https://postgr.es/m/4F246AE3-A7AE-471E-BD3D-C799D3748E03@yesql.se
      f7cd5896
  8. 16 Jan, 2020 5 commits
    • Tom Lane's avatar
      Rationalize code placement between wchar.c, encnames.c, and mbutils.c. · 5afaa2e4
      Tom Lane authored
      Move all the backend-only code that'd crept into wchar.c and encnames.c
      into mbutils.c.
      
      To remove the last few #ifdef dependencies from wchar.c and encnames.c,
      also make the following changes:
      
      * Adjust get_encoding_name_for_icu to return NULL, not throw an error,
      for unsupported encodings.  Its sole caller can perfectly well throw an
      error instead.  (While at it, I also made this function and its sibling
      is_encoding_supported_by_icu proof against out-of-range encoding IDs.)
      
      * Remove the overlength-name error condition from pg_char_to_encoding.
      It's completely silly not to treat that just like any other
      the-name-is-not-in-the-table case.
      
      Also, get rid of pg_mic_mblen --- there's no obvious reason why
      conv.c shouldn't call pg_mule_mblen instead.
      
      Other than that, this is just code movement and comment-polishing with
      no functional changes.  Notably, I reordered declarations in pg_wchar.h
      to show which functions are frontend-accessible and which are not.
      
      Discussion: https://postgr.es/m/CA+TgmoYO8oq-iy8E02rD8eX25T-9SmyxKWqqks5OMHxKvGXpXQ@mail.gmail.com
      5afaa2e4
    • Tom Lane's avatar
      Update header comments for wchar.c and encnames.c. · 3d4cb5d6
      Tom Lane authored
      Bring these into common style (including having proper copyright
      notices) and adjust their self-declaration of where they live.
      
      Discussion: https://postgr.es/m/CA+TgmoYO8oq-iy8E02rD8eX25T-9SmyxKWqqks5OMHxKvGXpXQ@mail.gmail.com
      3d4cb5d6
    • Tom Lane's avatar
      Move wchar.c and encnames.c to src/common/. · e6afa891
      Tom Lane authored
      Formerly, various frontend directories symlinked these two sources
      and then built them locally.  That's an ancient, ugly hack, and
      we now have a much better way: put them into libpgcommon.
      So do that.  (The immediate motivation for this is the prospect
      of having to introduce still more symlinking if we don't.)
      
      This commit moves these two files absolutely verbatim, for ease of
      reviewing the git history.  There's some follow-on work to be done
      that will modify them a bit.
      
      Robert Haas, Tom Lane
      
      Discussion: https://postgr.es/m/CA+TgmoYO8oq-iy8E02rD8eX25T-9SmyxKWqqks5OMHxKvGXpXQ@mail.gmail.com
      e6afa891
    • Robert Haas's avatar
      Fix problems with "read only query" checks, and refactor the code. · 2eb34ac3
      Robert Haas authored
      Previously, check_xact_readonly() was responsible for determining
      which types of queries could not be run in a read-only transaction,
      standard_ProcessUtility() was responsibility for prohibiting things
      which were allowed in read only transactions but not in recovery, and
      utility commands were basically prohibited in bulk in parallel mode by
      calls to CommandIsReadOnly() in functions.c and spi.c.  This situation
      was confusing and error-prone. Accordingly, move all the checks to a
      new function ClassifyUtilityCommandAsReadOnly(), which determines the
      degree to which a given statement is read only.
      
      In the old code, check_xact_readonly() inadvertently failed to handle
      several statement types that actually should have been prohibited,
      specifically T_CreatePolicyStmt, T_AlterPolicyStmt, T_CreateAmStmt,
      T_CreateStatsStmt, T_AlterStatsStmt, and T_AlterCollationStmt.  As a
      result, thes statements were erroneously allowed in read only
      transactions, parallel queries, and standby operation. Generally, they
      would fail anyway due to some lower-level error check, but we
      shouldn't rely on that.  In the new code structure, future omissions
      of this type should cause ClassifyUtilityCommandAsReadOnly() to
      complain about an unrecognized node type.
      
      As a fringe benefit, this means we can allow certain types of utility
      commands in parallel mode, where it's safe to do so. This allows
      ALTER SYSTEM, CALL, DO, CHECKPOINT, COPY FROM, EXPLAIN, and SHOW.
      It might be possible to allow additional commands with more work
      and thought.
      
      Along the way, document the thinking process behind the current set
      of checks, as per discussion especially with Peter Eisentraut. There
      is some interest in revising some of these rules, but that seems
      like a job for another patch.
      
      Patch by me, reviewed by Tom Lane, Stephen Frost, and Peter
      Eisentraut.
      
      Discussion: http://postgr.es/m/CA+TgmoZ_rLqJt5sYkvh+JpQnfX0Y+B2R+qfi820xNih6x-FQOQ@mail.gmail.com
      2eb34ac3
    • Tom Lane's avatar
      Minor code beautification in regexp.c. · 0db7c670
      Tom Lane authored
      Remove duplicated code (apparently introduced by commit c8ea87e4).
      Also get rid of some PG_USED_FOR_ASSERTS_ONLY variables we don't
      really need to have.
      
      Li Japin, Tom Lane
      
      Discussion: https://postgr.es/m/PS1PR0601MB3770A5595B6E5E3FD6F35724B6360@PS1PR0601MB3770.apcprd06.prod.outlook.com
      0db7c670
  9. 15 Jan, 2020 5 commits
    • Tom Lane's avatar
      Restructure ALTER TABLE execution to fix assorted bugs. · 1281a5c9
      Tom Lane authored
      We've had numerous bug reports about how (1) IF NOT EXISTS clauses in
      ALTER TABLE don't behave as-expected, and (2) combining certain actions
      into one ALTER TABLE doesn't work, though executing the same actions as
      separate statements does.  This patch cleans up all of the cases so far
      reported from the field, though there are still some oddities associated
      with identity columns.
      
      The core problem behind all of these bugs is that we do parse analysis
      of ALTER TABLE subcommands too soon, before starting execution of the
      statement.  The root of the bugs in group (1) is that parse analysis
      schedules derived commands (such as a CREATE SEQUENCE for a serial
      column) before it's known whether the IF NOT EXISTS clause should cause
      a subcommand to be skipped.  The root of the bugs in group (2) is that
      earlier subcommands may change the catalog state that later subcommands
      need to be parsed against.
      
      Hence, postpone parse analysis of ALTER TABLE's subcommands, and do
      that one subcommand at a time, during "phase 2" of ALTER TABLE which
      is the phase that does catalog rewrites.  Thus the catalog effects
      of earlier subcommands are already visible when we analyze later ones.
      (The sole exception is that we do parse analysis for ALTER COLUMN TYPE
      subcommands during phase 1, so that their USING expressions can be
      parsed against the table's original state, which is what we need.
      Arguably, these bugs stem from falsely concluding that because ALTER
      COLUMN TYPE must do early parse analysis, every other command subtype
      can too.)
      
      This means that ALTER TABLE itself must deal with execution of any
      non-ALTER-TABLE derived statements that are generated by parse analysis.
      Add a suitable entry point to utility.c to accept those recursive
      calls, and create a struct to pass through the information needed by
      the recursive call, rather than making the argument lists of
      AlterTable() and friends even longer.
      
      Getting this to work correctly required a little bit of fiddling
      with the subcommand pass structure, in particular breaking up
      AT_PASS_ADD_CONSTR into multiple passes.  But otherwise it's mostly
      a pretty straightforward application of the above ideas.
      
      Fixing the residual issues for identity columns requires refactoring of
      where the dependency link from an identity column to its sequence gets
      set up.  So that seems like suitable material for a separate patch,
      especially since this one is pretty big already.
      
      Discussion: https://postgr.es/m/10365.1558909428@sss.pgh.pa.us
      1281a5c9
    • Alvaro Herrera's avatar
      Report progress of ANALYZE commands · a166d408
      Alvaro Herrera authored
      This uses the progress reporting infrastructure added by c16dc1ac,
      adding support for ANALYZE.
      Co-authored-by: default avatarÁlvaro Herrera <alvherre@alvh.no-ip.org>
      Co-authored-by: default avatarTatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
      Reviewed-by: Julien Rouhaud, Robert Haas, Anthony Nowocien, Kyotaro Horiguchi,
      	Vignesh C, Amit Langote
      a166d408
    • Peter Eisentraut's avatar
      Remove libpq.rc, use win32ver.rc for libpq · 16a4a3d5
      Peter Eisentraut authored
      For historical reasons, libpq used a separate libpq.rc file for the
      Windows builds while all other components use a common file
      win32ver.rc.  With a bit of tweaking, the libpq build can also use the
      win32ver.rc file.  This removes a bit of duplicative code.
      Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      Discussion: https://www.postgresql.org/message-id/flat/ad505e61-a923-e114-9f38-9867d161073f@2ndquadrant.com
      16a4a3d5
    • Michael Paquier's avatar
      Fix buggy logic in isTempNamespaceInUse() · ac5bdf62
      Michael Paquier authored
      The logic introduced in this routine as of 246a6c8f would report an
      incorrect result when a session calls it to check if the temporary
      namespace owned by the session is in use or not.  It is possible to
      optimize more the routine in this case to avoid a PGPROC lookup, but
      let's keep the logic simple.  As this routine is used only by autovacuum
      for now, there were no live bugs, still let's be correct for any future
      code involving it.
      
      Author: Michael Paquier
      Reviewed-by: Julien Rouhaud
      Discussion: https://postgr.es/m/20200113093703.GA41902@paquier.xyz
      Backpatch-through: 11
      ac5bdf62
    • Amit Kapila's avatar
      Introduce IndexAM fields for parallel vacuum. · 4d8a8d0c
      Amit Kapila authored
      Introduce new fields amusemaintenanceworkmem and amparallelvacuumoptions
      in IndexAmRoutine for parallel vacuum.  The amusemaintenanceworkmem tells
      whether a particular IndexAM uses maintenance_work_mem or not.  This will
      help in controlling the memory used by individual workers as otherwise,
      each worker can consume memory equal to maintenance_work_mem.  The
      amparallelvacuumoptions tell whether a particular IndexAM participates in
      a parallel vacuum and if so in which phase (bulkdelete, vacuumcleanup) of
      vacuum.
      
      Author: Masahiko Sawada and Amit Kapila
      Reviewed-by: Dilip Kumar, Amit Kapila, Tomas Vondra and Robert Haas
      Discussion:
      https://postgr.es/m/CAD21AoDTPMgzSkV4E3SFo1CH_x50bf5PqZFQf4jmqjk-C03BWg@mail.gmail.com
      https://postgr.es/m/CAA4eK1LmcD5aPogzwim5Nn58Ki+74a6Edghx4Wd8hAskvHaq5A@mail.gmail.com
      4d8a8d0c
  10. 14 Jan, 2020 4 commits