1. 21 Feb, 2015 4 commits
    • Tom Lane's avatar
      Minor code beautification in conninfo_uri_parse_params(). · 3d9b6f31
      Tom Lane authored
      Reading this made me itch, so clean the logic a bit.
      3d9b6f31
    • Tom Lane's avatar
      Fix misparsing of empty value in conninfo_uri_parse_params(). · b26e2081
      Tom Lane authored
      After finding an "=" character, the pointer was advanced twice when it
      should only advance once.  This is harmless as long as the value after "="
      has at least one character; but if it doesn't, we'd miss the terminator
      character and include too much in the value.
      
      In principle this could lead to reading off the end of memory.  It does not
      seem worth treating as a security issue though, because it would happen on
      client side, and besides client logic that's taking conninfo strings from
      untrusted sources has much worse security problems than this.
      
      Report and patch received off-list from Thomas Fanghaenel.
      Back-patch to 9.2 where the faulty code was introduced.
      b26e2081
    • Robert Haas's avatar
      Don't require users of src/port/gettimeofday.c to initialize it. · 64235fec
      Robert Haas authored
      Commit 8001fe67 introduced this
      requirement, but per discussion, we want to avoid requirements of
      this type to make things easier on the calling code.  An especially
      important consideration is that this may be used in frontend code,
      not just the backend.
      
      Asif Naeem, reviewed by Michael Paquier
      64235fec
    • Tom Lane's avatar
      Some more FLEXIBLE_ARRAY_MEMBER fixes. · f2874feb
      Tom Lane authored
      f2874feb
  2. 20 Feb, 2015 12 commits
    • Tom Lane's avatar
      Fix statically allocated struct with FLEXIBLE_ARRAY_MEMBER member. · 33b2a2c9
      Tom Lane authored
      clang complains about this, not unreasonably, so define another struct
      that's explicitly for a WordEntryPos with exactly one element.
      
      While at it, get rid of pretty dubious use of a static variable for
      more than one purpose --- if it were being treated as const maybe
      I'd be okay with this, but it isn't.
      33b2a2c9
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in some more places. · 33a3b03d
      Tom Lane authored
      Fix a batch of structs that are only visible within individual .c files.
      
      Michael Paquier
      33a3b03d
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in struct RecordIOData. · c110eff1
      Tom Lane authored
      I (tgl) fixed this last night in rowtypes.c, but I missed that the
      code had been copied into a couple of other places.
      
      Michael Paquier
      c110eff1
    • Tom Lane's avatar
      Use FLEXIBLE_ARRAY_MEMBER in struct varlena. · e38b1eb0
      Tom Lane authored
      This forces some minor coding adjustments in tuptoaster.c and inv_api.c,
      but the new coding there is cleaner anyway.
      
      Michael Paquier
      e38b1eb0
    • Alvaro Herrera's avatar
      Remove unnecessary and unreliable test · 8902f792
      Alvaro Herrera authored
      8902f792
    • Alvaro Herrera's avatar
      Update PGSTAT_FILE_FORMAT_ID · 3b14bb77
      Alvaro Herrera authored
      Previous commit should have bumped it but didn't.  Oops.
      
      Per note from Tom.
      3b14bb77
    • Alvaro Herrera's avatar
      Have TRUNCATE update pgstat tuple counters · d42358ef
      Alvaro Herrera authored
      This works by keeping a per-subtransaction record of the ins/upd/del
      counters before the truncate, and then resetting them; this record is
      useful to return to the previous state in case the truncate is rolled
      back, either in a subtransaction or whole transaction.  The state is
      propagated upwards as subtransactions commit.
      
      When the per-table data is sent to the stats collector, a flag indicates
      to reset the live/dead counters to zero as well.
      
      Catalog version bumped due to the change in pgstat format.
      
      Author: Alexander Shulgin
      Discussion: 1007.1207238291@sss.pgh.pa.us
      Discussion: 548F7D38.2000401@BlueTreble.com
      Reviewed-by: Álvaro Herrera, Jim Nasby
      d42358ef
    • Tom Lane's avatar
      Some more FLEXIBLE_ARRAY_MEMBER hacking. · 5740be6d
      Tom Lane authored
      5740be6d
    • Tom Lane's avatar
      Remove unused variable. · 9aa53bbd
      Tom Lane authored
      Per buildfarm.
      9aa53bbd
    • Tom Lane's avatar
      Use "#ifdef CATALOG_VARLEN" to protect nullable fields of pg_authid. · 692bd09a
      Tom Lane authored
      This gives a stronger guarantee than a mere comment against accessing these
      fields as simple struct members.  Since rolpassword is in fact varlena,
      it's not clear why these didn't get marked from the beginning, but let's
      do it now.
      
      Michael Paquier
      692bd09a
    • 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
    • Tom Lane's avatar
      Add pg_stat_get_snapshot_timestamp() to show statistics snapshot timestamp. · 2fb7a75f
      Tom Lane authored
      Per discussion, this could be useful for purposes such as programmatically
      detecting a nonresponding stats collector.  We already have the timestamp
      anyway, it's just a matter of providing a SQL-accessible function to fetch
      it.
      
      Matt Kelly, reviewed by Jim Nasby
      2fb7a75f
  3. 19 Feb, 2015 4 commits
    • Heikki Linnakangas's avatar
      Remove dead structs. · 634618ec
      Heikki Linnakangas authored
      These are not used with the new WAL format anymore. GIN split records are
      simply always recorded as full-page images.
      
      Michael Paquier
      634618ec
    • Tom Lane's avatar
      Update assorted TOAST-related documentation. · 9bb955c8
      Tom Lane authored
      While working on documentation for expanded arrays, I noticed a number of
      details in the TOAST-related documentation that were already inaccurate or
      obsolete.  This should be fixed independently of whether expanded arrays
      get in or not.  One issue is that the already existing indirect-pointer
      facility was not documented at all.  Also, the documentation says that you
      only need to use VARSIZE/SET_VARSIZE if you've made your variable-length
      type TOAST-aware, but actually we've forced that business on all varlena
      types even if they've opted out of TOAST by setting storage = plain.
      Wordsmith a few other things too, like an amusingly archaic claim that
      there are few 64-bit machines.
      
      I thought about back-patching this, but since all this doco is oriented
      to hackers and C-coded extension authors, fixing it in HEAD is probably
      good enough.
      9bb955c8
    • Tom Lane's avatar
      Split array_push into separate array_append and array_prepend functions. · 56a79a86
      Tom Lane authored
      There wasn't any good reason for a single C function to implement both
      these SQL functions: it saved very little code overall, and it required
      significant pushups to re-determine at runtime which case applied.  Redoing
      it as two functions ends up with just slightly more lines of code, but it's
      simpler to understand, and faster too because we need not repeat syscache
      lookups on every call.
      
      An important side benefit is that this eliminates the only case in which
      different aliases of the same C function had both anyarray and anyelement
      arguments at the same position, which would almost always be a mistake.
      The opr_sanity regression test will now notice such mistakes since there's
      no longer a valid case where it happens.
      56a79a86
    • Peter Eisentraut's avatar
      Fix Perl coding error in msvc build system · d30292b8
      Peter Eisentraut authored
      Code like
      
          open(P, "cl /? 2>&1 |") || die "cl command not found";
      
      does not actually catch any errors, because the exit status of the
      command before the pipe is ignored.  The fix is to look at $?.
      
      This also gave the opportunity to clean up the logic of this code a bit.
      d30292b8
  4. 18 Feb, 2015 4 commits
    • Alvaro Herrera's avatar
      Fix opclass/opfamily identity strings · 9c7dd350
      Alvaro Herrera authored
      The original representation uses "opcname for amname", which is good
      enough; but if we replace "for" with "using", we can apply the returned
      identity directly in a DROP command, as in
      
      DROP OPERATOR CLASS opcname USING amname
      
      This slightly simplifies code using object identities to programatically
      execute commands on these kinds of objects.
      
      Note backwards-incompatible change:
      The previous representation dates back to 9.3 when object identities
      were introduced by commit f8348ea3, but we don't want to change the
      behavior on released branches unnecessarily and so this is not
      backpatched.
      9c7dd350
    • Alvaro Herrera's avatar
      Fix object identities for pg_conversion objects · 0d906798
      Alvaro Herrera authored
      We were neglecting to schema-qualify them.
      
      Backpatch to 9.3, where object identities were introduced as a concept
      by commit f8348ea3.
      0d906798
    • Tom Lane's avatar
      Fix placement of "SET row_security" command issuance in pg_dump. · 297b2c1e
      Tom Lane authored
      Somebody apparently threw darts at the code to decide where to insert
      these.  They certainly didn't proceed by adding them where other similar
      SETs were handled.  This at least broke pg_restore, and perhaps other
      use-cases too.
      297b2c1e
    • Tom Lane's avatar
      Fix failure to honor -Z compression level option in pg_dump -Fd. · 0e7e355f
      Tom Lane authored
      cfopen() and cfopen_write() failed to pass the compression level through
      to zlib, so that you always got the default compression level if you got
      any at all.
      
      In passing, also fix these and related functions so that the correct errno
      is reliably returned on failure; the original coding supposes that free()
      cannot change errno, which is untrue on at least some platforms.
      
      Per bug #12779 from Christoph Berg.  Back-patch to 9.1 where the faulty
      code was introduced.
      
      Michael Paquier
      0e7e355f
  5. 17 Feb, 2015 5 commits
    • Tom Lane's avatar
      Fix EXPLAIN output for cases where parent table is excluded by constraints. · abe45a9b
      Tom Lane authored
      The previous coding in EXPLAIN always labeled a ModifyTable node with the
      name of the target table affected by its first child plan.  When originally
      written, this was necessarily the parent table of the inheritance tree,
      so everything was unconfusing.  But when we added NO INHERIT constraints,
      it became possible for the parent table to be deleted from the plan by
      constraint exclusion while still leaving child tables present.  This led to
      the ModifyTable plan node being labeled with the first surviving child,
      which was deemed confusing.  Fix it by retaining the parent table's RT
      index in a new field in ModifyTable.
      
      Etsuro Fujita, reviewed by Ashutosh Bapat and myself
      abe45a9b
    • Heikki Linnakangas's avatar
      Fix a bug in pairing heap removal code. · 931bf3eb
      Heikki Linnakangas authored
      After removal, the next_sibling pointer of a node was sometimes incorrectly
      left to point to another node in the heap, which meant that a node was
      sometimes linked twice into the heap. Surprisingly that didn't cause any
      crashes in my testing, but it was clearly wrong and could easily segfault
      in other scenarios.
      
      Also always keep the prev_or_parent pointer as NULL on the root node. That
      was not a correctness issue AFAICS, but let's be tidy.
      
      Add a debugging function, to dump the contents of a pairing heap as a
      string. It's #ifdef'd out, as it's not used for anything in any normal
      code, but it was highly useful in debugging this. Let's keep it handy for
      further reference.
      931bf3eb
    • Heikki Linnakangas's avatar
      Fix knn-GiST queue comparison function to return heap tuples first. · d17b6df2
      Heikki Linnakangas authored
      The part of the comparison function that was supposed to keep heap tuples
      ahead of index items was backwards. It would not lead to incorrect results,
      but it is more efficient to return heap tuples first, before scanning more
      index pages, when both have the same distance.
      
      Alexander Korotkov
      d17b6df2
    • Tom Lane's avatar
      Remove code to match IPv4 pg_hba.conf entries to IPv4-in-IPv6 addresses. · 2e105def
      Tom Lane authored
      In investigating yesterday's crash report from Hugo Osvaldo Barrera, I only
      looked back as far as commit f3aec2c7 where the breakage occurred
      (which is why I thought the IPv4-in-IPv6 business was undocumented).  But
      actually the logic dates back to commit 3c9bb888 and was simply
      broken by erroneous refactoring in the later commit.  A bit of archives
      excavation shows that we added the whole business in response to a report
      that some 2003-era Linux kernels would report IPv4 connections as having
      IPv4-in-IPv6 addresses.  The fact that we've had no complaints since 9.0
      seems to be sufficient confirmation that no modern kernels do that, so
      let's just rip it all out rather than trying to fix it.
      
      Do this in the back branches too, thus essentially deciding that our
      effective behavior since 9.0 is correct.  If there are any platforms on
      which the kernel reports IPv4-in-IPv6 addresses as such, yesterday's fix
      would have made for a subtle and potentially security-sensitive change in
      the effective meaning of IPv4 pg_hba.conf entries, which does not seem like
      a good thing to do in minor releases.  So let's let the post-9.0 behavior
      stand, and change the documentation to match it.
      
      In passing, I failed to resist the temptation to wordsmith the description
      of pg_hba.conf IPv4 and IPv6 address entries a bit.  A lot of this text
      hasn't been touched since we were IPv4-only.
      2e105def
    • Robert Haas's avatar
      Improve pg_check_dir code and comments. · 5d6c2405
      Robert Haas authored
      Avoid losing errno if readdir() fails and closedir() works.  Consistently
      return 4 rather than 3 if both a lost+found directory and other files are
      found, rather than returning one value or the other depending on the
      order of the directory listing.  Update comments to match the actual
      behavior.
      
      These oversights date to commits 6f03927f
      and 17f15239.
      
      Marco Nenciarini
      5d6c2405
  6. 16 Feb, 2015 9 commits
    • Kevin Grittner's avatar
      Eliminate unnecessary NULL checks in picksplit method of intarray. · c923e82a
      Kevin Grittner authored
      Where these checks were being done there was no code path which
      could leave them NULL.
      
      Michael Paquier per Coverity
      c923e82a
    • Tom Lane's avatar
      Fix misuse of memcpy() in check_ip(). · cb66f495
      Tom Lane authored
      The previous coding copied garbage into a local variable, pretty much
      ensuring that the intended test of an IPv6 connection address against a
      promoted IPv4 address from pg_hba.conf would never match.  The lack of
      field complaints likely indicates that nobody realized this was supposed
      to work, which is unsurprising considering that no user-facing docs suggest
      it should work.
      
      In principle this could have led to a SIGSEGV due to reading off the end of
      memory, but since the source address would have pointed to somewhere in the
      function's stack frame, that's quite unlikely.  What led to discovery of
      the bug is Hugo Osvaldo Barrera's report of a crash after an OS upgrade,
      which is probably because he is now running a system in which memcpy raises
      abort() upon detecting overlapping source and destination areas.  (You'd
      have to additionally suppose some things about the stack frame layout to
      arrive at this conclusion, but it seems plausible.)
      
      This has been broken since the code was added, in commit f3aec2c7,
      so back-patch to all supported branches.
      cb66f495
    • Heikki Linnakangas's avatar
      Fix comment in libpq OpenSSL code about why a substitue BIO is used. · c478959a
      Heikki Linnakangas authored
      The comment was copy-pasted from the backend code along with the
      implementation, but libpq has different reasons for using the BIO.
      c478959a
    • Heikki Linnakangas's avatar
      Restore the SSL_set_session_id_context() call to OpenSSL renegotiation. · 1c2b7c08
      Heikki Linnakangas authored
      This reverts the removal of the call in commit (272923a0). It turns out it
      wasn't superfluous after all: without it, renegotiation fails if a client
      certificate was used. The rest of the changes in that commit are still OK
      and not reverted.
      
      Per investigation of bug #12769 by Arne Scheffer, although this doesn't fix
      the reported bug yet.
      1c2b7c08
    • Tom Lane's avatar
      Use fast path in plpgsql's RETURN/RETURN NEXT in more cases. · 9e3ad1aa
      Tom Lane authored
      exec_stmt_return() and exec_stmt_return_next() have fast-path code for
      handling a simple variable reference (i.e. "return var") without going
      through the full expression evaluation machinery.  For some reason,
      pl_gram.y was under the impression that this fast path only applied for
      record/row variables; but in reality code for handling regular scalar
      variables has been there all along.  Adjusting the logic to allow that
      code to be used actually results in a net savings of code in pl_gram.y
      (by eliminating some redundancy), and it buys a measurable though not
      very impressive amount of speedup.
      
      Noted while fooling with my expanded-array patch, wherein this makes a much
      bigger difference because it enables returning an expanded array variable
      without an extra flattening step.  But AFAICS this is a win regardless,
      so commit it separately.
      9e3ad1aa
    • Heikki Linnakangas's avatar
      In the SSL test suite, use a root CA cert that won't expire (so quickly) · 2c75531a
      Heikki Linnakangas authored
      All the other certificates were created to be valid for 10000 days, because
      we don't want to have to recreate them. But I missed the root CA cert, and
      the pre-created certificates included in the repository expired in January.
      Fix, and re-create all the certificates.
      2c75531a
    • Tom Lane's avatar
      Rationalize the APIs of array element/slice access functions. · e983c4d1
      Tom Lane authored
      The four functions array_ref, array_set, array_get_slice, array_set_slice
      have traditionally declared their array inputs and results as being of type
      "ArrayType *".  This is a lie, and has been since Berkeley days, because
      they actually also support "fixed-length array" types such as "name" and
      "point"; not to mention that the inputs could be toasted.  These values
      should be declared Datum instead to avoid confusion.  The current coding
      already risks possible misoptimization by compilers, and it'll get worse
      when "expanded" array representations become a valid alternative.
      
      However, there's a fair amount of code using array_ref and array_set with
      arrays that *are* known to be ArrayType structures, and there might be more
      such places in third-party code.  Rather than cluttering those call sites
      with PointerGetDatum/DatumGetArrayTypeP cruft, what I did was to rename the
      existing functions to array_get_element/array_set_element, fix their
      signatures, then reincarnate array_ref/array_set as backwards compatibility
      wrappers.
      
      array_get_slice/array_set_slice have no such constituency in the core code,
      and probably not in third-party code either, so I just changed their APIs.
      e983c4d1
    • Fujii Masao's avatar
      Correct the path of pg_lzcompress.c in doc. · cef30974
      Fujii Masao authored
      Commit 40bede54 moved pg_lzcompress.c to src/common, but forgot to
      update its path in doc. This commit fixes that oversight.
      cef30974
    • Tom Lane's avatar
      Fix null-pointer-deref crash while doing COPY IN with check constraints. · 08361cea
      Tom Lane authored
      In commit bf7ca158 I introduced an
      assumption that an RTE referenced by a whole-row Var must have a valid eref
      field.  This is false for RTEs constructed by DoCopy, and there are other
      places taking similar shortcuts.  Perhaps we should make all those places
      go through addRangeTableEntryForRelation or its siblings instead of having
      ad-hoc logic, but the most reliable fix seems to be to make the new code in
      ExecEvalWholeRowVar cope if there's no eref.  We can reasonably assume that
      there's no need to insert column aliases if no aliases were provided.
      
      Add a regression test case covering this, and also verifying that a sane
      column name is in fact available in this situation.
      
      Although the known case only crashes in 9.4 and HEAD, it seems prudent to
      back-patch the code change to 9.2, since all the ingredients for a similar
      failure exist in the variant patch applied to 9.3 and 9.2.
      
      Per report from Jean-Pierre Pelletier.
      08361cea
  7. 15 Feb, 2015 2 commits