1. 15 Apr, 2018 9 commits
    • Tom Lane's avatar
      Fix potentially-unportable code in contrib/adminpack. · 3a2d6365
      Tom Lane authored
      Spelling access(2)'s second argument as "2" is just horrid.
      POSIX makes no promises as to the numeric values of W_OK and related
      macros.  Even if it accidentally works as intended on every supported
      platform, it's still unreadable and inconsistent with adjacent code.
      
      In passing, don't spell "NULL" as "0" either.  Yes, that's legal C;
      no, it's not project style.
      
      Back-patch, just in case the unportability is real and not theoretical.
      (Most likely, even if a platform had different bit assignments for
      access()'s modes, there'd not be an observable behavior difference
      here; but I'm being paranoid today.)
      3a2d6365
    • Tom Lane's avatar
      Clean up callers of JsonbIteratorNext(). · f8a187bd
      Tom Lane authored
      Coverity complained about the lack of a check on the return value in
      parse_jsonb_index_flags' last call of JsonbIteratorNext.  Seems like
      a reasonable gripe to me, especially since the code is depending on
      that being WJB_DONE to not leak memory, so add a check.
      
      In passing, improve a couple other places where the result was being
      ignored, either by adding an assert or at least a cast to void.
      
      Also, don't spell "WJB_DONE" as "0".  That's horrid coding style,
      and it wasn't consistent either.
      f8a187bd
    • Magnus Hagander's avatar
      Don't attempt to verify checksums on new pages · 33cedf14
      Magnus Hagander authored
      Teach both base backups and pg_verify_checksums that if a page is new,
      it does not have a checksum yet, so it shouldn't be verified.
      
      Noted by Tomas Vondra, review by David Steele.
      33cedf14
    • Magnus Hagander's avatar
      Fix build of pg_verify_checksum docs · 90372729
      Magnus Hagander authored
      They were accidentally excluded when reverting the backend online
      checksum functionality, and since they weren't built the incorrect
      reference to a removed section also did not trigger a problem.
      
      Author: Christoph Berg
      90372729
    • Magnus Hagander's avatar
      Clarify pg_verify_checksum documentation · 64538792
      Magnus Hagander authored
      Make it clear that a cluster has to be shut down cleanly before
      pg_verify_checksum can be run against it.
      
      Author: Michael Paquier
      Review: Daniel Gustafsson
      64538792
    • Magnus Hagander's avatar
      Remove -f option from pg_verify_checksums · 44e2df46
      Magnus Hagander authored
      This option makes no sense when the cluster checksum state cannot be
      changed, and should have been removed in the revert.
      
      Author: Daniel Gustafsson
      Review: Michael Paquier
      44e2df46
    • Tom Lane's avatar
      Simplify view-expansion code in rewriteHandler.c. · 49ac4039
      Tom Lane authored
      In the wake of commit 50c6bb02, it's not necessary for ApplyRetrieveRule
      to have a forUpdatePushedDown parameter.  By the time control gets here for
      any given view-referencing RTE, we should already have pushed down the
      effects of any FOR UPDATE/SHARE clauses affecting the view from outer query
      levels.  Hence if we don't find a RowMarkClause at the current query level,
      that's sufficient proof that there is no outer one either.  This in turn
      means we need no forUpdatePushedDown parameter for fireRIRrules.
      
      I wonder whether we oughtn't also revert commit cba2d271, since it now
      seems likely that that was band-aiding around the bad effects of doing
      FOR UPDATE pushdown and view expansion in the wrong order.  However,
      in the absence of evidence that the current coding of markQueryForLocking
      is actually buggy (i.e. missing RTEs it ought to mark), it seems best to
      leave it alone.
      
      Discussion: https://postgr.es/m/24db7b8f-3de5-e25f-7ab9-d8848351d42c@gmail.com
      49ac4039
    • Alvaro Herrera's avatar
      List src/include/partitioning in src/include/Makefile · 4d64abc2
      Alvaro Herrera authored
      This omission prevented partitioning header files from being installed.
      
      Per buildfarm member crake.
      4d64abc2
    • Alvaro Herrera's avatar
      Reorganize partitioning code · da6f3e45
      Alvaro Herrera authored
      There's been a massive addition of partitioning code in PostgreSQL 11,
      with little oversight on its placement, resulting in a
      catalog/partition.c with poorly defined boundaries and responsibilities.
      This commit tries to set a couple of distinct modules to separate things
      a little bit.  There are no code changes here, only code movement.
      
      There are three new files:
        src/backend/utils/cache/partcache.c
        src/include/partitioning/partdefs.h
        src/include/utils/partcache.h
      
      The previous arrangement of #including catalog/partition.h almost
      everywhere is no more.
      
      Authors: Amit Langote and Álvaro Herrera
      Discussion: https://postgr.es/m/98e8d509-790a-128c-be7f-e48a5b2d8d97@lab.ntt.co.jp
      	https://postgr.es/m/11aa0c50-316b-18bb-722d-c23814f39059@lab.ntt.co.jp
      	https://postgr.es/m/143ed9a4-6038-76d4-9a55-502035815e68@lab.ntt.co.jp
      	https://postgr.es/m/20180413193503.nynq7bnmgh6vs5vm@alvherre.pgsql
      da6f3e45
  2. 14 Apr, 2018 5 commits
    • Tom Lane's avatar
      Improve regression test coverage of expand_tuple(). · b39fd897
      Tom Lane authored
      I was dissatisfied with the code coverage report for expand_tuple() in the
      wake of commit 7c44c46d: while better than no coverage at all, it was
      still not exercising the core function of inserting out-of-line default
      values, nor was the HeapTuple-output path covered.  So far as I can find,
      the only code path that reaches the latter at present is EvalPlanQual
      fetches for non-locked tables.  Hence, extend eval-plan-qual.spec to
      test cases where out-of-line defaults must be inserted into a tuple
      fetched from a non-locked table.
      
      Discussion: https://postgr.es/m/87woxi24uw.fsf@ansel.ydns.eu
      b39fd897
    • Tom Lane's avatar
      Fix enforcement of SELECT FOR UPDATE permissions with nested views. · 50c6bb02
      Tom Lane authored
      SELECT FOR UPDATE on a view should require UPDATE (as well as SELECT)
      permissions on the view, and then the view's owner needs those same
      permissions against the relations it references, and so on all the way
      down to base tables.  But ApplyRetrieveRule did things in the wrong order,
      resulting in failure to mark intermediate view levels as needing UPDATE
      permission.  Thus for example, if user A creates a table T and an updatable
      view V1 on T, then grants only SELECT permissions on V1 to user B, B could
      create a second view V2 on V1 and then would be allowed to perform SELECT
      FOR UPDATE via V2 (since V1 wouldn't be checked for UPDATE permissions).
      
      To fix, just switch the order of expanding sub-views and marking referenced
      objects as needing UPDATE permission.  I think additional simplifications
      are now possible, but that's distinct from the bug fix proper.
      
      This is certainly a security issue, but the consequences are pretty minor
      (just the ability to lock rows that shouldn't be lockable).  Against that
      we have a small risk of breaking applications that are working as-desired,
      since nested views have behaved this way since such cases worked at all.
      On balance I'm inclined not to back-patch.
      
      Per report from Alexander Lakhin.
      
      Discussion: https://postgr.es/m/24db7b8f-3de5-e25f-7ab9-d8848351d42c@gmail.com
      50c6bb02
    • Tom Lane's avatar
      Add commentary explaining why MaxIndexTuplesPerPage calculation is safe. · 2a67d644
      Tom Lane authored
      MaxIndexTuplesPerPage ignores the fact that btree indexes sometimes
      store tuples with no data payload.  But it also ignores the possibility
      of "special space" on index pages, which offsets that, so that the
      result isn't an underestimate.  This all seems worth documenting, though.
      
      In passing, remove #define MinIndexTupleSize, which was added by
      commit 2c03216d but not used in that commit nor later ones.
      
      Comment text by me; issue noticed by Peter Geoghegan.
      
      Discussion: https://postgr.es/m/CAH2-WzkQmb54Kbx-YHXstRKXcNc+_87jwV3DRb54xcybLR7Oig@mail.gmail.com
      2a67d644
    • Peter Eisentraut's avatar
      Improve code comments · e013288a
      Peter Eisentraut authored
      As of 0c2c81b4, the replication
      parameter in libpq is no longer "deliberately undocumented".
      e013288a
    • Peter Eisentraut's avatar
      Support named and default arguments in CALL · a8677e3f
      Peter Eisentraut authored
      We need to call expand_function_arguments() to expand named and default
      arguments.
      
      In PL/pgSQL, we also need to deal with named and default INOUT arguments
      when receiving the output values into variables.
      
      Author: Pavel Stehule <pavel.stehule@gmail.com>
      a8677e3f
  3. 13 Apr, 2018 5 commits
  4. 12 Apr, 2018 11 commits
  5. 11 Apr, 2018 10 commits
    • Tom Lane's avatar
      Ignore nextOid when replaying an ONLINE checkpoint. · d1e90792
      Tom Lane authored
      The nextOid value is from the start of the checkpoint and may well be stale
      compared to values from more recent XLOG_NEXTOID records.  Previously, we
      adopted it anyway, allowing the OID counter to go backwards during a crash.
      While this should be harmless, it contributed to the severity of the bug
      fixed in commit 0408e1ed, by allowing duplicate TOAST OIDs to be assigned
      immediately following a crash.  Without this error, that issue would only
      have arisen when TOAST objects just younger than a multiple of 2^32 OIDs
      were deleted and then not vacuumed in time to avoid a conflict.
      
      Pavan Deolasee
      
      Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com
      d1e90792
    • Tom Lane's avatar
      Do not select new object OIDs that match recently-dead entries. · 0408e1ed
      Tom Lane authored
      When selecting a new OID, we take care to avoid picking one that's already
      in use in the target table, so as not to create duplicates after the OID
      counter has wrapped around.  However, up to now we used SnapshotDirty when
      scanning for pre-existing entries.  That ignores committed-dead rows, so
      that we could select an OID matching a deleted-but-not-yet-vacuumed row.
      While that mostly worked, it has two problems:
      
      * If recently deleted, the dead row might still be visible to MVCC
      snapshots, creating a risk for duplicate OIDs when examining the catalogs
      within our own transaction.  Such duplication couldn't be visible outside
      the object-creating transaction, though, and we've heard few if any field
      reports corresponding to such a symptom.
      
      * When selecting a TOAST OID, deleted toast rows definitely *are* visible
      to SnapshotToast, and will remain so until vacuumed away.  This leads to
      a conflict that will manifest in errors like "unexpected chunk number 0
      (expected 1) for toast value nnnnn".  We've been seeing reports of such
      errors from the field for years, but the cause was unclear before.
      
      The fix is simple: just use SnapshotAny to search for conflicting rows.
      This results in a slightly longer window before object OIDs can be
      recycled, but that seems unlikely to create any large problems.
      
      Pavan Deolasee
      
      Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com
      0408e1ed
    • Heikki Linnakangas's avatar
      Allocate enough shared string memory for stats of auxiliary processes. · 811969b2
      Heikki Linnakangas authored
      This fixes a bug whereby the st_appname, st_clienthostname, and
      st_activity_raw fields for auxiliary processes point beyond the end of
      their respective shared memory segments. As a result, the application_name
      of a backend might show up as the client hostname of an auxiliary process.
      
      Backpatch to v10, where this bug was introduced, when the auxiliary
      processes were added to the array.
      
      Author: Edmund Horner
      Reviewed-by: Michael Paquier
      Discussion: https://www.postgresql.org/message-id/CAMyN-kA7aOJzBmrYFdXcc7Z0NmW%2B5jBaf_m%3D_-77uRNyKC9r%3DA%40mail.gmail.com
      811969b2
    • Heikki Linnakangas's avatar
      Make local copy of client hostnames in backend status array. · a820b4c3
      Heikki Linnakangas authored
      The other strings, application_name and query string, were snapshotted to
      local memory in pgstat_read_current_status(), but we forgot to do that for
      client hostnames. As a result, the client hostname would appear to change in
      the local copy, if the client disconnected.
      
      Backpatch to all supported versions.
      
      Author: Edmund Horner
      Reviewed-by: Michael Paquier
      Discussion: https://www.postgresql.org/message-id/CAMyN-kA7aOJzBmrYFdXcc7Z0NmW%2B5jBaf_m%3D_-77uRNyKC9r%3DA%40mail.gmail.com
      a820b4c3
    • Alvaro Herrera's avatar
      Fix ALTER TABLE .. ATTACH PARTITION ... DEFAULT · 72cf7f31
      Alvaro Herrera authored
      If the table being attached contained values that contradict the default
      partition's partition constraint, it would fail to complain, because
      CommandCounterIncrement changes in 4dba331c coupled with some bogus
      coding in the existing ValidatePartitionConstraints prevented the
      partition constraint from being validated after all -- or rather, it
      caused to constraint to become an empty one, always succeeding.
      
      Fix by not re-reading the OID of the default partition in
      ATExecAttachPartition.  To forestall similar problems, revise the
      existing code:
      * rename routine from ValidatePartitionConstraints() to
        QueuePartitionConstraintValidation, to better represent what it
        actually does.
      * add an Assert() to make sure that when queueing a constraint for a
        partition we're not overwriting a constraint previously queued.
      * add an Assert() that we don't try to invoke the special-purpose
        validation of the default partition when attaching the default
        partition itself.
      
      While at it, change some loops to obtain partition OIDs from
      partdesc->oids rather than find_all_inheritors; reduce the lock level
      of partitions being scanned from AccessExclusiveLock to ShareLock;
      rewrite QueuePartitionConstraintValidation in a recursive fashion rather
      than repetitive.
      
      Author: Álvaro Herrera.  Tests written by Amit Langote
      Reported-by: Rushabh Lathia
      Diagnosed-by: Kyotaro HORIGUCHI, who also provided the initial fix.
      Reviewed-by: Kyotaro HORIGUCHI, Amit Langote, Jeevan Ladhe
      Discussion: https://postgr.es/m/CAGPqQf0W+v-Ci_qNV_5R3A=Z9LsK4+jO7LzgddRncpp_rrnJqQ@mail.gmail.com
      72cf7f31
    • Tom Lane's avatar
      Invoke submake-generated-headers during "make check", too. · cee83ef4
      Tom Lane authored
      The MAKELEVEL hack to prevent submake-generated-headers from doing
      anything in child make runs means that we have to explicitly invoke
      it at top level for "make check", too, in case somebody proceeds
      directly to that without an explicit "make all".  (I think this
      usage had parallel-make hazards even before the addition of more
      generated headers; but it was totally broken as of 3b8f6e75.)
      
      Out of paranoia, force the submake-libpq target to depend on
      submake-generated-headers, too.  This seems to not be absolutely
      necessary today, but it's not really saving us anything to omit
      the ordering dependency, and it'll likely break someday without it.
      
      Discussion: https://postgr.es/m/20180411103930.GB31461@momjian.us
      cee83ef4
    • Teodor Sigaev's avatar
      Temporary revert 5c6110c6 · 92899992
      Teodor Sigaev authored
      It discovers one more bug in CompareIndexInfo(), should be fixed first.
      92899992
    • Peter Eisentraut's avatar
      Fix clashing function names between jsonb_plperl and jsonb_plperlu · 651cb909
      Peter Eisentraut authored
      This prevented them from being installed at the same time.
      
      Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
      651cb909
    • Teodor Sigaev's avatar
      Fix interference between cavering indexes and partitioned tables · 5c6110c6
      Teodor Sigaev authored
      The bug is caused due to the original IndexStmt that DefineIndex receives
      being overwritten when processing the INCLUDE columns. Use separate list of
      index params to propagate to child tables. Add tests covering this case.
      
      Amit Langote and Alexander Korotkov.
      5c6110c6
    • Peter Eisentraut's avatar
      doc: Add more information about logical replication privileges · f1f537cb
      Peter Eisentraut authored
      In particular, the requirement to have SELECT privilege for the initial
      table copy was previously not documented.
      
      Author: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
      f1f537cb