1. 02 Feb, 2018 3 commits
    • Robert Haas's avatar
      Refactor code for partition bound searching · 9aef1731
      Robert Haas authored
      Remove partition_bound_cmp() and partition_bound_bsearch(), whose
      void * argument could be, depending on the situation, of any of
      three different types: PartitionBoundSpec *, PartitionRangeBound *,
      Datum *.
      
      Instead, introduce separate bound-searching functions for each
      situation: partition_list_bsearch, partition_range_bsearch,
      partition_range_datum_bsearch, and partition_hash_bsearch.  This
      requires duplicating the code for binary search, but it makes the
      code much more type safe, involves fewer branches at runtime, and
      at least in my opinion, is much easier to understand.
      
      Along the way, add an option to partition_range_datum_bsearch
      allowing the number of keys to be specified, so that we can search
      for partitions based on a prefix of the full list of partition
      keys.  This is important for pending work to improve partition
      pruning.
      
      Amit Langote, per a suggestion from me.
      
      Discussion: http://postgr.es/m/CA+TgmoaVLDLc8=YESRwD32gPhodU_ELmXyKs77gveiYp+JE4vQ@mail.gmail.com
      9aef1731
    • Robert Haas's avatar
      Add new function WaitForParallelWorkersToAttach. · 9222c0d9
      Robert Haas authored
      Once this function has been called, we know that all workers have
      started and attached to their error queues -- so if any of them
      subsequently exit uncleanly, we'll be sure to throw an ERROR promptly.
      Otherwise, users of the ParallelContext machinery must be careful not
      to wait forever for a worker that has failed to start.  Parallel query
      manages to work without needing this for reasons explained in new
      comments added by this patch, but it's a useful primitive for other
      parallel operations, such as the pending patch to make creating a
      btree index run in parallel.
      
      Amit Kapila, revised by me.  Additional review by Peter Geoghegan.
      
      Discussion: http://postgr.es/m/CAA4eK1+e2MzyouF5bg=OtyhDSX+=Ao=3htN=T-r_6s3gCtKFiw@mail.gmail.com
      9222c0d9
    • Stephen Frost's avatar
      Improve ALTER TABLE synopsis · a2a22057
      Stephen Frost authored
      Add into the ALTER TABLE synopsis the definition of
      partition_bound_spec, column_constraint, index_parameters and
      exclude_element.
      
      Initial patch by Lætitia Avrot, with further improvements by Amit
      Langote and Thomas Munro.
      
      Discussion: https://postgr.es/m/flat/27ec4df3-d1ab-3411-f87f-647f944897e1%40lab.ntt.co.jp
      a2a22057
  2. 01 Feb, 2018 2 commits
  3. 31 Jan, 2018 11 commits
  4. 30 Jan, 2018 5 commits
  5. 29 Jan, 2018 7 commits
    • Andres Freund's avatar
      Introduce ExecQualAndReset() helper. · c12693d8
      Andres Freund authored
      It's a common task to evaluate a qual and reset the corresponding
      expression context. Currently that requires storing the result of the
      qual eval, resetting the context, and then reacting on the result. As
      that's awkward several places only reset the context next time through
      a node. That's not great, so introduce a helper that evaluates and
      resets.
      
      It's a bit ugly that it currently uses MemoryContextReset() instead of
      ResetExprContext(), but that seems easier than reordering all of
      executor.h.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20180109222544.f7loxrunqh3xjl5f@alap3.anarazel.de
      c12693d8
    • Tom Lane's avatar
      Save a few bytes by removing useless last argument to SearchCatCacheList. · 97d4445a
      Tom Lane authored
      There's never any value in giving a fully specified cache key to
      SearchCatCacheList: you might as well call SearchCatCache instead,
      since there could be only one match.  So the maximum useful number of
      key arguments is one less than the supported number of key columns.
      We might as well remove the useless extra argument and save some few
      bytes per call site, as well as a cycle or so per call.
      
      I believe the reason it was coded like this is that originally, callers
      had to write out all the dummy arguments in each call, and so it seemed
      less confusing if SearchCatCache and SearchCatCacheList took the same
      number of key arguments.  But since commit e26c539e, callers only write
      their live arguments explicitly, making that a non-factor; and there's
      surely been enough time for third-party modules to adapt to that coding
      style.  So this is only an ABI break not an API break for callers.
      
      Per discussion with Oliver Ford, this might also make it less confusing
      how to use SearchCatCacheList correctly.
      
      Discussion: https://postgr.es/m/27788.1517069693@sss.pgh.pa.us
      97d4445a
    • Andres Freund's avatar
      Initialize unused ExprEvalStep fields. · fc96c694
      Andres Freund authored
      ExecPushExprSlots didn't initialize ExprEvalStep's resvalue/resnull
      steps as it didn't use them. That caused wrong valgrind warnings for
      an upcoming patch, so zero-intialize.
      
      Also zero-initialize all scratch ExprEvalStep's allocated on the
      stack, to avoid issues with similar future omissions of non-critial
      data.
      fc96c694
    • Peter Eisentraut's avatar
      doc: Clarify pg_upgrade documentation · 1e1e599d
      Peter Eisentraut authored
      Clarify that the restriction against reg* types only applies to table
      columns using these types, not to the type appearing in any other way,
      for example as a function argument.
      1e1e599d
    • Andres Freund's avatar
      Prevent growth of simplehash tables when they're "too empty". · ab9f2c42
      Andres Freund authored
      In cases where simplehash tables where filled with either a lot of
      conflicting hash-values, or values that hash to consecutive
      values (i.e. build "chains") the growth heuristics in
      d4c62a6b could trigger rather
      explosively.
      
      To fix that, address some of the reasons (see previous commit) of why
      the growth heuristics where needed, and only allow growth when the
      table isn't too empty. While that means there's a few cases of bad
      input that can be slower, that seems a lot better than running very
      quickly out of memory.
      
      Author: Tomas Vondra and Andres Freund, with additional input by
          Thomas Munro, Tom Lane Todd A. Cook
      Reported-By: Todd A. Cook, Tomas Vondra, Thomas Munro
      Discussion: https://postgr.es/m/20171127185700.1470.20362@wrigleys.postgresql.org
      Backpatch: 10, where simplehash was introduced
      ab9f2c42
    • Andres Freund's avatar
      Improve bit perturbation in TupleHashTableHash. · c068f877
      Andres Freund authored
      The changes in b81b5a96 did not fully
      address the issue, because the bit-mixing of the IV into the final
      hash-key didn't prevent clustering in the input-data survive in the
      output data.
      
      This didn't cause a lot of problems because of the additional growth
      conditions added d4c62a6b. But as we
      want to rein those in due to explosive growth in some edges, this
      needs to be fixed.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20171127185700.1470.20362@wrigleys.postgresql.org
      Backpatch: 10, where simplehash was introduced
      c068f877
    • Tom Lane's avatar
      Avoid misleading psql password prompt when username is multiply specified. · 15be2746
      Tom Lane authored
      When a password is needed, cases such as
      	psql -d "postgresql://alice@localhost/testdb" -U bob
      would incorrectly prompt for "Password for user bob: ", when actually the
      connection will be attempted with username alice.  The priority order of
      which name to use isn't that important here, but the misleading prompt is.
      
      When we are prompting for a password after initial connection failure,
      we can fix this reliably by looking at PQuser(conn) to see how libpq
      interpreted the connection arguments.  But when we're doing a forced
      password prompt because of a -W switch, we can't use that solution.
      Fortunately, because the main use of -W is for noninteractive situations,
      it's less critical to produce a helpful prompt in such cases.  I made
      the startup prompt for -W just say "Password: " all the time, rather
      than expending extra code on trying to identify which username to use.
      In the case of a \c command (after -W has been given), there's already
      logic in do_connect that determines whether the "dbname" is a connstring
      or URI, so we can avoid lobotomizing the prompt except in cases that
      are actually dubious.  (We could do similarly in startup.c if anyone
      complains, but for now it seems not worthwhile, especially since that
      would still be only a partial solution.)
      
      Per bug #15025 from Akos Vandra.  Although this is arguably a bug fix,
      it doesn't seem worth back-patching.  The case where it matters seems
      like a very corner-case usage, and someone might complain that we'd
      changed the behavior of -W in a minor release.
      
      Discussion: https://postgr.es/m/20180123130013.7407.24749@wrigleys.postgresql.org
      15be2746
  6. 28 Jan, 2018 2 commits
    • Tom Lane's avatar
      Add stack-overflow guards in set-operation planning. · 35a52806
      Tom Lane authored
      create_plan_recurse lacked any stack depth check.  This is not per
      our normal coding rules, but I'd supposed it was safe because earlier
      planner processing is more complex and presumably should eat more
      stack.  But bug #15033 from Andrew Grossman shows this isn't true,
      at least not for queries having the form of a many-thousand-way
      INTERSECT stack.
      
      Further testing showed that recurse_set_operations is also capable
      of being crashed in this way, since it likewise will recurse to the
      bottom of a parsetree before calling any support functions that
      might themselves contain any stack checks.  However, its stack
      consumption is only perhaps a third of create_plan_recurse's.
      
      It's possible that this particular problem with create_plan_recurse can
      only manifest in 9.6 and later, since before that we didn't build a Path
      tree for set operations.  But having seen this example, I now have no
      faith in the proposition that create_plan_recurse doesn't need a stack
      check, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20180127050845.28812.58244@wrigleys.postgresql.org
      35a52806
    • Bruce Momjian's avatar
      C includes: Reorder C includes in partition.c · 010123e1
      Bruce Momjian authored
      Discussion: https://postgr.es/m/5A69AA50.2060600@lab.ntt.co.jp
      
      Author: Etsuro Fujita
      010123e1
  7. 27 Jan, 2018 3 commits
    • Tom Lane's avatar
      Update time zone data files to tzdata release 2018c. · 41fc04ff
      Tom Lane authored
      DST law changes in Brazil, Sao Tome and Principe.  Historical corrections
      for Bolivia, Japan, and South Sudan.  The "US/Pacific-New" zone has been
      removed (it was only a link to America/Los_Angeles anyway).
      41fc04ff
    • Tom Lane's avatar
      Avoid crash during EvalPlanQual recheck of an inner indexscan. · 2e668c52
      Tom Lane authored
      Commit 09529a70 changed nodeIndexscan.c and nodeIndexonlyscan.c to
      postpone initialization of the indexscan proper until the first tuple
      fetch.  It overlooked the question of mark/restore behavior, which means
      that if some caller attempts to mark the scan before the first tuple fetch,
      you get a null pointer dereference.
      
      The only existing user of mark/restore is nodeMergejoin.c, which (somewhat
      accidentally) will never attempt to set a mark before the first inner tuple
      unless the inner child node is a Material node.  Hence the case can't arise
      normally, so it seems sufficient to document the assumption at both ends.
      However, during an EvalPlanQual recheck, ExecScanFetch doesn't call
      IndexNext but just returns the jammed-in test tuple.  Therefore, if we're
      doing a recheck in a plan tree with a mergejoin with inner indexscan,
      it's possible to reach ExecIndexMarkPos with iss_ScanDesc still null,
      as reported by Guo Xiang Tan in bug #15032.
      
      Really, when there's a test tuple supplied during an EPQ recheck, touching
      the index at all is the wrong thing: rather, the behavior of mark/restore
      ought to amount to saving and restoring the es_epqScanDone flag.  We can
      avoid finding a place to actually save the flag, for the moment, because
      given the assumption that no caller will set a mark before fetching a
      tuple, es_epqScanDone must always be set by the time we try to mark.
      So the actual behavior change required is just to not reach the index
      access if a test tuple is supplied.
      
      The set of plan node types that need to consider this issue are those
      that support EPQ test tuples (i.e., call ExecScan()) and also support
      mark/restore; which is to say, IndexScan, IndexOnlyScan, and perhaps
      CustomScan.  It's tempting to try to fix the problem in one place by
      teaching ExecMarkPos() itself about EPQ; but ExecMarkPos supports some
      plan types that aren't Scans, and also it seems risky to make assumptions
      about what a CustomScan wants to do here.  Also, the most likely future
      change here is to decide that we do need to support marks placed before
      the first tuple, which would require additional work in IndexScan and
      IndexOnlyScan in any case.  Hence, fix the EPQ issue in nodeIndexscan.c
      and nodeIndexonlyscan.c, accepting the small amount of code duplicated
      thereby, and leave it to CustomScan providers to fix this bug if they
      have it.
      
      Back-patch to v10 where commit 09529a70 came in.  In earlier branches,
      the index_markpos() call is a waste of cycles when EPQ is active, but
      no more than that, so it doesn't seem appropriate to back-patch further.
      
      Discussion: https://postgr.es/m/20180126074932.3098.97815@wrigleys.postgresql.org
      2e668c52
    • Magnus Hagander's avatar
      Add missing semicolons in documentation examples · ba8c2dff
      Magnus Hagander authored
      Author: Daniel Gustafsson <daniel@yesql.se>
      ba8c2dff
  8. 26 Jan, 2018 7 commits
    • Tom Lane's avatar
      Avoid unnecessary use of pg_strcasecmp for already-downcased identifiers. · fb8697b3
      Tom Lane authored
      We have a lot of code in which option names, which from the user's
      viewpoint are logically keywords, are passed through the grammar as plain
      identifiers, and then matched to string literals during command execution.
      This approach avoids making words into lexer keywords unnecessarily.  Some
      places matched these strings using plain strcmp, some using pg_strcasecmp.
      But the latter should be unnecessary since identifiers would have been
      downcased on their way through the parser.  Aside from any efficiency
      concerns (probably not a big factor), the lack of consistency in this area
      creates a hazard of subtle bugs due to different places coming to different
      conclusions about whether two option names are the same or different.
      Hence, standardize on using strcmp() to match any option names that are
      expected to have been fed through the parser.
      
      This does create a user-visible behavioral change, which is that while
      formerly all of these would work:
      	alter table foo set (fillfactor = 50);
      	alter table foo set (FillFactor = 50);
      	alter table foo set ("fillfactor" = 50);
      	alter table foo set ("FillFactor" = 50);
      now the last case will fail because that double-quoted identifier is
      different from the others.  However, none of our documentation says that
      you can use a quoted identifier in such contexts at all, and we should
      discourage doing so since it would break if we ever decide to parse such
      constructs as true lexer keywords rather than poor man's substitutes.
      So this shouldn't create a significant compatibility issue for users.
      
      Daniel Gustafsson, reviewed by Michael Paquier, small changes by me
      
      Discussion: https://postgr.es/m/29405B24-564E-476B-98C0-677A29805B84@yesql.se
      fb8697b3
    • Robert Haas's avatar
      Factor some code out of create_grouping_paths. · 9fd8b7d6
      Robert Haas authored
      This is preparatory refactoring to prepare the way for partition-wise
      aggregate, which will reuse the new subroutines for child grouping
      rels.  It also does not seem like a bad idea on general principle,
      as the function was getting pretty long.
      
      Jeevan Chalke.  The larger patch series of which this patch is a part
      was reviewed and tested by Antonin Houska, Rajkumar Raghuwanshi,
      Ashutosh Bapat, David Rowley, Dilip Kumar, Konstantin Knizhnik,
      Pascal Legrand, and me.  Some cosmetic changes by me.
      
      Discussion: http://postgr.es/m/CAM2+6=V64_xhstVHie0Rz=KPEQnLJMZt_e314P0jaT_oJ9MR8A@mail.gmail.com
      9fd8b7d6
    • Tom Lane's avatar
      Remove the obsolete WITH clause of CREATE FUNCTION. · 4971d2a3
      Tom Lane authored
      This clause was superseded by SQL-standard syntax back in 7.3.
      We've kept it around for backwards-compatibility purposes ever since;
      but 15 years seems like long enough for that, especially seeing that
      there are undocumented weirdnesses in how it interacts with the
      SQL-standard syntax for specifying the same options.
      
      Michael Paquier, per an observation by Daniel Gustafsson;
      some small cosmetic adjustments to nearby code by me.
      
      Discussion: https://postgr.es/m/20180115022748.GB1724@paquier.xyz
      4971d2a3
    • Robert Haas's avatar
      pageinspect: Fix use of wrong memory context by hash_page_items. · b0313f9c
      Robert Haas authored
      This can cause it to produce incorrect output.
      
      Report and patch by Masahiko Sawada.
      
      Discussion: http://postgr.es/m/CAD21AoBc5Asx7pXdUWu6NqU_g=Ysn95EGL9SMeYhLLduYoO_OA@mail.gmail.com
      b0313f9c
    • Peter Eisentraut's avatar
      Use abstracted SSL API in server connection log messages · c1869542
      Peter Eisentraut authored
      The existing "connection authorized" server log messages used OpenSSL
      API calls directly, even though similar abstracted API calls exist.
      Change to use the latter instead.
      
      Change the function prototype for the functions that return the TLS
      version and the cipher to return const char * directly instead of
      copying into a buffer.  That makes them slightly easier to use.
      
      Add bits= to the message.  psql shows that, so we might as well show the
      same information on the client and server.
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      Reviewed-by: default avatarMichael Paquier <michael.paquier@gmail.com>
      c1869542
    • Peter Eisentraut's avatar
      Remove byte-masking macros for Datum conversion macros · a6ef00b5
      Peter Eisentraut authored
      As the comment there stated, these were needed for old-style
      user-defined functions, but since we removed support for those, we don't
      need this anymore.
      Reviewed-by: default avatarMichael Paquier <michael.paquier@gmail.com>
      a6ef00b5
    • Bruce Momjian's avatar
      Fix C comment typo · 6588a43b
      Bruce Momjian authored
      Reported-by: Masahiko Sawada
      
      Discussion: https://postgr.es/m/CAD21AoBgnHy2YKAUuB6iVG4ibvLYepHr+RDRkr1arqWwc1AHCw@mail.gmail.com
      
      Author: Masahiko Sawada
      6588a43b