Skip to content
Snippets Groups Projects
  1. Jun 21, 2017
    • 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
    • Tom Lane's avatar
      Phase 2 of pgindent updates. · c7b8998e
      Tom Lane authored
      Change pg_bsd_indent to follow upstream rules for placement of comments
      to the right of code, and remove pgindent hack that caused comments
      following #endif to not obey the general rule.
      
      Commit e3860ffa wasn't actually using
      the published version of pg_bsd_indent, but a hacked-up version that
      tried to minimize the amount of movement of comments to the right of
      code.  The situation of interest is where such a comment has to be
      moved to the right of its default placement at column 33 because there's
      code there.  BSD indent has always moved right in units of tab stops
      in such cases --- but in the previous incarnation, indent was working
      in 8-space tab stops, while now it knows we use 4-space tabs.  So the
      net result is that in about half the cases, such comments are placed
      one tab stop left of before.  This is better all around: it leaves
      more room on the line for comment text, and it means that in such
      cases the comment uniformly starts at the next 4-space tab stop after
      the code, rather than sometimes one and sometimes two tabs after.
      
      Also, ensure that comments following #endif are indented the same
      as comments following other preprocessor commands such as #else.
      That inconsistency turns out to have been self-inflicted damage
      from a poorly-thought-through post-indent "fixup" in pgindent.
      
      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
      c7b8998e
    • Tom Lane's avatar
      Initial pgindent run with pg_bsd_indent version 2.0. · e3860ffa
      Tom Lane authored
      The new indent version includes numerous fixes thanks to Piotr Stefaniak.
      The main changes visible in this commit are:
      
      * Nicer formatting of function-pointer declarations.
      * No longer unexpectedly removes spaces in expressions using casts,
        sizeof, or offsetof.
      * No longer wants to add a space in "struct structname *varname", as
        well as some similar cases for const- or volatile-qualified pointers.
      * Declarations using PG_USED_FOR_ASSERTS_ONLY are formatted more nicely.
      * Fixes bug where comments following declarations were sometimes placed
        with no space separating them from the code.
      * Fixes some odd decisions for comments following case labels.
      * Fixes some cases where comments following code were indented to less
        than the expected column 33.
      
      On the less good side, it now tends to put more whitespace around typedef
      names that are not listed in typedefs.list.  This might encourage us to
      put more effort into typedef name collection; it's not really a bug in
      indent itself.
      
      There are more changes coming after this round, having to do with comment
      indentation and alignment of lines appearing within parentheses.  I wanted
      to limit the size of the diffs to something that could be reviewed without
      one's eyes completely glazing over, so it seemed better to split up the
      changes as much as practical.
      
      Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
      Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
      e3860ffa
  2. May 18, 2017
  3. May 13, 2017
    • Tom Lane's avatar
      Redesign get_attstatsslot()/free_attstatsslot() for more safety and speed. · 9aab83fc
      Tom Lane authored
      The mess cleaned up in commit da075960 is clear evidence that it's a
      bug hazard to expect the caller of get_attstatsslot()/free_attstatsslot()
      to provide the correct type OID for the array elements in the slot.
      Moreover, we weren't even getting any performance benefit from that,
      since get_attstatsslot() was extracting the real type OID from the array
      anyway.  So we ought to get rid of that requirement; indeed, it would
      make more sense for get_attstatsslot() to pass back the type OID it found,
      in case the caller isn't sure what to expect, which is likely in binary-
      compatible-operator cases.
      
      Another problem with the current implementation is that if the stats array
      element type is pass-by-reference, we incur a palloc/memcpy/pfree cycle
      for each element.  That seemed acceptable when the code was written because
      we were targeting O(10) array sizes --- but these days, stats arrays are
      almost always bigger than that, sometimes much bigger.  We can save a
      significant number of cycles by doing one palloc/memcpy/pfree of the whole
      array.  Indeed, in the now-probably-common case where the array is toasted,
      that happens anyway so this method is basically free.  (Note: although the
      catcache code will inline any out-of-line toasted values, it doesn't
      decompress them.  At the other end of the size range, it doesn't expand
      short-header datums either.  In either case, DatumGetArrayTypeP would have
      to make a copy.  We do end up using an extra array copy step if the element
      type is pass-by-value and the array length is neither small enough for a
      short header nor large enough to have suffered compression.  But that
      seems like a very acceptable price for winning in pass-by-ref cases.)
      
      Hence, redesign to take these insights into account.  While at it,
      convert to an API in which we fill a struct rather than passing a bunch
      of pointers to individual output arguments.  That will make it less
      painful if we ever want further expansion of what get_attstatsslot can
      pass back.
      
      It's certainly arguable that this is new development and not something to
      push post-feature-freeze.  However, I view it as primarily bug-proofing
      and therefore something that's better to have sooner not later.  Since
      we aren't quite at beta phase yet, let's put it in.
      
      Discussion: https://postgr.es/m/16364.1494520862@sss.pgh.pa.us
      9aab83fc
  4. Mar 27, 2017
  5. Mar 13, 2017
    • Noah Misch's avatar
      Use wrappers of PG_DETOAST_DATUM_PACKED() more. · 3a0d4731
      Noah Misch authored
      This makes almost all core code follow the policy introduced in the
      previous commit.  Specific decisions:
      
      - Text search support functions with char* and length arguments, such as
        prsstart and lexize, may receive unaligned strings.  I doubt
        maintainers of non-core text search code will notice.
      
      - Use plain VARDATA() on values detoasted or synthesized earlier in the
        same function.  Use VARDATA_ANY() on varlenas sourced outside the
        function, even if they happen to always have four-byte headers.  As an
        exception, retain the universal practice of using VARDATA() on return
        values of SendFunctionCall().
      
      - Retain PG_GETARG_BYTEA_P() in pageinspect.  (Page images are too large
        for a one-byte header, so this misses no optimization.)  Sites that do
        not call get_page_from_raw() typically need the four-byte alignment.
      
      - For now, do not change btree_gist.  Its use of four-byte headers in
        memory is partly entangled with storage of 4-byte headers inside
        GBT_VARKEY, on disk.
      
      - For now, do not change gtrgm_consistent() or gtrgm_distance().  They
        incorporate the varlena header into a cache, and there are multiple
        credible implementation strategies to consider.
      3a0d4731
  6. Jan 17, 2017
  7. Jan 03, 2017
  8. Nov 29, 2016
    • Tom Lane's avatar
      Test all contrib-created operator classes with amvalidate. · ade49c60
      Tom Lane authored
      I'd supposed that people would do this manually when creating new operator
      classes, but the folly of that was exposed today.  The tests seem fast
      enough that we can just apply them during the normal regression tests.
      
      contrib/isn fails the checks for lack of complete sets of cross-type
      operators.  That's a nice-to-have policy rather than a functional
      requirement, so leave it as-is, but insert ORDER BY in the query to
      ensure consistent cross-platform output.
      
      Discussion: https://postgr.es/m/7076.1480446837@sss.pgh.pa.us
      ade49c60
  9. Aug 17, 2016
    • Tom Lane's avatar
      Fix -e option in contrib/intarray/bench/bench.pl. · 6657acc0
      Tom Lane authored
      As implemented, -e ran an EXPLAIN but then discarded the output, which
      certainly seems pointless.  Make it print to stdout instead.  It's been
      like that forever, so back-patch to all supported branches.
      
      Daniel Gustafsson, reviewed by Andreas Scherbaum
      
      Patch: <B97BDCB7-A3B3-4734-90B5-EDD586941629@yesql.se>
      6657acc0
  10. Jun 14, 2016
    • Robert Haas's avatar
      Update extensions with GIN/GIST support for parallel query. · 2910fc82
      Robert Haas authored
      Commit 749a787c bumped the extension
      version on all of these extensions already, and we haven't had a
      release since then, so we can make further changes without bumping the
      extension version again.  Take this opportunity to mark all of the
      functions exported by these modules PARALLEL SAFE -- except for
      pg_trgm's set_limit().  Mark that one PARALLEL RESTRICTED, because it
      makes a persistent change to a GUC value.
      
      Note that some of the markings added by this commit don't have any
      effect; for example, gseg_picksplit() isn't likely to be mentioned
      explicitly in a query and therefore it's parallel-safety marking will
      never be consulted.  But this commit just marks everything for
      consistency: if it were somehow used in a query, that would be fine as
      far as parallel query is concerned, since it does not consult any
      backend-private state, attempt to write data, etc.
      
      Andreas Karlsson, with a few revisions by me.
      2910fc82
  11. Jun 09, 2016
    • Tom Lane's avatar
      Handle contrib's GIN/GIST support function signature changes honestly. · 749a787c
      Tom Lane authored
      In commits 9ff60273 and dbe23289 I (tgl) fixed the
      signatures of a bunch of contrib's GIN and GIST support functions so that
      they would pass validation by the recently-added amvalidate functions.
      The backend does not actually consult or check those signatures otherwise,
      so I figured this was basically cosmetic and did not require an extension
      version bump.  However, Alexander Korotkov pointed out that that would
      leave us in a pretty messy situation if we ever wanted to redefine those
      functions later, because there wouldn't be a unique way to name them.
      Since we're going to be bumping these extensions' versions anyway for
      parallel-query cleanups, let's take care of this now.
      
      Andreas Karlsson, adjusted for more search-path-safety by me
      749a787c
  12. Jan 20, 2016
    • Tom Lane's avatar
      Fix assorted inconsistencies in GIN opclass support function declarations. · dbe23289
      Tom Lane authored
      GIN had some minor issues too, mostly using "internal" where something
      else would be more appropriate.  I went with the same approach as in
      9ff60273, namely preferring the opclass' indexed datatype for
      arguments that receive an operator RHS value, even if that's not
      necessarily what they really are.
      
      Again, this is with an eye to having a uniform rule for ginvalidate()
      to check support function signatures.
      dbe23289
  13. Jan 19, 2016
    • Tom Lane's avatar
      Fix assorted inconsistencies in GiST opclass support function declarations. · 9ff60273
      Tom Lane authored
      The conventions specified by the GiST SGML documentation were widely
      ignored.  For example, the strategy-number argument for "consistent" and
      "distance" functions is specified to be a smallint, but most of the
      built-in support functions declared it as an integer, and for that matter
      the core code passed it using Int32GetDatum not Int16GetDatum.  None of
      that makes any real difference at runtime, but it's quite confusing for
      newcomers to the code, and it makes it very hard to write an amvalidate()
      function that checks support function signatures.  So let's try to instill
      some consistency here.
      
      Another similar issue is that the "query" argument is not of a single
      well-defined type, but could have different types depending on the strategy
      (corresponding to search operators with different righthand-side argument
      types).  Some of the functions threw up their hands and declared the query
      argument as being of "internal" type, which surely isn't right ("any" would
      have been more appropriate); but the majority position seemed to be to
      declare it as being of the indexed data type, corresponding to a search
      operator with both input types the same.  So I've specified a convention
      that that's what to do always.
      
      Also, the result of the "union" support function actually must be of the
      index's storage type, but the documentation suggested declaring it to
      return "internal", and some of the functions followed that.  Standardize
      on telling the truth, instead.
      
      Similarly, standardize on declaring the "same" function's inputs as
      being of the storage type, not "internal".
      
      Also, somebody had forgotten to add the "recheck" argument to both
      the documentation of the "distance" support function and all of their
      SQL declarations, even though the C code was happily using that argument.
      Clean that up too.
      
      Fix up some other omissions in the docs too, such as documenting that
      union's second input argument is vestigial.
      
      So far as the errors in core function declarations go, we can just fix
      pg_proc.h and bump catversion.  Adjusting the erroneous declarations in
      contrib modules is more debatable: in principle any change in those
      scripts should involve an extension version bump, which is a pain.
      However, since these changes are purely cosmetic and make no functional
      difference, I think we can get away without doing that.
      9ff60273
  14. Jan 02, 2016
  15. Oct 05, 2015
    • Noah Misch's avatar
      Prevent stack overflow in query-type functions. · 5976097c
      Noah Misch authored
      The tsquery, ltxtquery and query_int data types have a common ancestor.
      Having acquired check_stack_depth() calls independently, each was
      missing at least one call.  Back-patch to 9.0 (all supported versions).
      5976097c
  16. Jul 21, 2015
  17. May 15, 2015
    • Alvaro Herrera's avatar
      Move strategy numbers to include/access/stratnum.h · 26df7066
      Alvaro Herrera authored
      For upcoming BRIN opclasses, it's convenient to have strategy numbers
      defined in a single place.  Since there's nothing appropriate, create
      it.  The StrategyNumber typedef now lives there, as well as existing
      strategy numbers for B-trees (from skey.h) and R-tree-and-friends (from
      gist.h).  skey.h is forced to include stratnum.h because of the
      StrategyNumber typedef, but gist.h is not; extensions that currently
      rely on gist.h for rtree strategy numbers might need to add a new
      
      A few .c files can stop including skey.h and/or gist.h, which is a nice
      side benefit.
      
      Per discussion:
      https://www.postgresql.org/message-id/20150514232132.GZ2523@alvh.no-ip.org
      
      Authored by Emre Hasegeli and Álvaro.
      
      (It's not clear to me why bootscanner.l has any #include lines at all.)
      26df7066
  18. Mar 25, 2015
    • Andres Freund's avatar
      Centralize definition of integer limits. · 83ff1618
      Andres Freund authored
      Several submitted and even committed patches have run into the problem
      that C89, our baseline, does not provide minimum/maximum values for
      various integer datatypes. C99's stdint.h does, but we can't rely on
      it.
      
      Several parts of the code defined limits locally, so instead centralize
      the definitions to c.h.
      
      This patch also changes the more obvious usages of literal limit values;
      there's more places that could be changed, but it's less clear whether
      it's beneficial to change those.
      
      Author: Andrew Gierth
      Discussion: 87619tc5wc.fsf@news-spur.riddles.org.uk
      83ff1618
  19. Mar 16, 2015
    • Tom Lane's avatar
      Replace insertion sort in contrib/intarray with qsort(). · 8d1f2390
      Tom Lane authored
      It's all very well to claim that a simplistic sort is fast in easy
      cases, but O(N^2) in the worst case is not good ... especially if the
      worst case is as easy to hit as "descending order input".  Replace that
      bit with our standard qsort.
      
      Per bug #12866 from Maksym Boguk.  Back-patch to all active branches.
      8d1f2390
  20. Feb 20, 2015
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in a bunch more places. · 09d8d110
      Tom Lane authored
      Replace some bogus "x[1]" declarations with "x[FLEXIBLE_ARRAY_MEMBER]".
      Aside from being more self-documenting, this should help prevent bogus
      warnings from static code analyzers and perhaps compiler misoptimizations.
      
      This patch is just a down payment on eliminating the whole problem, but
      it gets rid of a lot of easy-to-fix cases.
      
      Note that the main problem with doing this is that one must no longer rely
      on computing sizeof(the containing struct), since the result would be
      compiler-dependent.  Instead use offsetof(struct, lastfield).  Autoconf
      also warns against spelling that offsetof(struct, lastfield[0]).
      
      Michael Paquier, review and additional fixes by me.
      09d8d110
  21. Feb 16, 2015
  22. Aug 25, 2014
  23. Jul 14, 2014
  24. Jul 10, 2014
  25. May 06, 2014
    • Bruce Momjian's avatar
      pgindent run for 9.4 · 0a783200
      Bruce Momjian authored
      This includes removing tabs after periods in C comments, which was
      applied to back branches, so this change should not effect backpatching.
      0a783200
  26. Apr 18, 2014
    • Peter Eisentraut's avatar
      Create function prototype as part of PG_FUNCTION_INFO_V1 macro · e7128e8d
      Peter Eisentraut authored
      Because of gcc -Wmissing-prototypes, all functions in dynamically
      loadable modules must have a separate prototype declaration.  This is
      meant to detect global functions that are not declared in header files,
      but in cases where the function is called via dfmgr, this is redundant.
      Besides filling up space with boilerplate, this is a frequent source of
      compiler warnings in extension modules.
      
      We can fix that by creating the function prototype as part of the
      PG_FUNCTION_INFO_V1 macro, which such modules have to use anyway.  That
      makes the code of modules cleaner, because there is one less place where
      the entry points have to be listed, and creates an additional check that
      functions have the right prototype.
      
      Remove now redundant prototypes from contrib and other modules.
      e7128e8d
  27. Feb 17, 2014
    • Noah Misch's avatar
      Predict integer overflow to avoid buffer overruns. · 31400a67
      Noah Misch authored
      Several functions, mostly type input functions, calculated an allocation
      size such that the calculation wrapped to a small positive value when
      arguments implied a sufficiently-large requirement.  Writes past the end
      of the inadvertent small allocation followed shortly thereafter.
      Coverity identified the path_in() vulnerability; code inspection led to
      the rest.  In passing, add check_stack_depth() to prevent stack overflow
      in related functions.
      
      Back-patch to 8.4 (all supported versions).  The non-comment hstore
      changes touch code that did not exist in 8.4, so that part stops at 9.0.
      
      Noah Misch and Heikki Linnakangas, reviewed by Tom Lane.
      
      Security: CVE-2014-0064
      31400a67
  28. Nov 10, 2013
  29. Sep 07, 2013
  30. Jul 16, 2012
    • Peter Eisentraut's avatar
      Remove unreachable code · dd16f948
      Peter Eisentraut authored
      The Solaris Studio compiler warns about these instances, unlike more
      mainstream compilers such as gcc.  But manual inspection showed that
      the code is clearly not reachable, and we hope no worthy compiler will
      complain about removing this code.
      dd16f948
  31. Jul 05, 2012
  32. Jun 25, 2012
    • Peter Eisentraut's avatar
      Replace int2/int4 in C code with int16/int32 · b8b2e3b2
      Peter Eisentraut authored
      The latter was already the dominant use, and it's preferable because
      in C the convention is that intXX means XX bits.  Therefore, allowing
      mixed use of int2, int4, int8, int16, int32 is obviously confusing.
      
      Remove the typedefs for int2 and int4 for now.  They don't seem to be
      widely used outside of the PostgreSQL source tree, and the few uses
      can probably be cleaned up by the time this ships.
      b8b2e3b2
  33. Feb 28, 2012
  34. Feb 17, 2012
    • Tom Lane's avatar
      Fix longstanding error in contrib/intarray's int[] & int[] operator. · 06d9afa6
      Tom Lane authored
      The array intersection code would give wrong results if the first entry of
      the correct output array would be "1".  (I think only this value could be
      at risk, since the previous word would always be a lower-bound entry with
      that fixed value.)
      
      Problem spotted by Julien Rouhaud, initial patch by Guillaume Lelarge,
      cosmetic improvements by me.
      06d9afa6
  35. Oct 12, 2011
    • Tom Lane's avatar
      Throw a useful error message if an extension script file is fed to psql. · 458857cc
      Tom Lane authored
      We have seen one too many reports of people trying to use 9.1 extension
      files in the old-fashioned way of sourcing them in psql.  Not only does
      that usually not work (due to failure to substitute for MODULE_PATHNAME
      and/or @extschema@), but if it did work they'd get a collection of loose
      objects not an extension.  To prevent this, insert an \echo ... \quit
      line that prints a suitable error message into each extension script file,
      and teach commands/extension.c to ignore lines starting with \echo.
      That should not only prevent any adverse consequences of loading a script
      file the wrong way, but make it crystal clear to users that they need to
      do it differently now.
      
      Tom Lane, following an idea of Andrew Dunstan's.  Back-patch into 9.1
      ... there is not going to be much value in this if we wait till 9.2.
      458857cc
  36. Sep 11, 2011
    • Peter Eisentraut's avatar
      Remove many -Wcast-qual warnings · 1b81c2fe
      Peter Eisentraut authored
      This addresses only those cases that are easy to fix by adding or
      moving a const qualifier or removing an unnecessary cast.  There are
      many more complicated cases remaining.
      1b81c2fe
  37. Sep 01, 2011
Loading