1. 08 Mar, 2019 5 commits
  2. 07 Mar, 2019 9 commits
    • Alvaro Herrera's avatar
      Fix minor deficiencies in XMLTABLE, xpath(), xmlexists() · 251cf2e2
      Alvaro Herrera authored
      Correctly process nodes of more types than previously.  In some cases,
      nodes were being ignored (nothing was output); in other cases, trying to
      return them resulted in errors about unrecognized nodes.  In yet other
      cases, necessary escaping (of XML special characters) was not being
      done.  Fix all those (as far as the authors could find) and add
      regression tests cases verifying the new behavior.
      
      I (Álvaro) was of two minds about backpatching these changes.  They do
      seem bugfixes that would benefit most users of the affected functions;
      but on the other hand it would change established behavior in minor
      releases, so it seems prudent not to.
      
      Authors: Pavel Stehule, Markus Winand, Chapman Flack
      Discussion:
         https://postgr.es/m/CAFj8pRA6J25CtAZ2TuRvxK3gat7-bBUYh0rfE2yM7Hj9GD14Dg@mail.gmail.com
         https://postgr.es/m/8BDB0627-2105-4564-AA76-7849F028B96E@winand.at
      
      The elephant in the room as pointed out by Chapman Flack, not fixed in
      this commit, is that we still have XMLTABLE operating on XPath 1.0
      instead of the standard-mandated XQuery (or even its subset XPath 2.0).
      Fixing that is a major undertaking, however.
      251cf2e2
    • Tom Lane's avatar
      Fix handling of targetlist SRFs when scan/join relation is known empty. · 1d338584
      Tom Lane authored
      When we introduced separate ProjectSetPath nodes for application of
      set-returning functions in v10, we inadvertently broke some cases where
      we're supposed to recognize that the result of a subquery is known to be
      empty (contain zero rows).  That's because IS_DUMMY_REL was just looking
      for a childless AppendPath without allowing for a ProjectSetPath being
      possibly stuck on top.  In itself, this didn't do anything much worse
      than produce slightly worse plans for some corner cases.
      
      Then in v11, commit 11cf92f6 rearranged things to allow the scan/join
      targetlist to be applied directly to partial paths before they get
      gathered.  But it inserted a short-circuit path for dummy relations
      that was a little too short: it failed to insert a ProjectSetPath node
      at all for a targetlist containing set-returning functions, resulting in
      bogus "set-valued function called in context that cannot accept a set"
      errors, as reported in bug #15669 from Madelaine Thibaut.
      
      The best way to fix this mess seems to be to reimplement IS_DUMMY_REL
      so that it drills down through any ProjectSetPath nodes that might be
      there (and it seems like we'd better allow for ProjectionPath as well).
      
      While we're at it, make it look at rel->pathlist not cheapest_total_path,
      so that it gives the right answer independently of whether set_cheapest
      has been done lately.  That dependency looks pretty shaky in the context
      of code like apply_scanjoin_target_to_paths, and even if it's not broken
      today it'd certainly bite us at some point.  (Nastily, unsafe use of the
      old coding would almost always work; the hazard comes down to possibly
      looking through a dangling pointer, and only once in a blue moon would
      you find something there that resulted in the wrong answer.)
      
      It now looks like it was a mistake for IS_DUMMY_REL to be a macro: if
      there are any extensions using it, they'll continue to use the old
      inadequate logic until they're recompiled, after which they'll fail
      to load into server versions predating this fix.  Hopefully there are
      few such extensions.
      
      Having fixed IS_DUMMY_REL, the special path for dummy rels in
      apply_scanjoin_target_to_paths is unnecessary as well as being wrong,
      so we can just drop it.
      
      Also change a few places that were testing for partitioned-ness of a
      planner relation but not using IS_PARTITIONED_REL for the purpose; that
      seems unsafe as well as inconsistent, plus it required an ugly hack in
      apply_scanjoin_target_to_paths.
      
      In passing, save a few cycles in apply_scanjoin_target_to_paths by
      skipping processing of pre-existing paths for partitioned rels,
      and do some cosmetic cleanup and comment adjustment in that function.
      
      I renamed IS_DUMMY_PATH to IS_DUMMY_APPEND with the intention of breaking
      any code that might be using it, since in almost every case that would
      be wrong; IS_DUMMY_REL is what to be using instead.
      
      In HEAD, also make set_dummy_rel_pathlist static (since it's no longer
      used from outside allpaths.c), and delete is_dummy_plan, since it's no
      longer used anywhere.
      
      Back-patch as appropriate into v11 and v10.
      
      Tom Lane and Julien Rouhaud
      
      Discussion: https://postgr.es/m/15669-02fb3296cca26203@postgresql.org
      1d338584
    • Robert Haas's avatar
      Allow ATTACH PARTITION with only ShareUpdateExclusiveLock. · 898e5e32
      Robert Haas authored
      We still require AccessExclusiveLock on the partition itself, because
      otherwise an insert that violates the newly-imposed partition
      constraint could be in progress at the same time that we're changing
      that constraint; only the lock level on the parent relation is
      weakened.
      
      To make this safe, we have to cope with (at least) three separate
      problems. First, relevant DDL might commit while we're in the process
      of building a PartitionDesc.  If so, find_inheritance_children() might
      see a new partition while the RELOID system cache still has the old
      partition bound cached, and even before invalidation messages have
      been queued.  To fix that, if we see that the pg_class tuple seems to
      be missing or to have a null relpartbound, refetch the value directly
      from the table. We can't get the wrong value, because DETACH PARTITION
      still requires AccessExclusiveLock throughout; if we ever want to
      change that, this will need more thought. In testing, I found it quite
      difficult to hit even the null-relpartbound case; the race condition
      is extremely tight, but the theoretical risk is there.
      
      Second, successive calls to RelationGetPartitionDesc might not return
      the same answer.  The query planner will get confused if lookup up the
      PartitionDesc for a particular relation does not return a consistent
      answer for the entire duration of query planning.  Likewise, query
      execution will get confused if the same relation seems to have a
      different PartitionDesc at different times.  Invent a new
      PartitionDirectory concept and use it to ensure consistency.  This
      ensures that a single invocation of either the planner or the executor
      sees the same view of the PartitionDesc from beginning to end, but it
      does not guarantee that the planner and the executor see the same
      view.  Since this allows pointers to old PartitionDesc entries to
      survive even after a relcache rebuild, also postpone removing the old
      PartitionDesc entry until we're certain no one is using it.
      
      For the most part, it seems to be OK for the planner and executor to
      have different views of the PartitionDesc, because the executor will
      just ignore any concurrently added partitions which were unknown at
      plan time; those partitions won't be part of the inheritance
      expansion, but invalidation messages will trigger replanning at some
      point.  Normally, this happens by the time the very next command is
      executed, but if the next command acquires no locks and executes a
      prepared query, it can manage not to notice until a new transaction is
      started.  We might want to tighten that up, but it's material for a
      separate patch.  There would still be a small window where a query
      that started just after an ATTACH PARTITION command committed might
      fail to notice its results -- but only if the command starts before
      the commit has been acknowledged to the user. All in all, the warts
      here around serializability seem small enough to be worth accepting
      for the considerable advantage of being able to add partitions without
      a full table lock.
      
      Although in general the consequences of new partitions showing up
      between planning and execution are limited to the query not noticing
      the new partitions, run-time partition pruning will get confused in
      that case, so that's the third problem that this patch fixes.
      Run-time partition pruning assumes that indexes into the PartitionDesc
      are stable between planning and execution.  So, add code so that if
      new partitions are added between plan time and execution time, the
      indexes stored in the subplan_map[] and subpart_map[] arrays within
      the plan's PartitionedRelPruneInfo get adjusted accordingly.  There
      does not seem to be a simple way to generalize this scheme to cope
      with partitions that are removed, mostly because they could then get
      added back again with different bounds, but it works OK for added
      partitions.
      
      This code does not try to ensure that every backend participating in
      a parallel query sees the same view of the PartitionDesc.  That
      currently doesn't matter, because we never pass PartitionDesc
      indexes between backends.  Each backend will ignore the concurrently
      added partitions which it notices, and it doesn't matter if different
      backends are ignoring different sets of concurrently added partitions.
      If in the future that matters, for example because we allow writes in
      parallel query and want all participants to do tuple routing to the same
      set of partitions, the PartitionDirectory concept could be improved to
      share PartitionDescs across backends.  There is a draft patch to
      serialize and restore PartitionDescs on the thread where this patch
      was discussed, which may be a useful place to start.
      
      Patch by me.  Thanks to Alvaro Herrera, David Rowley, Simon Riggs,
      Amit Langote, and Michael Paquier for discussion, and to Alvaro
      Herrera for some review.
      
      Discussion: http://postgr.es/m/CA+Tgmobt2upbSocvvDej3yzokd7AkiT+PvgFH+a9-5VV1oJNSQ@mail.gmail.com
      Discussion: http://postgr.es/m/CA+TgmoZE0r9-cyA-aY6f8WFEROaDLLL7Vf81kZ8MtFCkxpeQSw@mail.gmail.com
      Discussion: http://postgr.es/m/CA+TgmoY13KQZF-=HNTrt9UYWYx3_oYOQpu9ioNT49jGgiDpUEA@mail.gmail.com
      898e5e32
    • Alvaro Herrera's avatar
      Fix broken markup · ec51727f
      Alvaro Herrera authored
      ec51727f
    • Alvaro Herrera's avatar
      Fix the BY {REF,VALUE} clause of XMLEXISTS/XMLTABLE · eaaa5986
      Alvaro Herrera authored
      This clause is used to indicate the passing mode of a XML document, but
      we were doing it wrong: we accepted BY REF and ignored it, and rejected
      BY VALUE as a syntax error.  The reality, however, is that documents are
      always passed BY VALUE, so rejecting that clause was silly.  Change
      things so that we accept BY VALUE.
      
      BY REF continues to be accepted, and continues to be ignored.
      
      Author: Chapman Flack
      Reviewed-by: Pavel Stehule
      Discussion: https://postgr.es/m/5C297BB7.9070509@anastigmatix.net
      eaaa5986
    • Alvaro Herrera's avatar
      Add missing <limits.h> · cb706ec4
      Alvaro Herrera authored
      Per buildfarm
      cb706ec4
    • Alvaro Herrera's avatar
      pg_dump: allow multiple rows per insert · 7e413a0f
      Alvaro Herrera authored
      This is useful to speed up loading data in a different database engine.
      
      Authors: Surafel Temesgen and David Rowley.  Lightly edited by Álvaro.
      Reviewed-by: Fabien Coelho
      Discussion: https://postgr.es/m/CALAY4q9kumSdnRBzvRJvSRf2+BH20YmSvzqOkvwpEmodD-xv6g@mail.gmail.com
      7e413a0f
    • Thomas Munro's avatar
      Remove useless header inclusion. · 42210524
      Thomas Munro authored
      42210524
    • Thomas Munro's avatar
      Drop the vestigial "smgr" type. · 91595f9d
      Thomas Munro authored
      Before commit 3fa2bb31 this type appeared in the catalogs to
      select which of several block storage mechanisms each relation
      used.
      
      New features under development propose to revive the concept of
      different block storage managers for new kinds of data accessed
      via bufmgr.c, but don't need to put references to them in the
      catalogs.  So, avoid useless maintenance work on this type by
      dropping it.  Update some regression tests that were referencing
      it where any type would do.
      
      Discussion: https://postgr.es/m/CA%2BhUKG%2BDE0mmiBZMtZyvwWtgv1sZCniSVhXYsXkvJ_Wo%2B83vvw%40mail.gmail.com
      91595f9d
  3. 06 Mar, 2019 12 commits
  4. 05 Mar, 2019 4 commits
  5. 04 Mar, 2019 9 commits
  6. 03 Mar, 2019 1 commit