1. 15 Apr, 2018 1 commit
  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 11 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
    • Peter Eisentraut's avatar
      doc: Fix typos in pgbench documentation · 036ca6f7
      Peter Eisentraut authored
      Author: Fabien COELHO <coelho@cri.ensmp.fr>
      Reviewed-by: default avatarEdmund Horner <ejrh00@gmail.com>
      036ca6f7
  6. 10 Apr, 2018 7 commits
    • Andrew Dunstan's avatar
      minor comment fixes in nbtinsert.c · 8716b264
      Andrew Dunstan authored
      8716b264
    • Tom Lane's avatar
      Fix incorrect close() call in dsm_impl_mmap(). · 231bcd08
      Tom Lane authored
      One improbable error-exit path in this function used close() where
      it should have used CloseTransientFile().  This is unlikely to be
      hit in the field, and I think the consequences wouldn't be awful
      (just an elog(LOG) bleat later).  But a bug is a bug, so back-patch
      to 9.4 where this code came in.
      
      Pan Bian
      
      Discussion: https://postgr.es/m/152056616579.4966.583293218357089052@wrigleys.postgresql.org
      231bcd08
    • Andrew Dunstan's avatar
      Adjustments to the btree fastpath optimization. · 074251db
      Andrew Dunstan authored
      This optimization was introduced in commit 2b272734. The changes include
      some additional comments and documentation, and also these more
      substantive changes:
      . ensure the optimization is only applied on the leaf node of a tree
      whose root is on level 2 or more. It's of little value on small trees.
      . Delay calling RelationSetTargetBlock() until after the critical
      section of _bt_insertonpg
      . ensure the optimization is also applied to unlogged tables.
      
      Pavan Deolasee and Peter Geoghegan with some very light editing from me.
      
      Discussion: https://postgr.es/m/CABOikdO8jhRarNC60nZLktZYhxt+TK8z_V97+Ny499YQdyAfug@mail.gmail.com
      074251db
    • Tom Lane's avatar
      Put back parallel-safety guards in plpython and src/test/regress/. · 31f1f0bb
      Tom Lane authored
      I'd hoped that commit 3b8f6e75 was sufficient to ensure parallel safety
      even when a build started in a subdirectory requires rebuilding of
      generated headers.  This isn't so, because making submake-generated-headers
      a prerequisite of "all" isn't enough to ensure it's completed before
      starting on "all"'s other prerequisites.  The explicit dependencies we put
      on the recursive make targets ensure safe ordering before we recurse into
      child directories, but they don't protect targets to be made in the current
      directory.  Hence, put back some ordering dependencies in directories that
      we've traditionally expected to be starting points for "standalone" builds,
      to wit src/pl/plpython and src/test/regress.  (The former needs this in
      order to minimize the work involved in building for both python 2 and
      python 3; the latter to support packagings that make the regression tests
      available for out-of-build-tree execution.)  Adjust some other dependencies
      so that these two cases work correctly even at high -j settings.
      
      I'm not terribly happy with this partial solution, but I don't see a
      way to do better without massive makefile restructuring, which we surely
      aren't doing at this point in the development cycle.  In any case, it's
      little if any worse than what we had in prior releases.
      
      Discussion: https://postgr.es/m/1523353963.8169.26.camel@gunduz.org
      31f1f0bb
    • Alvaro Herrera's avatar
      Fix IndexOnlyScan counter for heap fetches in parallel mode · 15a8f8ca
      Alvaro Herrera authored
      The HeapFetches counter was using a simple value in IndexOnlyScanState,
      which fails to propagate values from parallel workers; so the counts are
      wrong when IndexOnlyScan runs in parallel.  Move it to Instrumentation,
      like all the other counters.
      
      While at it, change INSERT ON CONFLICT conflicting tuple counter to use
      the new ntuples2 instead of nfiltered2, which is a blatant misuse.
      
      Discussion: https://postgr.es/m/20180409215851.idwc75ct2bzi6tea@alvherre.pgsql
      15a8f8ca
    • Tom Lane's avatar
      Fix pgxs.mk to not try to build generated headers in external builds. · 1a40485a
      Tom Lane authored
      Per Julien Rouhaud and the buildfarm.  This is not quite Julien's
      patch: there's no need to lobotomize this build rule when building
      contrib modules in-tree, so set NO_GENERATED_HEADERS only if PGXS.
      
      In passing, also set NO_TEMP_INSTALL in external builds.  This doesn't
      seem to be fixing any live bug, because "make check" in an external
      build just produces the expected error message without first trying to
      make a temp install ... but it's far from obvious why it doesn't, so
      this change seems like good future-proofing.
      
      Julien Rouhaud and Tom Lane
      
      Discussion: https://postgr.es/m/CAOBaU_YH=g68opbbMk8is3jNwhoXGa8ckRSre1nx0Obe1C7i-Q@mail.gmail.com
      1a40485a
    • Heikki Linnakangas's avatar
      Fix comment on B-tree insertion fastpath condition. · 29d7ebf5
      Heikki Linnakangas authored
      The comment earlier in the function correctly states "and the insertion
      key is strictly greater than the first key in this page". That is what
      we check here, not "greater than or equal".
      29d7ebf5