1. 15 Nov, 2016 7 commits
    • Tom Lane's avatar
      Check that result tupdesc has exactly 1 column in return_next scalar case. · 4ecd1974
      Tom Lane authored
      This should always be true, but since we're relying on a tuple descriptor
      passed from outside pltcl itself, let's check.  Per a gripe from Coverity.
      4ecd1974
    • Robert Haas's avatar
      Reserve zero as an invalid DSM handle. · b40b4dd9
      Robert Haas authored
      Previously, the handle for the control segment could not be zero, but
      some other DSM segment could potentially have a handle value of zero.
      However, that means that if someone wanted to store a dsm_handle that
      might or might not be valid, they would need a separate boolean to
      keep track of whether the associated value is legal.  That's annoying,
      so change things so that no DSM segment can ever have a handle of 0 -
      or as we call it here, DSM_HANDLE_INVALID.
      
      Thomas Munro.  This was submitted as part of a much larger patch to
      add an malloc-like allocator for dynamic shared memory, but this part
      seems like a good idea independently of the rest of the patch.
      b40b4dd9
    • Tom Lane's avatar
      Allow DOS-style line endings in ~/.pgpass files. · 0a748193
      Tom Lane authored
      On Windows, libc will mask \r\n line endings for us, since we read the
      password file in text mode.  But that doesn't happen on Unix.  People
      who share password files across both systems might have \r\n line endings
      in a file they use on Unix, so as a convenience, ignore trailing \r.
      Per gripe from Josh Berkus.
      
      In passing, put the existing check for empty line somewhere where it's
      actually useful, ie after stripping the newline not before.
      
      Vik Fearing, adjusted a bit by me
      
      Discussion: <0de37763-5843-b2cc-855e-5d0e5df25807@agliodbs.com>
      0a748193
    • Tom Lane's avatar
      Account for catalog snapshot in PGXACT->xmin updates. · ffaa44cb
      Tom Lane authored
      The CatalogSnapshot was not plugged into SnapshotResetXmin()'s accounting
      for whether MyPgXact->xmin could be cleared or advanced.  In normal
      transactions this was masked by the fact that the transaction snapshot
      would be older, but during backend startup and certain utility commands
      it was possible to re-use the CatalogSnapshot after MyPgXact->xmin had
      been cleared, meaning that recently-deleted rows could be pruned even
      though this snapshot could still see them, causing unexpected catalog
      lookup failures.  This effect appears to be the explanation for a recent
      failure on buildfarm member piculet.
      
      To fix, add the CatalogSnapshot to the RegisteredSnapshots heap whenever
      it is valid.
      
      In the previous logic, it was possible for the CatalogSnapshot to remain
      valid across waits for client input, but with this change that would mean
      it delays advance of global xmin in cases where it did not before.  To
      avoid possibly causing new table-bloat problems with clients that sit idle
      for long intervals, add code to invalidate the CatalogSnapshot before
      waiting for client input.  (When the backend is busy, it's unlikely that
      the CatalogSnapshot would be the oldest snap for very long, so we don't
      worry about forcing early invalidation of it otherwise.)
      
      In passing, remove the CatalogSnapshotStale flag in favor of using
      "CatalogSnapshot != NULL" to represent validity, as we do for the other
      special snapshots in snapmgr.c.  And improve some obsolete comments.
      
      No regression test because I don't know a deterministic way to cause this
      failure.  But the stress test shown in the original discussion provokes
      "cache lookup failed for relation 1255" within a few dozen seconds for me.
      
      Back-patch to 9.4 where MVCC catalog scans were introduced.  (Note: it's
      quite easy to produce similar failures with the same test case in branches
      before 9.4.  But MVCC catalog scans were supposed to fix that.)
      
      Discussion: <16447.1478818294@sss.pgh.pa.us>
      ffaa44cb
    • Robert Haas's avatar
      Limit the number of number of tapes used for a sort to 501. · fc19c180
      Robert Haas authored
      Gigantic numbers of tapes don't work out well.
      
      Original patch by Peter Geoghegan; comments entirely rewritten by me.
      fc19c180
    • Robert Haas's avatar
      Fix broken statement in UCS_to_most.pl. · 00c6d807
      Robert Haas authored
      This has been wrong for a very long time, and it's puzzling to me how
      it ever worked for anyone.
      
      Kyotaro Horiguchi
      00c6d807
    • Robert Haas's avatar
      pgbench: Increase maximum size of log filename from 64 to MAXPGPATH. · 56eba9b8
      Robert Haas authored
      Commit 41124a91 allowed the
      transaction log file prefix to be changed but left in place the
      existing 64-character limit on the total length of a log file name.
      It's possible that could be inconvenient for somebody, so increase the
      limit to MAXPGPATH, which ought to be enough for anybody.
      
      Per a suggestion from Tom Lane.
      56eba9b8
  2. 14 Nov, 2016 6 commits
  3. 13 Nov, 2016 2 commits
  4. 12 Nov, 2016 1 commit
    • Andres Freund's avatar
      Add minimal set of regression tests for pg_stat_statements. · 9be244db
      Andres Freund authored
      While the set of covered functionality is fairly small, the added tests
      still are useful to get some basic buildfarm testing of
      pg_stat_statements itself, but also to exercise the lwlock tranch code
      on the buildfarm.
      
      Author: Amit Kapila, slightly editorialized by me
      Reviewed-By: Ashutosh Sharma, Andres Freund
      Discussion: <CAA4eK1JOjkdXYtHxh=2aDK4VgDtN-LNGKY_YqX0N=YEvuzQVWg@mail.gmail.com>
      9be244db
  5. 11 Nov, 2016 1 commit
  6. 10 Nov, 2016 5 commits
    • Tom Lane's avatar
      Cleanup of rewriter and planner handling of Query.hasRowSecurity flag. · 24aef338
      Tom Lane authored
      Be sure to pull up the subquery's hasRowSecurity flag when flattening a
      subquery in pull_up_simple_subquery().  This isn't a bug today because
      we don't look at the hasRowSecurity flag during planning, but it could
      easily be a bug tomorrow.
      
      Likewise, make rewriteRuleAction() pull up the hasRowSecurity flag when
      absorbing RTEs from a rule action.  This isn't a bug either, for the
      opposite reason: the flag should never be set yet.  But again, it seems
      like good future proofing.
      
      Add a comment explaining why rewriteTargetView() should *not* set
      hasRowSecurity when adding stuff to securityQuals.
      
      Improve some nearby comments about securityQuals processing, and document
      that field more completely in parsenodes.h.
      
      Patch by me, analysis by Dean Rasheed.
      
      Discussion: <CAEZATCXZ8tb2DV6f=bkhsMV6u_gRcZ0CZBw2J-qU84RxSukZog@mail.gmail.com>
      24aef338
    • Tom Lane's avatar
      Re-allow user_catalog_table option for materialized views. · 530f8065
      Tom Lane authored
      The reloptions stuff allows this option to be set on a matview.
      While it's questionable whether that is useful or was really intended,
      it does work, and we shouldn't change that in minor releases.  Commit
      e3e66d8a disabled the option since I didn't realize that it was
      possible for it to be set on a matview.  Tweak the test to re-allow it.
      
      Discussion: <19749.1478711862@sss.pgh.pa.us>
      530f8065
    • Tom Lane's avatar
      Support "COPY view FROM" for views with INSTEAD OF INSERT triggers. · 279c439c
      Tom Lane authored
      We just pass the data to the INSTEAD trigger.
      
      Haribabu Kommi, reviewed by Dilip Kumar
      
      Patch: <CAJrrPGcSQkrNkO+4PhLm4B8UQQQmU9YVUuqmtgM=pmzMfxWaWQ@mail.gmail.com>
      279c439c
    • Tom Lane's avatar
      Fix partial aggregation for the case of a degenerate GROUP BY clause. · e1b449be
      Tom Lane authored
      The plan generated for sorted partial aggregation with "GROUP BY constant"
      included a Sort node with no sort keys, which the executor does not like.
      
      Per report from Steve Randall.  I'd add a regression test case if I could
      think of a compact one, but it doesn't seem worth expending lots of cycles
      on.
      
      Report: <CABVd52UAdGXpg_rCk46egpNKYdXOzCjuJ1zG26E2xBe_8bj+Fg@mail.gmail.com>
      e1b449be
    • Tom Lane's avatar
      Doc: improve link. · 0b1b5033
      Tom Lane authored
      Discussion: <5019.1478790246@sss.pgh.pa.us>
      0b1b5033
  7. 09 Nov, 2016 2 commits
  8. 08 Nov, 2016 11 commits
    • Tom Lane's avatar
      Simplify code by getting rid of SPI_push, SPI_pop, SPI_restore_connection. · 1833f1a1
      Tom Lane authored
      The idea behind SPI_push was to allow transitioning back into an
      "unconnected" state when a SPI-using procedure calls unrelated code that
      might or might not invoke SPI.  That sounds good, but in practice the only
      thing it does for us is to catch cases where a called SPI-using function
      forgets to call SPI_connect --- which is a highly improbable failure mode,
      since it would be exposed immediately by direct testing of said function.
      As against that, we've had multiple bugs induced by forgetting to call
      SPI_push/SPI_pop around code that might invoke SPI-using functions; these
      are much harder to catch and indeed have gone undetected for years in some
      cases.  And we've had to band-aid around some problems of this ilk by
      introducing conditional push/pop pairs in some places, which really kind
      of defeats the purpose altogether; if we can't draw bright lines between
      connected and unconnected code, what's the point?
      
      Hence, get rid of SPI_push[_conditional], SPI_pop[_conditional], and the
      underlying state variable _SPI_curid.  It turns out SPI_restore_connection
      can go away too, which is a nice side benefit since it was never more than
      a kluge.  Provide no-op macros for the deleted functions so as to avoid an
      API break for external modules.
      
      A side effect of this removal is that SPI_palloc and allied functions no
      longer permit being called when unconnected; they'll throw an error
      instead.  The apparent usefulness of the previous behavior was a mirage
      as well, because it was depended on by only a few places (which I fixed in
      preceding commits), and it posed a risk of allocations being unexpectedly
      long-lived if someone forgot a SPI_push call.
      
      Discussion: <20808.1478481403@sss.pgh.pa.us>
      1833f1a1
    • Robert Haas's avatar
      psql: Tab completion for renaming enum values. · 577f0bdd
      Robert Haas authored
      For ALTER TYPE .. RENAME, add "VALUE" to the list of possible
      completions.  Complete ALTER TYPE .. RENAME VALUE with possible
      enum values.  After that, complete with "TO".
      
      Dagfinn Ilmari Mannsåker, reviewed by Artur Zakirov.
      577f0bdd
    • Tom Lane's avatar
      Replace uses of SPI_modifytuple that intend to allocate in current context. · 9257f078
      Tom Lane authored
      Invent a new function heap_modify_tuple_by_cols() that is functionally
      equivalent to SPI_modifytuple except that it always allocates its result
      by simple palloc.  I chose however to make the API details a bit more
      like heap_modify_tuple: pass a tupdesc rather than a Relation, and use
      bool convention for the isnull array.
      
      Use this function in place of SPI_modifytuple at all call sites where the
      intended behavior is to allocate in current context.  (There actually are
      only two call sites left that depend on the old behavior, which makes me
      wonder if we should just drop this function rather than keep it.)
      
      This new function is easier to use than heap_modify_tuple() for purposes
      of replacing a single column (or, really, any fixed number of columns).
      There are a number of places where it would simplify the code to change
      over, but I resisted that temptation for the moment ... everywhere except
      in plpgsql's exec_assign_value(); changing that might offer some small
      performance benefit, so I did it.
      
      This is on the way to removing SPI_push/SPI_pop, but it seems like
      good code cleanup in its own right.
      
      Discussion: <9633.1478552022@sss.pgh.pa.us>
      9257f078
    • Robert Haas's avatar
      Fix typo. · dce429b1
      Robert Haas authored
      Michael Paquier
      dce429b1
    • Tom Lane's avatar
      Make SPI_fnumber() reject dropped columns. · 6d30fb1f
      Tom Lane authored
      There's basically no scenario where it's sensible for this to match
      dropped columns, so put a test for dropped-ness into SPI_fnumber()
      itself, and excise the test from the small number of callers that
      were paying attention to the case.  (Most weren't :-(.)
      
      In passing, normalize tests at call sites: always reject attnum <= 0
      if we're disallowing system columns.  Previously there was a mixture
      of "< 0" and "<= 0" tests.  This makes no practical difference since
      SPI_fnumber() never returns 0, but I'm feeling pedantic today.
      
      Also, in the places that are actually live user-facing code and not
      legacy cruft, distinguish "column not found" from "can't handle
      system column".
      
      Per discussion with Jim Nasby; thi supersedes his original patch
      that just changed the behavior at one call site.
      
      Discussion: <b2de8258-c4c0-1cb8-7b97-e8538e5c975c@BlueTreble.com>
      6d30fb1f
    • Magnus Hagander's avatar
      Fix typo · 36ac6d0e
      Magnus Hagander authored
      36ac6d0e
    • Robert Haas's avatar
      Fix mistake in XLOG_SEG_SIZE test. · 60379f66
      Robert Haas authored
      The intent of the test is to check whether XLOG_SEG_SIZE is in a
      particular range, but actually in one case it compares XLOG_BLCKSZ
      by mistake.  Repair.
      
      Commit 88e98230 introduced this
      faulty test.
      
      Kuntal Ghosh, reviewed by Michael Paquier.
      60379f66
    • Tom Lane's avatar
      Use heap_modify_tuple not SPI_modifytuple in pl/python triggers. · de4026c6
      Tom Lane authored
      The code here would need some change anyway given planned change in
      SPI_modifytuple semantics, since this executes after we've exited the
      SPI environment.  But really it's better to just use heap_modify_tuple.
      
      While at it, normalize use of SPI_fnumber: make error messages distinguish
      no-such-column from can't-set-system-column, and remove test for deleted
      column which is going to migrate into SPI_fnumber.  The lack of a check
      for system column names is actually a pre-existing bug here, and might
      even qualify as a security bug except that we don't have any trusted
      version of plpython.
      de4026c6
    • Tom Lane's avatar
      Use heap_modify_tuple not SPI_modifytuple in pl/perl triggers. · 0d444608
      Tom Lane authored
      The code here would need some change anyway given planned change in
      SPI_modifytuple semantics, since this executes after we've exited the
      SPI environment.  But really it's better to just use heap_modify_tuple.
      The code's actually shorter this way, and this avoids depending on some
      rather indirect reasoning about why the temporary arrays can't be overrun.
      (I think the old code is safe, as long as Perl hashes can't contain
      duplicate keys; but with this way we don't need that assumption, only
      the assumption that SPI_fnumber doesn't return an out-of-range attnum.)
      
      While at it, normalize use of SPI_fnumber: make error messages distinguish
      no-such-column from can't-set-system-column, and remove test for deleted
      column which is going to migrate into SPI_fnumber.
      0d444608
    • Robert Haas's avatar
      Improve handling of dead tuples in hash indexes. · f0e72a25
      Robert Haas authored
      When squeezing a bucket during vacuum, it's not necessary to retain
      any tuples already marked as dead, so ignore them when deciding which
      tuples must be moved in order to empty a bucket page.  Similarly, when
      splitting a bucket, relocating dead tuples to the new bucket is a
      waste of effort; instead, just ignore them.
      
      Amit Kapila, reviewed by me.  Testing help provided by Ashutosh
      Sharma.
      f0e72a25
    • Noah Misch's avatar
      Change qr/foo$/m to qr/foo\n/m, for Perl 5.8.8. · 650b9670
      Noah Misch authored
      In each case, absence of a trailing newline would itself constitute a
      PostgreSQL bug.  Therefore, this slightly enhances the changed tests.
      This works around a bug that last appeared in Perl 5.8.8, fixing
      src/test/modules/test_pg_dump when run against that version.  Commit
      e7293e32 worked around the bug, but the
      subsequent addition of test_pg_dump introduced affected code.  As that
      commit had shown, slight increases in pattern complexity can suppress
      the bug.  This commit edits qr/foo$/m patterns too complex to encounter
      the bug today, for style consistency and robustness against unrelated
      pattern changes.  Back-patch to 9.6, where test_pg_dump was introduced.
      
      As of this writing, a fresh MSYS installation includes an affected Perl
      5.8.8.  The Perl 5.8.8 in Red Hat Enterprise Linux 5.11 carries a patch
      that renders it unaffected, but the Perl 5.8.5 of Red Hat Enterprise
      Linux 4.4 is affected.
      650b9670
  9. 07 Nov, 2016 5 commits