1. 05 Aug, 2021 1 commit
    • Etsuro Fujita's avatar
      postgres_fdw: Fix issues with generated columns in foreign tables. · 588d3f59
      Etsuro Fujita authored
      postgres_fdw imported generated columns from the remote tables as plain
      columns, and caused failures like "ERROR: cannot insert a non-DEFAULT
      value into column "foo"" when inserting into the foreign tables, as it
      tried to insert values into the generated columns.  To fix, we do the
      following under the assumption that generated columns in a postgres_fdw
      foreign table are defined so that they represent generated columns in
      the underlying remote table:
      
      * Send DEFAULT for the generated columns to the foreign server on insert
        or update, not generated column values computed on the local server.
      * Add to postgresImportForeignSchema() an option "import_generated" to
        include column generated expressions in the definitions of foreign
        tables imported from a foreign server.  The option is true by default.
      
      The assumption seems reasonable, because that would make a query of the
      postgres_fdw foreign table return values for the generated columns that
      are consistent with the generated expression.
      
      While here, fix another issue in postgresImportForeignSchema(): it tried
      to include column generated expressions as column default expressions in
      the foreign table definitions when the import_default option was enabled.
      
      Per bug #16631 from Daniel Cherniy.  Back-patch to v12 where generated
      columns were added.
      
      Discussion: https://postgr.es/m/16631-e929fe9db0ffc7cf%40postgresql.org
      588d3f59
  2. 12 May, 2021 1 commit
    • Tom Lane's avatar
      Initial pgindent and pgperltidy run for v14. · def5b065
      Tom Lane authored
      Also "make reformat-dat-files".
      
      The only change worthy of note is that pgindent messed up the formatting
      of launcher.c's struct LogicalRepWorkerId, which led me to notice that
      that struct wasn't used at all anymore, so I just took it out.
      def5b065
  3. 27 Apr, 2021 1 commit
    • Fujii Masao's avatar
      Don't pass "ONLY" options specified in TRUNCATE to foreign data wrapper. · 8e9ea08b
      Fujii Masao authored
      Commit 8ff1c946 allowed TRUNCATE command to truncate foreign tables.
      Previously the information about "ONLY" options specified in TRUNCATE
      command were passed to the foreign data wrapper. Then postgres_fdw
      constructed the TRUNCATE command to issue the remote server and
      included "ONLY" options in it based on the passed information.
      
      On the other hand, "ONLY" options specified in SELECT, UPDATE or DELETE
      have no effect when accessing or modifying the remote table, i.e.,
      are not passed to the foreign data wrapper. So it's inconsistent to
      make only TRUNCATE command pass the "ONLY" options to the foreign data
      wrapper. Therefore this commit changes the TRUNCATE command so that
      it doesn't pass the "ONLY" options to the foreign data wrapper,
      for the consistency with other statements. Also this commit changes
      postgres_fdw so that it always doesn't include "ONLY" options in
      the TRUNCATE command that it constructs.
      
      Author: Fujii Masao
      Reviewed-by: Bharath Rupireddy, Kyotaro Horiguchi, Justin Pryzby, Zhihong Yu
      Discussion: https://postgr.es/m/551ed8c1-f531-818b-664a-2cecdab99cd8@oss.nttdata.com
      8e9ea08b
  4. 08 Apr, 2021 1 commit
    • Fujii Masao's avatar
      Allow TRUNCATE command to truncate foreign tables. · 8ff1c946
      Fujii Masao authored
      This commit introduces new foreign data wrapper API for TRUNCATE.
      It extends TRUNCATE command so that it accepts foreign tables as
      the targets to truncate and invokes that API. Also it extends postgres_fdw
      so that it can issue TRUNCATE command to foreign servers, by adding
      new routine for that TRUNCATE API.
      
      The information about options specified in TRUNCATE command, e.g.,
      ONLY, CACADE, etc is passed to FDW via API. The list of foreign tables to
      truncate is also passed to FDW. FDW truncates the foreign data sources
      that the passed foreign tables specify, based on those information.
      For example, postgres_fdw constructs TRUNCATE command using them
      and issues it to the foreign server.
      
      For performance, TRUNCATE command invokes the FDW routine for
      TRUNCATE once per foreign server that foreign tables to truncate belong to.
      
      Author: Kazutaka Onishi, Kohei KaiGai, slightly modified by Fujii Masao
      Reviewed-by: Bharath Rupireddy, Michael Paquier, Zhihong Yu, Alvaro Herrera, Stephen Frost, Ashutosh Bapat, Amit Langote, Daniel Gustafsson, Ibrar Ahmed, Fujii Masao
      Discussion: https://postgr.es/m/CAOP8fzb_gkReLput7OvOK+8NHgw-RKqNv59vem7=524krQTcWA@mail.gmail.com
      Discussion: https://postgr.es/m/CAJuF6cMWDDqU-vn_knZgma+2GMaout68YUgn1uyDnexRhqqM5Q@mail.gmail.com
      8ff1c946
  5. 31 Mar, 2021 1 commit
    • Tom Lane's avatar
      Rework planning and execution of UPDATE and DELETE. · 86dc9005
      Tom Lane authored
      This patch makes two closely related sets of changes:
      
      1. For UPDATE, the subplan of the ModifyTable node now only delivers
      the new values of the changed columns (i.e., the expressions computed
      in the query's SET clause) plus row identity information such as CTID.
      ModifyTable must re-fetch the original tuple to merge in the old
      values of any unchanged columns.  The core advantage of this is that
      the changed columns are uniform across all tables of an inherited or
      partitioned target relation, whereas the other columns might not be.
      A secondary advantage, when the UPDATE involves joins, is that less
      data needs to pass through the plan tree.  The disadvantage of course
      is an extra fetch of each tuple to be updated.  However, that seems to
      be very nearly free in context; even worst-case tests don't show it to
      add more than a couple percent to the total query cost.  At some point
      it might be interesting to combine the re-fetch with the tuple access
      that ModifyTable must do anyway to mark the old tuple dead; but that
      would require a good deal of refactoring and it seems it wouldn't buy
      all that much, so this patch doesn't attempt it.
      
      2. For inherited UPDATE/DELETE, instead of generating a separate
      subplan for each target relation, we now generate a single subplan
      that is just exactly like a SELECT's plan, then stick ModifyTable
      on top of that.  To let ModifyTable know which target relation a
      given incoming row refers to, a tableoid junk column is added to
      the row identity information.  This gets rid of the horrid hack
      that was inheritance_planner(), eliminating O(N^2) planning cost
      and memory consumption in cases where there were many unprunable
      target relations.
      
      Point 2 of course requires point 1, so that there is a uniform
      definition of the non-junk columns to be returned by the subplan.
      We can't insist on uniform definition of the row identity junk
      columns however, if we want to keep the ability to have both
      plain and foreign tables in a partitioning hierarchy.  Since
      it wouldn't scale very far to have every child table have its
      own row identity column, this patch includes provisions to merge
      similar row identity columns into one column of the subplan result.
      In particular, we can merge the whole-row Vars typically used as
      row identity by FDWs into one column by pretending they are type
      RECORD.  (It's still okay for the actual composite Datums to be
      labeled with the table's rowtype OID, though.)
      
      There is more that can be done to file down residual inefficiencies
      in this patch, but it seems to be committable now.
      
      FDW authors should note several API changes:
      
      * The argument list for AddForeignUpdateTargets() has changed, and so
      has the method it must use for adding junk columns to the query.  Call
      add_row_identity_var() instead of manipulating the parse tree directly.
      You might want to reconsider exactly what you're adding, too.
      
      * PlanDirectModify() must now work a little harder to find the
      ForeignScan plan node; if the foreign table is part of a partitioning
      hierarchy then the ForeignScan might not be the direct child of
      ModifyTable.  See postgres_fdw for sample code.
      
      * To check whether a relation is a target relation, it's no
      longer sufficient to compare its relid to root->parse->resultRelation.
      Instead, check it against all_result_relids or leaf_result_relids,
      as appropriate.
      
      Amit Langote and Tom Lane
      
      Discussion: https://postgr.es/m/CA+HiwqHpHdqdDn48yCEhynnniahH78rwcrv1rEX65-fsZGBOLQ@mail.gmail.com
      86dc9005
  6. 20 Jan, 2021 1 commit
    • Tomas Vondra's avatar
      Implement support for bulk inserts in postgres_fdw · b663a413
      Tomas Vondra authored
      Extends the FDW API to allow batching inserts into foreign tables. That
      is usually much more efficient than inserting individual rows, due to
      high latency for each round-trip to the foreign server.
      
      It was possible to implement something similar in the regular FDW API,
      but it was inconvenient and there were issues with reporting the number
      of actually inserted rows etc. This extends the FDW API with two new
      functions:
      
      * GetForeignModifyBatchSize - allows the FDW picking optimal batch size
      
      * ExecForeignBatchInsert - inserts a batch of rows at once
      
      Currently, only INSERT queries support batching. Support for DELETE and
      UPDATE may be added in the future.
      
      This also implements batching for postgres_fdw. The batch size may be
      specified using "batch_size" option both at the server and table level.
      
      The initial patch version was written by me, but it was rewritten and
      improved in many ways by Takayuki Tsunakawa.
      
      Author: Takayuki Tsunakawa
      Reviewed-by: Tomas Vondra, Amit Langote
      Discussion: https://postgr.es/m/20200628151002.7x5laxwpgvkyiu3q@development
      b663a413
  7. 02 Jan, 2021 1 commit
  8. 09 Dec, 2020 1 commit
    • Tom Lane's avatar
      Support subscripting of arbitrary types, not only arrays. · c7aba7c1
      Tom Lane authored
      This patch generalizes the subscripting infrastructure so that any
      data type can be subscripted, if it provides a handler function to
      define what that means.  Traditional variable-length (varlena) arrays
      all use array_subscript_handler(), while the existing fixed-length
      types that support subscripting use raw_array_subscript_handler().
      It's expected that other types that want to use subscripting notation
      will define their own handlers.  (This patch provides no such new
      features, though; it only lays the foundation for them.)
      
      To do this, move the parser's semantic processing of subscripts
      (including coercion to whatever data type is required) into a
      method callback supplied by the handler.  On the execution side,
      replace the ExecEvalSubscriptingRef* layer of functions with direct
      calls to callback-supplied execution routines.  (Thus, essentially
      no new run-time overhead should be caused by this patch.  Indeed,
      there is room to remove some overhead by supplying specialized
      execution routines.  This patch does a little bit in that line,
      but more could be done.)
      
      Additional work is required here and there to remove formerly
      hard-wired assumptions about the result type, collation, etc
      of a SubscriptingRef expression node; and to remove assumptions
      that the subscript values must be integers.
      
      One useful side-effect of this is that we now have a less squishy
      mechanism for identifying whether a data type is a "true" array:
      instead of wiring in weird rules about typlen, we can look to see
      if pg_type.typsubscript == F_ARRAY_SUBSCRIPT_HANDLER.  For this
      to be bulletproof, we have to forbid user-defined types from using
      that handler directly; but there seems no good reason for them to
      do so.
      
      This patch also removes assumptions that the number of subscripts
      is limited to MAXDIM (6), or indeed has any hard-wired limit.
      That limit still applies to types handled by array_subscript_handler
      or raw_array_subscript_handler, but to discourage other dependencies
      on this constant, I've moved it from c.h to utils/array.h.
      
      Dmitry Dolgov, reviewed at various times by Tom Lane, Arthur Zakirov,
      Peter Eisentraut, Pavel Stehule
      
      Discussion: https://postgr.es/m/CA+q6zcVDuGBv=M0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w@mail.gmail.com
      Discussion: https://postgr.es/m/CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com
      c7aba7c1
  9. 17 Sep, 2020 1 commit
    • Tom Lane's avatar
      Remove support for postfix (right-unary) operators. · 1ed6b895
      Tom Lane authored
      This feature has been a thorn in our sides for a long time, causing
      many grammatical ambiguity problems.  It doesn't seem worth the
      pain to continue to support it, so remove it.
      
      There are some follow-on improvements we can make in the grammar,
      but this commit only removes the bare minimum number of productions,
      plus assorted backend support code.
      
      Note that pg_dump and psql continue to have full support, since
      they may be used against older servers.  However, pg_dump warns
      about postfix operators.  There is also a check in pg_upgrade.
      
      Documentation-wise, I (tgl) largely removed the "left unary"
      terminology in favor of saying "prefix operator", which is
      a more standard and IMO less confusing term.
      
      I included a catversion bump, although no initial catalog data
      changes here, to mark the boundary at which oprkind = 'r'
      stopped being valid in pg_operator.
      
      Mark Dilger, based on work by myself and Robert Haas;
      review by John Naylor
      
      Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com
      1ed6b895
  10. 26 Jan, 2020 1 commit
    • Tom Lane's avatar
      In postgres_fdw, don't try to ship MULTIEXPR updates to remote server. · 215824f9
      Tom Lane authored
      In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)",
      we'd conclude that the statement could be directly executed remotely,
      because the sub-SELECT is in a resjunk tlist item that's not examined
      for shippability.  Currently that ends up crashing if the sub-SELECT
      contains any remote Vars.  Prevent the crash by deeming MULTIEXEC
      Params to be unshippable.
      
      This is a bit of a brute-force solution, since if the sub-SELECT
      *doesn't* contain any remote Vars, the current execution technology
      would work; but that's not a terribly common use-case for this syntax,
      I think.  In any case, we generally don't try to ship sub-SELECTs, so
      it won't surprise anybody that this doesn't end up as a remote direct
      update.  I'd be inclined to see if that general limitation can be fixed
      before worrying about this case further.
      
      Per report from Lukáš Sobotka.
      
      Back-patch to 9.6.  9.5 had MULTIEXPR, but we didn't try to perform
      remote direct updates then, so the case didn't arise anyway.
      
      Discussion: https://postgr.es/m/CAJif3k+iA_ekBB5Zw2hDBaE1wtiQa4LH4_JUXrrMGwTrH0J01Q@mail.gmail.com
      215824f9
  11. 01 Jan, 2020 1 commit
  12. 24 Oct, 2019 1 commit
  13. 12 Aug, 2019 1 commit
    • Tom Lane's avatar
      Rationalize use of list_concat + list_copy combinations. · 5ee190f8
      Tom Lane authored
      In the wake of commit 1cff1b95, the result of list_concat no longer
      shares the ListCells of the second input.  Therefore, we can replace
      "list_concat(x, list_copy(y))" with just "list_concat(x, y)".
      
      To improve call sites that were list_copy'ing the first argument,
      or both arguments, invent "list_concat_copy()" which produces a new
      list sharing no ListCells with either input.  (This is a bit faster
      than "list_concat(list_copy(x), y)" because it makes the result list
      the right size to start with.)
      
      In call sites that were not list_copy'ing the second argument, the new
      semantics mean that we are usually leaking the second List's storage,
      since typically there is no remaining pointer to it.  We considered
      inventing another list_copy variant that would list_free the second
      input, but concluded that for most call sites it isn't worth worrying
      about, given the relative compactness of the new List representation.
      (Note that in cases where such leakage would happen, the old code
      already leaked the second List's header; so we're only discussing
      the size of the leak not whether there is one.  I did adjust two or
      three places that had been troubling to free that header so that
      they manually free the whole second List.)
      
      Patch by me; thanks to David Rowley for review.
      
      Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
      5ee190f8
  14. 22 Jul, 2019 1 commit
  15. 15 Jul, 2019 1 commit
    • Tom Lane's avatar
      Represent Lists as expansible arrays, not chains of cons-cells. · 1cff1b95
      Tom Lane authored
      Originally, Postgres Lists were a more or less exact reimplementation of
      Lisp lists, which consist of chains of separately-allocated cons cells,
      each having a value and a next-cell link.  We'd hacked that once before
      (commit d0b4399d) to add a separate List header, but the data was still
      in cons cells.  That makes some operations -- notably list_nth() -- O(N),
      and it's bulky because of the next-cell pointers and per-cell palloc
      overhead, and it's very cache-unfriendly if the cons cells end up
      scattered around rather than being adjacent.
      
      In this rewrite, we still have List headers, but the data is in a
      resizable array of values, with no next-cell links.  Now we need at
      most two palloc's per List, and often only one, since we can allocate
      some values in the same palloc call as the List header.  (Of course,
      extending an existing List may require repalloc's to enlarge the array.
      But this involves just O(log N) allocations not O(N).)
      
      Of course this is not without downsides.  The key difficulty is that
      addition or deletion of a list entry may now cause other entries to
      move, which it did not before.
      
      For example, that breaks foreach() and sister macros, which historically
      used a pointer to the current cons-cell as loop state.  We can repair
      those macros transparently by making their actual loop state be an
      integer list index; the exposed "ListCell *" pointer is no longer state
      carried across loop iterations, but is just a derived value.  (In
      practice, modern compilers can optimize things back to having just one
      loop state value, at least for simple cases with inline loop bodies.)
      In principle, this is a semantics change for cases where the loop body
      inserts or deletes list entries ahead of the current loop index; but
      I found no such cases in the Postgres code.
      
      The change is not at all transparent for code that doesn't use foreach()
      but chases lists "by hand" using lnext().  The largest share of such
      code in the backend is in loops that were maintaining "prev" and "next"
      variables in addition to the current-cell pointer, in order to delete
      list cells efficiently using list_delete_cell().  However, we no longer
      need a previous-cell pointer to delete a list cell efficiently.  Keeping
      a next-cell pointer doesn't work, as explained above, but we can improve
      matters by changing such code to use a regular foreach() loop and then
      using the new macro foreach_delete_current() to delete the current cell.
      (This macro knows how to update the associated foreach loop's state so
      that no cells will be missed in the traversal.)
      
      There remains a nontrivial risk of code assuming that a ListCell *
      pointer will remain good over an operation that could now move the list
      contents.  To help catch such errors, list.c can be compiled with a new
      define symbol DEBUG_LIST_MEMORY_USAGE that forcibly moves list contents
      whenever that could possibly happen.  This makes list operations
      significantly more expensive so it's not normally turned on (though it
      is on by default if USE_VALGRIND is on).
      
      There are two notable API differences from the previous code:
      
      * lnext() now requires the List's header pointer in addition to the
      current cell's address.
      
      * list_delete_cell() no longer requires a previous-cell argument.
      
      These changes are somewhat unfortunate, but on the other hand code using
      either function needs inspection to see if it is assuming anything
      it shouldn't, so it's not all bad.
      
      Programmers should be aware of these significant performance changes:
      
      * list_nth() and related functions are now O(1); so there's no
      major access-speed difference between a list and an array.
      
      * Inserting or deleting a list element now takes time proportional to
      the distance to the end of the list, due to moving the array elements.
      (However, it typically *doesn't* require palloc or pfree, so except in
      long lists it's probably still faster than before.)  Notably, lcons()
      used to be about the same cost as lappend(), but that's no longer true
      if the list is long.  Code that uses lcons() and list_delete_first()
      to maintain a stack might usefully be rewritten to push and pop at the
      end of the list rather than the beginning.
      
      * There are now list_insert_nth...() and list_delete_nth...() functions
      that add or remove a list cell identified by index.  These have the
      data-movement penalty explained above, but there's no search penalty.
      
      * list_concat() and variants now copy the second list's data into
      storage belonging to the first list, so there is no longer any
      sharing of cells between the input lists.  The second argument is
      now declared "const List *" to reflect that it isn't changed.
      
      This patch just does the minimum needed to get the new implementation
      in place and fix bugs exposed by the regression tests.  As suggested
      by the foregoing, there's a fair amount of followup work remaining to
      do.
      
      Also, the ENABLE_LIST_COMPAT macros are finally removed in this
      commit.  Code using those should have been gone a dozen years ago.
      
      Patch by me; thanks to David Rowley, Jesper Pedersen, and others
      for review.
      
      Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
      1cff1b95
  16. 04 Jul, 2019 1 commit
  17. 22 May, 2019 1 commit
  18. 27 Apr, 2019 1 commit
    • Tom Lane's avatar
      Avoid postgres_fdw crash for a targetlist entry that's just a Param. · 8cad5adb
      Tom Lane authored
      foreign_grouping_ok() is willing to put fairly arbitrary expressions into
      the targetlist of a remote SELECT that's doing grouping or aggregation on
      the remote side, including expressions that have no foreign component to
      them at all.  This is possibly a bit dubious from an efficiency standpoint;
      but it rises to the level of a crash-causing bug if the expression is just
      a Param or non-foreign Var.  In that case, the expression will necessarily
      also appear in the fdw_exprs list of values we need to send to the remote
      server, and then setrefs.c's set_foreignscan_references will mistakenly
      replace the fdw_exprs entry with a Var referencing the targetlist result.
      
      The root cause of this problem is bad design in commit e7cb7ee1: it put
      logic into set_foreignscan_references that IMV is postgres_fdw-specific,
      and yet this bug shows that it isn't postgres_fdw-specific enough.  The
      transformation being done on fdw_exprs assumes that fdw_exprs is to be
      evaluated with the fdw_scan_tlist as input, which is not how postgres_fdw
      uses it; yet it could be the right thing for some other FDW.  (In the
      bigger picture, setrefs.c has no business assuming this for the other
      expression fields of a ForeignScan either.)
      
      The right fix therefore would be to expand the FDW API so that the
      FDW could inform setrefs.c how it intends to evaluate these various
      expressions.  We can't change that in the back branches though, and we
      also can't just summarily change setrefs.c's behavior there, or we're
      likely to break external FDWs.
      
      As a stopgap, therefore, hack up postgres_fdw so that it won't attempt
      to send targetlist entries that look exactly like the fdw_exprs entries
      they'd produce.  In most cases this actually produces a superior plan,
      IMO, with less data needing to be transmitted and returned; so we probably
      ought to think harder about whether we should ship tlist expressions at
      all when they don't contain any foreign Vars or Aggs.  But that's an
      optimization not a bug fix so I left it for later.  One case where this
      produces an inferior plan is where the expression in question is actually
      a GROUP BY expression: then the restriction prevents us from using remote
      grouping.  It might be possible to work around that (since that would
      reduce to group-by-a-constant on the remote side); but it seems like a
      pretty unlikely corner case, so I'm not sure it's worth expending effort
      solely to improve that.  In any case the right long-term answer is to fix
      the API as sketched above, and then revert this hack.
      
      Per bug #15781 from Sean Johnston.  Back-patch to v10 where the problem
      was introduced.
      
      Discussion: https://postgr.es/m/15781-2601b1002bad087c@postgresql.org
      8cad5adb
  19. 02 Apr, 2019 2 commits
    • Etsuro Fujita's avatar
      postgres_fdw: Perform the (FINAL, NULL) upperrel operations remotely. · d50d172e
      Etsuro Fujita authored
      The upper-planner pathification allows FDWs to arrange to push down
      different types of upper-stage operations to the remote side.  This
      commit teaches postgres_fdw to do it for the (FINAL, NULL) upperrel,
      which is responsible for doing LockRows, LIMIT, and/or ModifyTable.
      This provides the ability for postgres_fdw to handle SELECT commands
      so that it 1) skips the LockRows step (if any) (note that this is
      safe since it performs early locking) and 2) pushes down the LIMIT
      and/or OFFSET restrictions (if any) to the remote side.  This doesn't
      handle the INSERT/UPDATE/DELETE cases.
      
      Author: Etsuro Fujita
      Reviewed-By: Antonin Houska and Jeff Janes
      Discussion: https://postgr.es/m/87pnz1aby9.fsf@news-spur.riddles.org.uk
      d50d172e
    • Etsuro Fujita's avatar
      postgres_fdw: Perform the (ORDERED, NULL) upperrel operations remotely. · ffab494a
      Etsuro Fujita authored
      The upper-planner pathification allows FDWs to arrange to push down
      different types of upper-stage operations to the remote side.  This
      commit teaches postgres_fdw to do it for the (ORDERED, NULL) upperrel,
      which is responsible for evaluating the query's ORDER BY ordering.
      Since postgres_fdw is already able to evaluate that ordering remotely
      for foreign baserels and foreign joinrels (see commit aa09cd24 et al.),
      this adds support for that for foreign grouping relations.
      
      Author: Etsuro Fujita
      Reviewed-By: Antonin Houska and Jeff Janes
      Discussion: https://postgr.es/m/87pnz1aby9.fsf@news-spur.riddles.org.uk
      ffab494a
  20. 01 Feb, 2019 1 commit
  21. 29 Jan, 2019 1 commit
    • Tom Lane's avatar
      Refactor planner's header files. · f09346a9
      Tom Lane authored
      Create a new header optimizer/optimizer.h, which exposes just the
      planner functions that can be used "at arm's length", without need
      to access Paths or the other planner-internal data structures defined
      in nodes/relation.h.  This is intended to provide the whole planner
      API seen by most of the rest of the system; although FDWs still need
      to use additional stuff, and more thought is also needed about just
      what selfuncs.c should rely on.
      
      The main point of doing this now is to limit the amount of new
      #include baggage that will be needed by "planner support functions",
      which I expect to introduce later, and which will be in relevant
      datatype modules rather than anywhere near the planner.
      
      This commit just moves relevant declarations into optimizer.h from
      other header files (a couple of which go away because everything
      got moved), and adjusts #include lists to match.  There's further
      cleanup that could be done if we want to decide that some stuff
      being exposed by optimizer.h doesn't belong in the planner at all,
      but I'll leave that for another day.
      
      Discussion: https://postgr.es/m/11460.1548706639@sss.pgh.pa.us
      f09346a9
  22. 21 Jan, 2019 2 commits
  23. 02 Jan, 2019 1 commit
  24. 21 Nov, 2018 1 commit
    • Andres Freund's avatar
      Remove WITH OIDS support, change oid catalog column visibility. · 578b2297
      Andres Freund authored
      Previously tables declared WITH OIDS, including a significant fraction
      of the catalog tables, stored the oid column not as a normal column,
      but as part of the tuple header.
      
      This special column was not shown by default, which was somewhat odd,
      as it's often (consider e.g. pg_class.oid) one of the more important
      parts of a row.  Neither pg_dump nor COPY included the contents of the
      oid column by default.
      
      The fact that the oid column was not an ordinary column necessitated a
      significant amount of special case code to support oid columns. That
      already was painful for the existing, but upcoming work aiming to make
      table storage pluggable, would have required expanding and duplicating
      that "specialness" significantly.
      
      WITH OIDS has been deprecated since 2005 (commit ff02d0a05280e0).
      Remove it.
      
      Removing includes:
      - CREATE TABLE and ALTER TABLE syntax for declaring the table to be
        WITH OIDS has been removed (WITH (oids[ = true]) will error out)
      - pg_dump does not support dumping tables declared WITH OIDS and will
        issue a warning when dumping one (and ignore the oid column).
      - restoring an pg_dump archive with pg_restore will warn when
        restoring a table with oid contents (and ignore the oid column)
      - COPY will refuse to load binary dump that includes oids.
      - pg_upgrade will error out when encountering tables declared WITH
        OIDS, they have to be altered to remove the oid column first.
      - Functionality to access the oid of the last inserted row (like
        plpgsql's RESULT_OID, spi's SPI_lastoid, ...) has been removed.
      
      The syntax for declaring a table WITHOUT OIDS (or WITH (oids = false)
      for CREATE TABLE) is still supported. While that requires a bit of
      support code, it seems unnecessary to break applications / dumps that
      do not use oids, and are explicit about not using them.
      
      The biggest user of WITH OID columns was postgres' catalog. This
      commit changes all 'magic' oid columns to be columns that are normally
      declared and stored. To reduce unnecessary query breakage all the
      newly added columns are still named 'oid', even if a table's column
      naming scheme would indicate 'reloid' or such.  This obviously
      requires adapting a lot code, mostly replacing oid access via
      HeapTupleGetOid() with access to the underlying Form_pg_*->oid column.
      
      The bootstrap process now assigns oids for all oid columns in
      genbki.pl that do not have an explicit value (starting at the largest
      oid previously used), only oids assigned later by oids will be above
      FirstBootstrapObjectId. As the oid column now is a normal column the
      special bootstrap syntax for oids has been removed.
      
      Oids are not automatically assigned during insertion anymore, all
      backend code explicitly assigns oids with GetNewOidWithIndex(). For
      the rare case that insertions into the catalog via SQL are called for
      the new pg_nextoid() function can be used (which only works on catalog
      tables).
      
      The fact that oid columns on system tables are now normal columns
      means that they will be included in the set of columns expanded
      by * (i.e. SELECT * FROM pg_class will now include the table's oid,
      previously it did not). It'd not technically be hard to hide oid
      column by default, but that'd mean confusing behavior would either
      have to be carried forward forever, or it'd cause breakage down the
      line.
      
      While it's not unlikely that further adjustments are needed, the
      scope/invasiveness of the patch makes it worthwhile to get merge this
      now. It's painful to maintain externally, too complicated to commit
      after the code code freeze, and a dependency of a number of other
      patches.
      
      Catversion bump, for obvious reasons.
      
      Author: Andres Freund, with contributions by John Naylor
      Discussion: https://postgr.es/m/20180930034810.ywp2c7awz7opzcfr@alap3.anarazel.de
      578b2297
  25. 08 Jul, 2018 1 commit
    • Jeff Davis's avatar
      Fix WITH CHECK OPTION on views referencing postgres_fdw tables. · a45adc74
      Jeff Davis authored
      If a view references a foreign table, and the foreign table has a
      BEFORE INSERT trigger, then it's possible for a tuple inserted or
      updated through the view to be changed such that it violates the
      view's WITH CHECK OPTION constraint.
      
      Before this commit, postgres_fdw handled this case inconsistently. A
      RETURNING clause on the INSERT or UPDATE statement targeting the view
      would cause the finally-inserted tuple to be read back, and the WITH
      CHECK OPTION violation would throw an error. But without a RETURNING
      clause, postgres_fdw would not read the final tuple back, and WITH
      CHECK OPTION would not throw an error for the violation (or may throw
      an error when there is no real violation). AFTER ROW triggers on the
      foreign table had a similar effect as a RETURNING clause on the INSERT
      or UPDATE statement.
      
      To fix, this commit retrieves the attributes needed to enforce the
      WITH CHECK OPTION constraint along with the attributes needed for the
      RETURNING clause (if any) from the remote side. Thus, the WITH CHECK
      OPTION constraint is always evaluated against the final tuple after
      any triggers on the remote side.
      
      This fix may be considered inconsistent with CHECK constraints
      declared on foreign tables, which are not enforced locally at all
      (because the constraint is on a remote object). The discussion
      concluded that this difference is reasonable, because the WITH CHECK
      OPTION is a constraint on the local view (not any remote object);
      therefore it only makes sense to enforce its WITH CHECK OPTION
      constraint locally.
      
      Author: Etsuro Fujita
      Reviewed-by: Arthur Zakirov, Stephen Frost
      Discussion: https://www.postgresql.org/message-id/7eb58fab-fd3b-781b-ac33-f7cfec96021f%40lab.ntt.co.jp
      a45adc74
  26. 01 Jul, 2018 1 commit
  27. 01 May, 2018 1 commit
    • Robert Haas's avatar
      Fix interaction of foreign tuple routing with remote triggers. · 37a3058b
      Robert Haas authored
      Without these fixes, changes to the inserted tuple made by remote
      triggers are ignored when building local RETURNING tuples.
      
      In the core code, call ExecInitRoutingInfo at a later point from
      within ExecInitPartitionInfo so that the FDW callback gets invoked
      after the returning list has been built.  But move CheckValidResultRel
      out of ExecInitRoutingInfo so that it can happen at an earlier stage.
      
      In postgres_fdw, refactor assorted deparsing functions to work with
      the RTE rather than the PlannerInfo, which saves us having to
      construct a fake PlannerInfo in cases where we don't have a real one.
      Then, we can pass down a constructed RTE that yields the correct
      deparse result when no real one exists.  Unfortunately, this
      necessitates a hack that understands how the core code manages RT
      indexes for update tuple routing, which is ugly, but we don't have a
      better idea right now.
      
      Original report, analysis, and patch by Etsuro Fujita.  Heavily
      refactored by me.  Then worked over some more by Amit Langote.
      
      Discussion: http://postgr.es/m/5AD4882B.10002@lab.ntt.co.jp
      37a3058b
  28. 01 Mar, 2018 1 commit
  29. 19 Feb, 2018 1 commit
  30. 17 Feb, 2018 1 commit
    • Alvaro Herrera's avatar
      Refactor format_type APIs to be more modular · a26116c6
      Alvaro Herrera authored
      Introduce a new format_type_extended, with a flags bitmask argument that
      can modify the default behavior.  A few compatibility and readability
      wrappers remain:
      	format_type_be
      	format_type_be_qualified
      	format_type_with_typemod
      while format_type_with_typemod_qualified, which had a single caller, is
      removed.
      
      Author: Michael Paquier, some revisions by me
      Discussion: 20180213035107.GA2915@paquier.xyz
      a26116c6
  31. 12 Feb, 2018 1 commit
  32. 07 Feb, 2018 1 commit
    • Robert Haas's avatar
      postgres_fdw: Push down UPDATE/DELETE joins to remote servers. · 1bc0100d
      Robert Haas authored
      Commit 0bf3ae88 allowed direct
      foreign table modification; instead of fetching each row, updating
      it locally, and then pushing the modification back to the remote
      side, we would instead do all the work on the remote server via a
      single remote UPDATE or DELETE command.  However, that commit only
      enabled this optimization when join tree consisted only of the
      target table.
      
      This change allows the same optimization when an UPDATE statement
      has a FROM clause or a DELETE statement has a USING clause.  This
      works much like ordinary foreign join pushdown, in that the tables
      must be on the same remote server, relevant parts of the query
      must be pushdown-safe, and so forth.
      
      Etsuro Fujita, reviewed by Ashutosh Bapat, Rushabh Lathia, and me.
      Some formatting corrections by me.
      
      Discussion: http://postgr.es/m/5A57193A.2080003@lab.ntt.co.jp
      Discussion: http://postgr.es/m/b9cee735-62f8-6c07-7528-6364ce9347d0@lab.ntt.co.jp
      1bc0100d
  33. 12 Jan, 2018 1 commit
    • Tom Lane's avatar
      Fix postgres_fdw to cope with duplicate GROUP BY entries. · e9f2703a
      Tom Lane authored
      Commit 7012b132, which added the ability to push down aggregates and
      grouping to the remote server, wasn't careful to ensure that the remote
      server would have the same idea we do about which columns are the grouping
      columns, in cases where there are textually identical GROUP BY expressions.
      Such cases typically led to "targetlist item has multiple sortgroupref
      labels" errors.
      
      To fix this reliably, switch over to using "GROUP BY column-number" syntax
      rather than "GROUP BY expression" in transmitted queries, and adjust
      foreign_grouping_ok() to be more careful about duplicating the sortgroupref
      labeling of the local pathtarget.
      
      Per bug #14890 from Sean Johnston.  Back-patch to v10 where the buggy code
      was introduced.
      
      Jeevan Chalke, reviewed by Ashutosh Bapat
      
      Discussion: https://postgr.es/m/20171107134948.1508.94783@wrigleys.postgresql.org
      e9f2703a
  34. 03 Jan, 2018 1 commit
  35. 20 Aug, 2017 1 commit
  36. 16 Aug, 2017 1 commit
  37. 21 Jun, 2017 1 commit
    • Tom Lane's avatar
      Phase 3 of pgindent updates. · 382ceffd
      Tom Lane authored
      Don't move parenthesized lines to the left, even if that means they
      flow past the right margin.
      
      By default, BSD indent lines up statement continuation lines that are
      within parentheses so that they start just to the right of the preceding
      left parenthesis.  However, traditionally, if that resulted in the
      continuation line extending to the right of the desired right margin,
      then indent would push it left just far enough to not overrun the margin,
      if it could do so without making the continuation line start to the left of
      the current statement indent.  That makes for a weird mix of indentations
      unless one has been completely rigid about never violating the 80-column
      limit.
      
      This behavior has been pretty universally panned by Postgres developers.
      Hence, disable it with indent's new -lpl switch, so that parenthesized
      lines are always lined up with the preceding left paren.
      
      This patch is much less interesting than the first round of indent
      changes, but also bulkier, so I thought it best to separate the effects.
      
      Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
      Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
      382ceffd
  38. 17 May, 2017 1 commit