Skip to content
Snippets Groups Projects
  1. Jan 16, 2015
    • Noah Misch's avatar
      Update "pg_regress --no-locale" for Darwin and Windows. · 28df6a0d
      Noah Misch authored
      Commit 894459e5 revealed this option to
      be broken for NLS builds on Darwin, but "make -C contrib/unaccent check"
      and the buildfarm client rely on it.  Fix that configuration by
      redefining the option to imply LANG=C on Darwin.  In passing, use LANG=C
      instead of LANG=en on Windows; since only postmaster startup uses that
      value, testers are unlikely to notice the change.  Back-patch to 9.0,
      like the predecessor commit.
      28df6a0d
    • Tom Lane's avatar
      Fix use-of-already-freed-memory problem in EvalPlanQual processing. · c480cb9d
      Tom Lane authored
      Up to now, the "child" executor state trees generated for EvalPlanQual
      rechecks have simply shared the ResultRelInfo arrays used for the original
      execution tree.  However, this leads to dangling-pointer problems, because
      ExecInitModifyTable() is all too willing to scribble on some fields of the
      ResultRelInfo(s) even when it's being run in one of those child trees.
      This trashes those fields from the perspective of the parent tree, because
      even if the generated subtree is logically identical to what was in use in
      the parent, it's in a memory context that will go away when we're done
      with the child state tree.
      
      We do however want to share information in the direction from the parent
      down to the children; in particular, fields such as es_instrument *must*
      be shared or we'll lose the stats arising from execution of the children.
      So the simplest fix is to make a copy of the parent's ResultRelInfo array,
      but not copy any fields back at end of child execution.
      
      Per report from Manuel Kniep.  The added isolation test is based on his
      example.  In an unpatched memory-clobber-enabled build it will reliably
      fail with "ctid is NULL" errors in all branches back to 9.1, as a
      consequence of junkfilter->jf_junkAttNo being overwritten with $7f7f.
      This test cannot be run as-is before that for lack of WITH syntax; but
      I have no doubt that some variant of this problem can arise in older
      branches, so apply the code change all the way back.
      c480cb9d
  2. Jan 15, 2015
    • Heikki Linnakangas's avatar
      Fix thinko in re-setting wal_log_hints flag from a parameter-change record. · 49b04188
      Heikki Linnakangas authored
      The flag is supposed to be copied from the record. Same issue with
      track_commit_timestamps, but that's master-only.
      
      Report and fix by Petr Jalinek. Backpatch to 9.4, where wal_log_hints was
      added.
      49b04188
    • Tom Lane's avatar
      Rearrange explain.c's API so callers need not embed sizeof(ExplainState). · 8e166e16
      Tom Lane authored
      The folly of the previous arrangement was just demonstrated: there's no
      convenient way to add fields to ExplainState without breaking ABI, even
      if callers have no need to touch those fields.  Since we might well need
      to do that again someday in back branches, let's change things so that
      only explain.c has to have sizeof(ExplainState) compiled into it.  This
      costs one extra palloc() per EXPLAIN operation, which is surely pretty
      negligible.
      8e166e16
    • Tom Lane's avatar
      Improve performance of EXPLAIN with large range tables. · a5cd70dc
      Tom Lane authored
      As of 9.3, ruleutils.c goes to some lengths to ensure that table and column
      aliases used in its output are unique.  Of course this takes more time than
      was required before, which in itself isn't fatal.  However, EXPLAIN was set
      up so that recalculation of the unique aliases was repeated for each
      subexpression printed in a plan.  That results in O(N^2) time and memory
      consumption for large plan trees, which did not happen in older branches.
      
      Fortunately, the expensive work is the same across a whole plan tree,
      so there is no need to repeat it; we can do most of the initialization
      just once per query and re-use it for each subexpression.  This buys
      back most (not all) of the performance loss since 9.2.
      
      We need an extra ExplainState field to hold the precalculated deparse
      context.  That's no problem in HEAD, but in the back branches, expanding
      sizeof(ExplainState) seems risky because third-party extensions might
      have local variables of that struct type.  So, in 9.4 and 9.3, introduce
      an auxiliary struct to keep sizeof(ExplainState) the same.  We should
      refactor the APIs to avoid such local variables in future, but that's
      material for a separate HEAD-only commit.
      
      Per gripe from Alexey Bashtanov.  Back-patch to 9.3 where the issue
      was introduced.
      a5cd70dc
    • Andres Freund's avatar
      Blindly try to fix a warning in s_lock.h when compiling with gcc on HPPA. · 6cfd5086
      Andres Freund authored
      The possibly, depending on compiler settings, generated warning was
      "warning: `S_UNLOCK' redefined".
      
      The hppa spinlock implementation doesn't follow the rules of s_lock.h
      and provides a gcc specific implementation outside of the the part of
      the file that's supposed to do that.  It does so to avoid duplication
      between the HP compiler and gcc. That unfortunately means that
      S_UNLOCK is already defined when the HPPA specific section is reached.
      
      Undefine the generic fallback S_UNLOCK definition inside the HPPA
      section. That's far from pretty, but has the big advantage of being
      simple. If somebody is interested to fix this in a prettier way...
      
      This presumably got broken in the course of 0709b7ee.
      
      Discussion: 20150114225919.GY5245@awork2.anarazel.de
      
      Per complaint from Tom Lane.
      6cfd5086
  3. Jan 14, 2015
    • Andres Freund's avatar
      Add a default local latch for use in signal handlers. · 59f71a0d
      Andres Freund authored
      To do so, move InitializeLatchSupport() into the new common process
      initialization functions, and add a new global variable MyLatch.
      
      MyLatch is usable as soon InitPostmasterChild() has been called
      (i.e. very early during startup). Initially it points to a process
      local latch that exists in all processes. InitProcess/InitAuxiliaryProcess
      then replaces that local latch with PGPROC->procLatch. During shutdown
      the reverse happens.
      
      This is primarily advantageous for two reasons: For one it simplifies
      dealing with the shared process latch, especially in signal handlers,
      because instead of having to check for MyProc, MyLatch can be used
      unconditionally. For another, a later patch that makes FEs/BE
      communication use latches, now can rely on the existence of a latch,
      even before having gone through InitProcess.
      
      Discussion: 20140927191243.GD5423@alap3.anarazel.de
      59f71a0d
    • Tom Lane's avatar
      Remove duplicate specification of -Ae for HP-UX C compiler. · fd3d894e
      Tom Lane authored
      Autoconf has known about automatically selecting -Ae when needed for
      quite some time now, so remove the redundant addition in template/hpux.
      Noted while setting up buildfarm member pademelon.
      fd3d894e
    • Andres Freund's avatar
      Remove some dead IsUnderPostmaster code from bootstrap.c. · 0139dea8
      Andres Freund authored
      Since commit 626eb021 has introduced the auxiliary process
      infrastructure, bootstrap_signals() was never used when forked from
      postmaster.
      
      Remove the IsUnderPostmaster specific code, and add a appropriate
      assertion.
      0139dea8
    • Andres Freund's avatar
      Commonalize process startup code. · 31c45316
      Andres Freund authored
      Move common code, that was duplicated in every postmaster child/every
      standalone process, into two functions in miscinit.c.  Not only does
      that already result in a fair amount of net code reduction but it also
      makes it much easier to remove more duplication in the future. The
      prime motivation wasn't code deduplication though, but easier addition
      of new common code.
      31c45316
    • Andres Freund's avatar
      Make logging_collector=on work with non-windows EXEC_BACKEND again. · 2be82dcf
      Andres Freund authored
      Commit b94ce6e8 reordered postmaster's startup sequence so that the
      tempfile directory is only cleaned up after all the necessary state
      for pg_ctl is collected.  Unfortunately the chosen location is after
      the syslogger has been started; which normally is fine, except for
      !WIN32 EXEC_BACKEND builds, which pass information to children via
      files in the temp directory.
      
      Move the call to RemovePgTempFiles() to just before the syslogger has
      started. That's the first child we fork.
      
      Luckily EXEC_BACKEND is pretty much only used by endusers on windows,
      which has a separate method to pass information to children. That
      means the real world impact of this bug is very small.
      
      Discussion: 20150113182344.GF12272@alap3.anarazel.de
      
      Backpatch to 9.1, just as the previous commit was.
      2be82dcf
  4. Jan 13, 2015
    • Heikki Linnakangas's avatar
      Spell the X072 feature correctly, was missing "with". · e922a130
      Heikki Linnakangas authored
      Also use lower-case for a few more features, to be consistent with the
      others and with the SQL spec.
      e922a130
    • Andres Freund's avatar
      Add barriers to the latch code. · 14e8803f
      Andres Freund authored
      Since their introduction latches have required barriers in SetLatch
      and ResetLatch - but when they were introduced there wasn't any
      barrier abstraction. Instead latches were documented to rely on the
      callsites to provide barrier semantics.
      
      Now that the barrier support looks halfway complete, add the necessary
      barriers to both latch implementations.
      
      Also remove a now superflous lock acquisition from syncrep.c and a
      superflous (and insufficient) barrier from freelist.c. There might be
      other cases that can now be simplified, but those are the only ones
      I've seen on a quick scan.
      
      We might want to backpatch this at some later point, but right now the
      barrier infrastructure in the backbranches isn't totally on par with
      master.
      
      Discussion: 20150112154026.GB2092@awork2.anarazel.de
      14e8803f
    • Andres Freund's avatar
      Allow latches to wait for socket writability without waiting for readability. · 4bad60e3
      Andres Freund authored
      So far WaitLatchOrSocket() required to pass in WL_SOCKET_READABLE as
      that solely was used to indicate error conditions, like EOF. Waiting
      for WL_SOCKET_WRITEABLE would have meant to busy wait upon socket
      errors.
      
      Adjust the API to signal errors by returning the socket as readable,
      writable or both, depending on WL_SOCKET_READABLE/WL_SOCKET_WRITEABLE
      being specified.  It would arguably be nicer to return WL_SOCKET_ERROR
      but that's not possible on platforms and would probably also result in
      more complex callsites.
      
      This previously had explicitly been forbidden in e42a21b9, as
      there was no strong use case at that point. We now are looking into
      making FE/BE communication use latches, so changing this makes sense.
      
      There also are some portability concerns because there cases of older
      platforms where select(2) is known to, in violation of POSIX, not
      return a socket as writable after the peer has closed it.  So far the
      platforms where that's the case provide a working poll(2). If we find
      one where that's not the case, we'll need to add a workaround for that
      platform.
      
      Discussion: 20140927191243.GD5423@alap3.anarazel.de
      Reviewed-By: Heikki Linnakangas, Noah Misch
      4bad60e3
    • Heikki Linnakangas's avatar
      Fix typos in comment. · 3dfce376
      Heikki Linnakangas authored
      Plus some tiny wordsmithing of not-quite-typos.
      3dfce376
  5. Jan 12, 2015
    • Tom Lane's avatar
      Fix some functions that were declared static then defined not-static. · 7391e251
      Tom Lane authored
      Per testing with a compiler that whines about this.
      7391e251
    • Tom Lane's avatar
      Avoid unexpected slowdown in vacuum regression test. · 5b3ce2c9
      Tom Lane authored
      I noticed the "vacuum" regression test taking really significantly longer
      than it used to on a slow machine.  Investigation pointed the finger at
      commit e415b469, which added creation of
      an index using an extremely expensive index function.  That function was
      evidently meant to be applied only twice ... but the test re-used an
      existing test table, which up till a couple lines before that had had over
      two thousand rows.  Depending on timing of the concurrent regression tests,
      the intervening VACUUMs might have been unable to remove those
      recently-dead rows, and then the index build would need to create index
      entries for them too, leading to the wrap_do_analyze() function being
      executed 2000+ times not twice.  Avoid this by using a different table
      that is guaranteed to have only the intended two rows in it.
      
      Back-patch to 9.0, like the commit that created the problem.
      5b3ce2c9
    • Alvaro Herrera's avatar
      Tweak heapam's rmgr desc output slightly · d126e1e9
      Alvaro Herrera authored
      Some spaces were missing, and putting the affected tuple offset first in
      the lock cases instead of the locking data makes more sense.
      
      No backpatch since this is cosmetic and surrounding code has changed.
      d126e1e9
    • Alvaro Herrera's avatar
      Fix get_object_address argument type for extension statement · 5c5ffee8
      Alvaro Herrera authored
      Commit 3f88672a neglected to update the AlterExtensionContentsStmt
      production in the grammar to use TypeName to represent types when
      passing objects to get_object_address.
      
      Reported as a pg_upgrade failure by Jeff Janes.
      5c5ffee8
    • Tom Lane's avatar
      Use correct text domain for errcontext() appearing within ereport(). · 1f9bf05e
      Tom Lane authored
      The mechanism added in commit dbdf9679
      for associating the correct translation domain with errcontext strings
      potentially fails in cases where errcontext() is used within an ereport()
      macro.  Such usage was not originally envisioned for errcontext(), but we
      do have a few places that do it.  In this situation, the intended comma
      expression becomes just a couple of arguments to errfinish(), which the
      compiler might choose to evaluate right-to-left.
      
      Fortunately, in such cases the textdomain for the errcontext string must
      be the same as for the surrounding ereport.  So we can fix this by letting
      errstart initialize context_domain along with domain; then it will have
      the correct value no matter which order the calls occur in.  (Note that
      error stack callback functions are not invoked until errfinish, so normal
      usage of errcontext won't affect what happens for errcontext calls within
      the ereport macro.)
      
      In passing, make sure that errcontext calls within the main backend set
      context_domain to something non-NULL.  This isn't a live bug because
      NULL would select the current textdomain() setting which should be the
      right thing anyway --- but it seems better to handle this completely
      consistently with the regular domain field.
      
      Per report from Dmitry Voronin.  Backpatch to 9.3; before that, there
      wasn't any attempt to ensure that errcontext strings were translated
      in an appropriate domain.
      1f9bf05e
    • Stephen Frost's avatar
      Skip dead backends in MinimumActiveBackends · 1bf4a84d
      Stephen Frost authored
      Back in ed0b409d, PGPROC was split and moved to static variables in
      procarray.c, with procs in ProcArrayStruct replaced by an array of
      integers representing process numbers (pgprocnos), with -1 indicating a
      dead process which has yet to be removed.  Access to procArray is
      generally done under ProcArrayLock and therefore most code does not have
      to concern itself with -1 entries.
      
      However, MinimumActiveBackends intentionally does not take
      ProcArrayLock, which means it has to be extra careful when accessing
      procArray.  Prior to ed0b409d, this was handled by checking for a NULL
      in the pointer array, but that check was no longer valid after the
      split.  Coverity pointed out that the check could never happen and so
      it was removed in 5592ebac.  That didn't make anything worse, but it
      didn't fix the issue either.
      
      The correct fix is to check for pgprocno == -1 and skip over that entry
      if it is encountered.
      
      Back-patch to 9.2, since there can be attempts to access the arrays
      prior to their start otherwise.  Note that the changes prior to 9.4 will
      look a bit different due to the change in 5592ebac.
      
      Note that MinimumActiveBackends only returns a bool for heuristic
      purposes and any pre-array accesses are strictly read-only and so there
      is no security implication and the lack of fields complaints indicates
      it's very unlikely to run into issues due to this.
      
      Pointed out by Noah.
      1bf4a84d
  6. Jan 11, 2015
    • Tom Lane's avatar
      Fix portability breakage in pg_dump. · 44096f1c
      Tom Lane authored
      Commit 0eea8047 introduced some overly
      optimistic assumptions about what could be in a local struct variable's
      initializer.  (This might in fact be valid code according to C99, but I've
      got at least one pre-C99 compiler that falls over on those nonconstant
      address expressions.)  There is no reason whatsoever for main()'s workspace
      to not be static, so revert long_options[] to a static and make the
      DumpOptions struct static as well.
      44096f1c
    • Tom Lane's avatar
      Remove configure test for nonstandard variants of getpwuid_r(). · 8883bae3
      Tom Lane authored
      We had code that supposed that some platforms might offer a nonstandard
      version of getpwuid_r() with only four arguments.  However, the 5-argument
      definition has been standardized at least since the Single Unix Spec v2,
      which is our normal reference for what's portable across all Unix-oid
      platforms.  (What's more, this wasn't the only pre-standardization version
      of getpwuid_r(); my old HPUX 10.20 box has still another signature.)
      So let's just get rid of the now-useless configure step.
      8883bae3
    • Tom Lane's avatar
      Fix libpq's behavior when /etc/passwd isn't readable. · 080eabe2
      Tom Lane authored
      Some users run their applications in chroot environments that lack an
      /etc/passwd file.  This means that the current UID's user name and home
      directory are not obtainable.  libpq used to be all right with that,
      so long as the database role name to use was specified explicitly.
      But commit a4c8f143 broke such cases by
      causing any failure of pg_fe_getauthname() to be treated as a hard error.
      In any case it did little to advance its nominal goal of causing errors
      in pg_fe_getauthname() to be reported better.  So revert that and instead
      put some real error-reporting code in place.  This requires changes to the
      APIs of pg_fe_getauthname() and pqGetpwuid(), since the latter had
      departed from the POSIX-specified API of getpwuid_r() in a way that made
      it impossible to distinguish actual lookup errors from "no such user".
      
      To allow such failures to be reported, while not failing if the caller
      supplies a role name, add a second call of pg_fe_getauthname() in
      connectOptions2().  This is a tad ugly, and could perhaps be avoided with
      some refactoring of PQsetdbLogin(), but I'll leave that idea for later.
      (Note that the complained-of misbehavior only occurs in PQsetdbLogin,
      not when using the PQconnect functions, because in the latter we will
      never bother to call pg_fe_getauthname() if the user gives a role name.)
      
      In passing also clean up the Windows-side usage of GetUserName(): the
      recommended buffer size is 257 bytes, the passed buffer length should
      be the buffer size not buffer size less 1, and any error is reported
      by GetLastError() not errno.
      
      Per report from Christoph Berg.  Back-patch to 9.4 where the chroot
      failure case was introduced.  The generally poor reporting of errors
      here is of very long standing, of course, but given the lack of field
      complaints about it we won't risk changing these APIs further back
      (even though they're theoretically internal to libpq).
      080eabe2
    • Andres Freund's avatar
      Provide a generic fallback for pg_compiler_barrier using an extern function. · de6429a8
      Andres Freund authored
      If the compiler/arch combination does not provide compiler barriers,
      provide a fallback. That fallback simply consists out of a function
      call into a externally defined function.  That should guarantee
      compiler barrierer semantics except for compilers that do inter
      translation unit/global optimization - those better provide an actual
      compiler barrier.
      
      Hopefully this fixes Tom's report of linker failures due to
      pg_compiler_barrier_impl not being provided.
      
      I'm not backpatching this commit as it builds on the new atomics
      infrastructure. If we decide an equivalent fix needs to be
      backpatched, I'll do so in a separate commit.
      
      Discussion: 27746.1420930690@sss.pgh.pa.us
      
      Per report from Tom Lane.
      de6429a8
    • Andres Freund's avatar
      Fix alignment of pg_atomic_uint64 variables on some 32bit platforms. · db4ec2ff
      Andres Freund authored
      I failed to recognize that pg_atomic_uint64 wasn't guaranteed to be 8
      byte aligned on some 32bit platforms - which it has to be on some
      platforms to guarantee the desired atomicity and which we assert.
      
      As this is all compiler specific code anyway we can just rely on
      compiler specific tricks to enforce alignment.
      
      I've been unable to find concrete documentation about the version that
      introduce the sunpro alignment support, so that might need additional
      guards.
      
      I've verified that this works with gcc x86 32bit, but I don't have
      access to any other 32bit environment.
      
      Discussion: op.xpsjdkil0sbe7t@vld-kuci
      
      Per report from Vladimir Koković.
      db4ec2ff
  7. Jan 09, 2015
  8. Jan 08, 2015
    • Stephen Frost's avatar
      Move rowsecurity event trigger test · c219cbfe
      Stephen Frost authored
      The event trigger test for rowsecurity can cause problems for other
      tests which are run in parallel with it.  Instead of running that test
      in the rowsecurity set, move it to the event_trigger set, which runs
      isolated from other tests.
      
      Also reverts 7161b082, which moved rowsecurity into its own test group.
      That's no longer necessary, now that the event trigger test is gone from
      the rowsecurity set of tests.
      
      Pointed out by Tom.
      c219cbfe
    • Andres Freund's avatar
      Remove comment that was intended to have been removed before commit. · f454144a
      Andres Freund authored
      Noticed by Amit Kapila
      f454144a
    • Andres Freund's avatar
      Move comment about sun cc's __machine_rw_barrier being a full barrier. · 93be0950
      Andres Freund authored
      I'd accidentally written the comment besides the read barrier, instead
      of the full barrier, implementation.
      
      Noticed by Oskari Saarenmaa
      93be0950
    • Andres Freund's avatar
      Fix logging of pages skipped due to pins during vacuum. · 17eaae98
      Andres Freund authored
      The new logging introduced in 35192f06 made the incorrect assumption
      that scan_all vacuums would always wait for buffer pins; but they only
      do so if the page actually needs to be frozen.
      
      Fix that inaccuracy by removing the difference in log output based on
      scan_all and just always remove the same message.  I chose to keep the
      split log message from the original commit for now, it seems likely
      that it'll be of use in the future.
      
      Also merge the line about buffer pins in autovacuum's log output into
      the existing "pages: ..." line. It seems odd to have a separate line
      about pins, without the "topic: " prefix others have.
      
      Also rename the new 'pinned_pages' variable to 'pinskipped_pages'
      because it actually tracks the number of pages that could *not* be
      pinned.
      
      Discussion: 20150104005324.GC9626@awork2.anarazel.de
      17eaae98
    • Noah Misch's avatar
      On Darwin, refuse postmaster startup when multithreaded. · 2048e5b8
      Noah Misch authored
      The previous commit introduced its report at LOG level to avoid
      surprises at minor release upgrade time.  Compel users deploying the
      next major release to also deploy the reported workaround.
      2048e5b8
    • Noah Misch's avatar
      On Darwin, detect and report a multithreaded postmaster. · 894459e5
      Noah Misch authored
      Darwin --enable-nls builds use a substitute setlocale() that may start a
      thread.  Buildfarm member orangutan experienced BackendList corruption
      on account of different postmaster threads executing signal handlers
      simultaneously.  Furthermore, a multithreaded postmaster risks undefined
      behavior from sigprocmask() and fork().  Emit LOG messages about the
      problem and its workaround.  Back-patch to 9.0 (all supported versions).
      894459e5
    • Noah Misch's avatar
      Always set the six locale category environment variables in main(). · 6fdba8ce
      Noah Misch authored
      Typical server invocations already achieved that.  Invalid locale
      settings in the initial postmaster environment interfered, as could
      malloc() failure.  Setting "LC_MESSAGES=pt_BR.utf8 LC_ALL=invalid" in
      the postmaster environment will now choose C-locale messages, not
      Brazilian Portuguese messages.  Most localized programs, including all
      PostgreSQL frontend executables, do likewise.  Users are unlikely to
      observe changes involving locale categories other than LC_MESSAGES.
      CheckMyDatabase() ensures that we successfully set LC_COLLATE and
      LC_CTYPE; main() sets the remaining three categories to locale "C",
      which almost cannot fail.  Back-patch to 9.0 (all supported versions).
      6fdba8ce
    • Noah Misch's avatar
      Reject ANALYZE commands during VACUUM FULL or another ANALYZE. · e415b469
      Noah Misch authored
      vacuum()'s static variable handling makes it non-reentrant; an ensuing
      null pointer deference crashed the backend.  Back-patch to 9.0 (all
      supported versions).
      e415b469
  9. Jan 07, 2015
    • Heikki Linnakangas's avatar
      Don't open a WAL segment for writing at end of recovery. · 1e78d81e
      Heikki Linnakangas authored
      Since commit ba94518a, we used XLogFileOpen to open the next segment for
      writing, but if the end-of-recovery happens exactly at a segment boundary,
      the new segment might not exist yet. (Before ba94518a, XLogFileOpen was
      correct, because we would open the previous segment if the switch happened
      at the boundary.)
      
      Instead of trying to create it if necessary, it's simpler to not bother
      opening the segment at all. XLogWrite() will open or create it soon anyway,
      after writing the checkpoint or end-of-recovery record.
      
      Reported by Andres Freund.
      1e78d81e
    • Peter Eisentraut's avatar
      Fix namespace handling in xpath function · 79af9a1d
      Peter Eisentraut authored
      Previously, the xml value resulting from an xpath query would not have
      namespace declarations if the namespace declarations were attached to
      an ancestor element in the input xml value.  That means the output value
      was not correct XML.  Fix that by running the result value through
      xmlCopyNode(), which produces the correct namespace declarations.
      
      Author: Ali Akbar <the.apaan@gmail.com>
      79af9a1d
    • Andres Freund's avatar
      Correctly handle relcache invalidation corner case during logical decoding. · 3fabed07
      Andres Freund authored
      When using a historic snapshot for logical decoding it can validly
      happen that a relation that's in the relcache isn't visible to that
      historic snapshot.  E.g. if a newly created relation is referenced in
      the query that uses the SQL interface for logical decoding and a
      sinval reset occurs.
      
      The earlier commit that fixed the error handling for that corner case
      already improves the situation as a ERROR is better than hitting an
      assertion... But it's obviously not good enough.  So additionally
      allow that case without an error if a historic snapshot is set up -
      that won't allow an invalid entry to stay in the cache because it's a)
      already marked invalid and will thus be rebuilt during the next access
      b) the syscaches will be reset at the end of decoding.
      
      There might be prettier solutions to handle this case, but all that we
      could think of so far end up being much more complex than this quite
      simple fix.
      
      This fixes the assertion failures reported by the buildfarm (markhor,
      tick, leech) after the introduction of new regression tests in
      89fd41b3. The failure there weren't actually directly caused by
      CLOBBER_CACHE_ALWAYS but the extraordinary long runtimes due to it
      lead to sinval resets triggering the behaviour.
      
      Discussion: 22459.1418656530@sss.pgh.pa.us
      
      Backpatch to 9.4 where logical decoding was introduced.
      3fabed07
    • Andres Freund's avatar
      Improve relcache invalidation handling of currently invisible relations. · 31912d01
      Andres Freund authored
      The corner case where a relcache invalidation tried to rebuild the
      entry for a referenced relation but couldn't find it in the catalog
      wasn't correct.
      
      The code tried to RelationCacheDelete/RelationDestroyRelation the
      entry. That didn't work when assertions are enabled because the latter
      contains an assertion ensuring the refcount is zero. It's also more
      generally a bad idea, because by virtue of being referenced somebody
      might actually look at the entry, which is possible if the error is
      trapped and handled via a subtransaction abort.
      
      Instead just error out, without deleting the entry. As the entry is
      marked invalid, the worst that can happen is that the invalid (and at
      some point unused) entry lingers in the relcache.
      
      Discussion: 22459.1418656530@sss.pgh.pa.us
      
      There should be no way to hit this case < 9.4 where logical decoding
      introduced a bug that can hit this. But since the code for handling
      the corner case is there it should do something halfway sane, so
      backpatch all the the way back.  The logical decoding bug will be
      handled in a separate commit.
      31912d01
Loading