1. 26 Dec, 2018 1 commit
    • Tom Lane's avatar
      Fix portability failure introduced in commits d2b0b60e et al. · e9fcfed3
      Tom Lane authored
      I made a frontend fprintf() format use %m, forgetting that that's only
      safe in HEAD not the back branches; prior to 96bf88d5 and d6c55de1,
      it would work on glibc platforms but not elsewhere.  Revert to using
      %s ... strerror(errno) as the code did before.
      
      We could have left HEAD as-is, but for code consistency across branches,
      I chose to apply this patch there too.
      
      Per Coverity and a few buildfarm members.
      e9fcfed3
  2. 25 Dec, 2018 1 commit
  3. 24 Dec, 2018 1 commit
    • Michael Paquier's avatar
      Prioritize history files when archiving · b981df4c
      Michael Paquier authored
      At the end of recovery for the post-promotion process, a new history
      file is created followed by the last partial segment of the previous
      timeline.  Based on the timing, the archiver would first try to archive
      the last partial segment and then the history file.  This can delay the
      detection of a new timeline taken, particularly depending on the time it
      takes to transfer the last partial segment as it delays the moment the
      history file of the new timeline gets archived.  This can cause promoted
      standbys to use the same timeline as one already taken depending on the
      circumstances if multiple instances look at archives at the same
      location.
      
      This commit changes the order of archiving so as history files are
      archived in priority over other file types, which reduces the likelihood
      of the same timeline being taken (still not reducing the window to
      zero), and it makes the archiver behave more consistently with the
      startup process doing its post-promotion business.
      
      Author: David Steele
      Reviewed-by: Michael Paquier, Kyotaro Horiguchi
      Discussion: https://postgr.es/m/929068cf-69e1-bba2-9dc0-e05986aed471@pgmasters.net
      Backpatch-through: 9.5
      b981df4c
  4. 23 Dec, 2018 2 commits
  5. 22 Dec, 2018 3 commits
  6. 20 Dec, 2018 9 commits
    • Alexander Korotkov's avatar
      Check for conflicting queries during replay of gistvacuumpage() · c952eae5
      Alexander Korotkov authored
      013ebc0a implements so-called GiST microvacuum.  That is gistgettuple() marks
      index tuples as dead when kill_prior_tuple is set.  Later, when new tuple
      insertion claims page space, those dead index tuples are physically deleted
      from page.  When this deletion is replayed on standby, it might conflict with
      read-only queries.  But 013ebc0a doesn't handle this.  That may lead to
      disappearance of some tuples from read-only snapshots on standby.
      
      This commit implements resolving of conflicts between replay of GiST microvacuum
      and standby queries.  On the master we implement new WAL record type
      XLOG_GIST_DELETE, which comprises necessary information.  On stable releases
      we've to be tricky to keep WAL compatibility.  Information required for conflict
      processing is just appended to data of XLOG_GIST_PAGE_UPDATE record.  So,
      PostgreSQL version, which doesn't know about conflict processing, will just
      ignore that.
      
      Reported-by: Andres Freund
      Diagnosed-by: Andres Freund
      Discussion: https://postgr.es/m/20181212224524.scafnlyjindmrbe6%40alap3.anarazel.de
      Author: Alexander Korotkov
      Backpatch-through: 9.6
      c952eae5
    • Tom Lane's avatar
      Base information_schema.sql_identifier domain on name, not varchar. · 7c15cef8
      Tom Lane authored
      The SQL spec says that sql_identifier is a domain over varchar,
      but it also says that that domain is supposed to represent the set
      of valid identifiers for the implementation, in particular applying
      a length limit matching the implementation's identifier length limit.
      We were declaring sql_identifier as just "character varying", thus
      duplicating what the spec says about base type, but entirely failing
      at the rest of it.
      
      Instead, let's declare sql_identifier as a domain over type "name".
      (We can drop the COLLATE "C" added by commit 6b0faf72, since that's
      now implicit in "name".)  With the recent improvements to name's
      comparison support, there's not a lot of functional difference between
      name and varchar.  So although in principle this is a spec deviation,
      it's a pretty minor one.  And correctly enforcing PG's name length limit
      is a good thing; on balance this seems closer to the intent of the spec
      than what we had.
      
      But that's all just language-lawyering.  The *real* reason to do this is
      that it makes sql_identifier columns exposed by information_schema views
      be just direct representations of the underlying "name" catalog columns,
      eliminating a semantic mismatch that was disastrous for performance of
      typical queries on the information_schema.  In combination with the
      recent change to allow dropping no-op CoerceToDomain nodes, this allows
      (for example) queries such as
      
          select ... from information_schema.tables where table_name = 'foo';
      
      to produce an indexscan rather than a seqscan on pg_class.
      
      Discussion: https://postgr.es/m/CAFj8pRBUCX4LZ2rA2BbEkdD6NN59mgx+BLo1gO08Wod4RLtcTg@mail.gmail.com
      7c15cef8
    • Tom Lane's avatar
      Avoid producing over-length specific_name outputs in information_schema. · 5bbee34d
      Tom Lane authored
      information_schema output columns that are declared as being type
      sql_identifier are supposed to conform to the implementation's rules
      for valid identifiers, in particular the identifier length limit.
      Several places potentially violated this limit by concatenating a
      function's name and OID.  (The OID is added to ensure name uniqueness
      within a schema, since the spec doesn't expect function name overloading.)
      
      Simply truncating the concatenation result to fit in "name" won't do,
      since losing part of the OID might wind up giving non-unique results.
      Instead, let's truncate the function name as necessary.
      
      The most practical way to do that is to do it in a C function; the
      information_schema.sql script doesn't have easy access to the value
      of NAMEDATALEN, nor does it have an easy way to truncate on the basis
      of resulting byte-length rather than number of characters.
      
      (There are still a couple of places that cast concatenation results to
      sql_identifier, but as far as I can see they are guaranteed not to produce
      over-length strings, at least with the normal value of NAMEDATALEN.)
      
      Discussion: https://postgr.es/m/23817.1545283477@sss.pgh.pa.us
      5bbee34d
    • Alvaro Herrera's avatar
      Fix lock level used for partition when detaching it · 7b14bcc0
      Alvaro Herrera authored
      For probably bogus reasons, we acquire only AccessShareLock on the
      partition when we try to detach it from its parent partitioned table.
      This can cause ugly things to happen if another transaction is doing
      any sort of DDL to the partition concurrently.
      
      Upgrade that lock to ShareUpdateExclusiveLock, which per discussion
      seems to be the minimum needed.
      
      Reported by Robert Haas.
      
      Discussion: https://postgr.es/m/CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com
      7b14bcc0
    • Tom Lane's avatar
      Doc: fix ancient mistake in search_path documentation. · 42bdf853
      Tom Lane authored
      "$user" in a search_path string is replaced by CURRENT_USER not
      SESSION_USER.  (It actually was SESSION_USER in the initial implementation,
      but we changed it shortly later, and evidently forgot to fix the docs to
      match.)
      
      Noted by antonov@stdpr.ru
      
      Discussion: https://postgr.es/m/159151fb45d490c8d31ea9707e9ba99d@stdpr.ru
      42bdf853
    • Tom Lane's avatar
      Make bitmapset.c use 64-bit bitmap words on 64-bit machines. · 216af5ee
      Tom Lane authored
      Using the full width of the CPU's native word size shouldn't cost
      anything in typical cases.  When working with large bitmapsets,
      this halves the number of operations needed for many common BMS
      operations.  On the right sort of test case, a measurable improvement
      is obtained.
      
      David Rowley
      
      Discussion: https://postgr.es/m/CAKJS1f9EGBd2h-VkXvb=51tf+X46zMX5T8h-KYgXEV_u2zmLUw@mail.gmail.com
      216af5ee
    • Alvaro Herrera's avatar
      DETACH PARTITION: hold locks on indexes until end of transaction · 0c237715
      Alvaro Herrera authored
      When a partition is detached from its parent, we acquire locks on all
      attached indexes to also detach them ... but we release those locks
      immediately.  This is a violation of the policy of keeping locks on user
      objects to the end of the transaction.  Bug introduced in 8b08f7d4.
      
      It's unclear that there are any ill effects possible, but it's clearly
      wrong nonetheless.  It's likely that bad behavior *is* possible, but
      mostly because the relation that the index is for is only locked with
      AccessShareLock, which is an older bug that shall be fixed separately.
      
      While touching that line of code, close the index opened with
      index_open() using index_close() instead of relation_close().
      No difference in practice, but let's be consistent.
      
      Unearthed by Robert Haas.
      
      Discussion: https://postgr.es/m/CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com
      0c237715
    • Michael Paquier's avatar
      Add more tab completion for CREATE TABLE in psql · 4cba9c2a
      Michael Paquier authored
      The following completion patterns are added:
      - CREATE TABLE <name> with '(', OF or PARTITION OF.
      - CREATE TABLE <name> OF with list of composite types.
      - CREATE TABLE name (...) with PARTITION OF, WITH, TABLESPACE, ON
      COMMIT (depending on the presence of a temporary table).
      - CREATE TABLE ON COMMIT with actions (only for temporary tables).
      
      Author: Dagfinn Ilmari Mannsåker
      Discussion: https://postgr.es/m/d8j1s77kdbb.fsf@dalvik.ping.uio.no
      4cba9c2a
    • Greg Stark's avatar
      Fix ADD IF NOT EXISTS used in conjunction with ALTER TABLE ONLY · 1075dfda
      Greg Stark authored
      The flag for IF NOT EXISTS was only being passed down in the normal
      recursing case. It's been this way since originally added in 9.6 in
      commit 2cd40adb so backpatch back to 9.6.
      1075dfda
  7. 19 Dec, 2018 8 commits
    • Tom Lane's avatar
      Add text-vs-name cross-type operators, and unify name_ops with text_ops. · 2ece7c07
      Tom Lane authored
      Now that name comparison has effectively the same behavior as text
      comparison, we might as well merge the name_ops opfamily into text_ops,
      allowing cross-type comparisons to be processed without forcing a
      datatype coercion first.  We need do little more than add cross-type
      operators to make the opfamily complete, and fix one or two places
      in the planner that assumed text_ops was a single-datatype opfamily.
      
      I chose to unify hash name_ops into hash text_ops as well, since the
      types have compatible hashing semantics.  This allows marking the
      new cross-type equality operators as oprcanhash.
      
      (Note: this doesn't remove the name_ops opclasses, so there's no
      breakage of index definitions.  Those opclasses are just reparented
      into the text_ops opfamily.)
      
      Discussion: https://postgr.es/m/15938.1544377821@sss.pgh.pa.us
      2ece7c07
    • Tom Lane's avatar
      Make type "name" collation-aware. · 586b98fd
      Tom Lane authored
      The "name" comparison operators now all support collations, making them
      functionally equivalent to "text" comparisons, except for the different
      physical representation of the datatype.  They do, in fact, mostly share
      the varstr_cmp and varstr_sortsupport infrastructure, which has been
      slightly enlarged to handle the case.
      
      To avoid changes in the default behavior of the datatype, set name's
      typcollation to C_COLLATION_OID not DEFAULT_COLLATION_OID, so that
      by default comparisons to a name value will continue to use strcmp
      semantics.  (This would have been the case for system catalog columns
      anyway, because of commit 6b0faf72, but doing this makes it true for
      user-created name columns as well.  In particular, this avoids
      locale-dependent changes in our regression test results.)
      
      In consequence, tweak a couple of places that made assumptions about
      collatable base types always having typcollation DEFAULT_COLLATION_OID.
      I have not, however, attempted to relax the restriction that user-
      defined collatable types must have that.  Hence, "name" doesn't
      behave quite like a user-defined type; it acts more like a domain
      with COLLATE "C".  (Conceivably, if we ever get rid of the need for
      catalog name columns to be fixed-length, "name" could actually become
      such a domain over text.  But that'd be a pretty massive undertaking,
      and I'm not volunteering.)
      
      Discussion: https://postgr.es/m/15938.1544377821@sss.pgh.pa.us
      586b98fd
    • Alvaro Herrera's avatar
      Remove function names from error messages · 68f6f2b7
      Alvaro Herrera authored
      They are not necessary, and having them there gives useless work for
      translators.
      68f6f2b7
    • Tom Lane's avatar
      Small improvements for allocation logic in ginHeapTupleFastCollect(). · c6e394c1
      Tom Lane authored
      Avoid repetitive calls to repalloc() when the required size of the
      collector array grows more than 2x in one call.  Also ensure that the
      array size is a power of 2 (since palloc will probably consume a power
      of 2 anyway) and doesn't start out very small (which'd likely just lead
      to extra repallocs).
      
      David Rowley, tweaked a bit by me
      
      Discussion: https://postgr.es/m/CAKJS1f8vn-iSBE8PKeVHrnhvyjRNYCxguPFFY08QLYmjWG9hPQ@mail.gmail.com
      c6e394c1
    • Tom Lane's avatar
    • Peter Geoghegan's avatar
      Remove obsolete nbtree duplicate entries comment. · 61a4480a
      Peter Geoghegan authored
      Remove a comment from the Berkeley days claiming that nbtree must
      disambiguate duplicate keys within _bt_moveright().  There is no special
      care taken around duplicates within _bt_moveright(), at least since
      commit 9e85183b removed inscrutable _bt_moveright() code to handle
      pages full of duplicates.
      61a4480a
    • Peter Geoghegan's avatar
      Correct obsolete nbtree recovery comments. · 60f3cc95
      Peter Geoghegan authored
      Commit 40dae7ec, which made the handling of interrupted nbtree page
      splits more robust, removed an nbtree-specific end-of-recovery cleanup
      step.  This meant that it was no longer possible to complete an
      interrupted page split during recovery.  However, a reference to
      recovery as a reason for using a NULL stack while inserting into a
      parent page was missed.  Remove the reference.
      
      Remove a similar obsolete reference to recovery that was introduced much
      more recently, as part of the btree fastpath optimization enhancement
      that made it into Postgres 11 (commit 2b272734, and follow-up commits).
      
      Backpatch: 11-, where the fastpath optimization was introduced.
      60f3cc95
    • Tatsuo Ishii's avatar
      3cab5487
  8. 18 Dec, 2018 7 commits
    • Tom Lane's avatar
      Make collation-aware system catalog columns use "C" collation. · 6b0faf72
      Tom Lane authored
      Up to now we allowed text columns in system catalogs to use collation
      "default", but that isn't really safe because it might mean something
      different in template0 than it means in a database cloned from template0.
      In particular, this could mean that cloned pg_statistic entries for such
      columns weren't entirely valid, possibly leading to bogus planner
      estimates, though (probably) not any outright failures.
      
      In the wake of commit 5e092800, a better solution is available: if we
      label such columns with "C" collation, then their pg_statistic entries
      will also use that collation and hence will be valid independently of
      the database collation.
      
      This also provides a cleaner solution for indexes on such columns than
      the hack added by commit 0b28ea79: the indexes will naturally inherit
      "C" collation and don't have to be forced to use text_pattern_ops.
      
      Also, with the planned improvement of type "name" to be collation-aware,
      this policy will apply cleanly to both text and name columns.
      
      Because of the pg_statistic angle, we should also apply this policy
      to the tables in information_schema.  This patch does that by adjusting
      information_schema's textual domain types to specify "C" collation.
      That has the user-visible effect that order-sensitive comparisons to
      textual information_schema view columns will now use "C" collation
      by default.  The SQL standard says that the collation of those view
      columns is implementation-defined, so I think this is legal per spec.
      At some point this might allow for translation of such comparisons
      into indexable conditions on the underlying "name" columns, although
      additional work will be needed before that can happen.
      
      Discussion: https://postgr.es/m/19346.1544895309@sss.pgh.pa.us
      6b0faf72
    • Tom Lane's avatar
      Update sepgsql regression test results for commit ca410302. · b2d9e177
      Tom Lane authored
      Per buildfarm.
      b2d9e177
    • Tom Lane's avatar
      Fix ancient thinko in mergejoin cost estimation. · d364e881
      Tom Lane authored
      "rescanratio" was computed as 1 + rescanned-tuples / total-inner-tuples,
      which is sensible if it's to be multiplied by total-inner-tuples or a cost
      value corresponding to scanning all the inner tuples.  But in reality it
      was (mostly) multiplied by inner_rows or a related cost, numbers that take
      into account the possibility of stopping short of scanning the whole inner
      relation thanks to a limited key range in the outer relation.  This'd
      still make sense if we could expect that stopping short would result in a
      proportional decrease in the number of tuples that have to be rescanned.
      It does not, however.  The argument that establishes the validity of our
      estimate for that number is independent of whether we scan all of the inner
      relation or stop short, and experimentation also shows that stopping short
      doesn't reduce the number of rescanned tuples.  So the correct calculation
      is 1 + rescanned-tuples / inner_rows, and we should be sure to multiply
      that by inner_rows or a corresponding cost value.
      
      Most of the time this doesn't make much difference, but if we have
      both a high rescan rate (due to lots of duplicate values) and an outer
      key range much smaller than the inner key range, then the error can
      be significant, leading to a large underestimate of the cost associated
      with rescanning.
      
      Per report from Vijaykumar Jain.  This thinko appears to go all the way
      back to the introduction of the rescan estimation logic in commit
      70fba704, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/CAE7uO5hMb_TZYJcZmLAgO6iD68AkEK6qCe7i=vZUkCpoKns+EQ@mail.gmail.com
      d364e881
    • Michael Paquier's avatar
      Include partitioned indexes to system view pg_indexes · f94cec64
      Michael Paquier authored
      pg_tables already includes partitioned tables, so for consistency
      pg_indexes should show partitioned indexes.
      
      Author: Suraj Kharage
      Reviewed-by: Amit Langote, Michael Paquier
      Discussion: https://postgr.es/m/CAF1DzPVrYo4XNTEnc=PqVp6aLJc7LFYpYR4rX=_5pV=wJ2KdZg@mail.gmail.com
      f94cec64
    • Michael Paquier's avatar
      Tweak description comments in tests for partition functions · 3e514c12
      Michael Paquier authored
      The new wording is more generic and fixes one grammar mistake and one
      typo on the way.
      
      Per discussion between Amit Langote and me.
      
      Discussion: https://postgr.es/m/20181217064028.GJ31474@paquier.xyz
      3e514c12
    • Michael Paquier's avatar
      Update project link of pgBadger in documentation · ed6f15eb
      Michael Paquier authored
      The project has moved to a new place.
      
      Reported-by: Peter Neave
      Discussion: https://postgr.es/m/154474118231.5066.16352227860913505754@wrigleys.postgresql.org
      ed6f15eb
    • Michael Paquier's avatar
      Include ALTER INDEX SET STATISTICS in pg_dump · e4fca461
      Michael Paquier authored
      The new grammar pattern of ALTER INDEX SET STATISTICS able to use column
      numbers on top of the existing column names introduced by commit 5b6d13ee
      forgot to add support for the feature in pg_dump, so defining statistics
      on index columns was missing from the dumps, potentially causing silent
      planning problems with a subsequent restore.
      
      pg_dump ought to not use column names in what it generates as these are
      automatically generated by the server and could conflict with real
      relation attributes with matching patterns.  "expr" and "exprN", N
      incremented automatically after the creation of the first one, are used
      as default attribute names for index expressions, and that could easily
      match what is defined in other relations, causing the dumps to fail if
      some of those attributes are renamed at some point.  So to avoid any
      problems, the new grammar with column numbers gets used.
      
      Reported-by: Ronan Dunklau
      Author: Michael Paquier
      Reviewed-by: Tom Lane, Adrien Nayrat, Amul Sul
      Discussion: https://postgr.es/m/CAARsnT3UQ4V=yDNW468w8RqHfYiY9mpn2r_c5UkBJ97NAApUEw@mail.gmail.com
      Backpatch-through: 11, where the new syntax has been introduced.
      e4fca461
  9. 17 Dec, 2018 7 commits
  10. 16 Dec, 2018 1 commit
    • Tom Lane's avatar
      Make error handling in parallel pg_upgrade less bogus. · 16fda4b8
      Tom Lane authored
      reap_child() basically ignored the possibility of either an error in
      waitpid() itself or a child process failure on signal.  We don't really
      need to do more than report and crash hard, but proceeding as though
      nothing is wrong is definitely Not Acceptable.  The error report for
      nonzero child exit status was pretty off-point, as well.
      
      Noted while fooling around with child-process failure detection
      logic elsewhere.  It's been like this a long time, so back-patch to
      all supported branches.
      16fda4b8