1. 18 Jul, 2018 1 commit
  2. 17 Jul, 2018 4 commits
    • Michael Paquier's avatar
      Rework error messages around file handling · 811b6e36
      Michael Paquier authored
      Some error messages related to file handling are using the code path
      context to define their state.  For example, 2PC-related errors are
      referring to "two-phase status files", or "relation mapping file" is
      used for catalog-to-filenode mapping, however those prove to be
      difficult to translate, and are not more helpful than just referring to
      the path of the file being worked on.  So simplify all those error
      messages by just referring to files with their path used.  In some
      cases, like the manipulation of WAL segments, the context is actually
      helpful so those are kept.
      
      Calls to the system function read() have also been rather inconsistent
      with their error handling sometimes not reporting the number of bytes
      read, and some other code paths trying to use an errno which has not
      been set.  The in-core functions are using a more consistent pattern
      with this patch, which checks for both errno if set or if an
      inconsistent read is happening.
      
      So as to care about pluralization when reading an unexpected number of
      byte(s), "could not read: read %d of %zu" is used as error message, with
      %d field being the output result of read() and %zu the expected size.
      This simplifies the work of translators with less variations of the same
      message.
      
      Author: Michael Paquier
      Reviewed-by: Álvaro Herrera
      Discussion: https://postgr.es/m/20180520000522.GB1603@paquier.xyz
      811b6e36
    • Alvaro Herrera's avatar
      doc: move PARTITION OF stanza to just below PARTITION BY · c6736ff7
      Alvaro Herrera authored
      It's more logical this way, since the new ordering matches the way the
      tables are created; but in any case, the previous location of PARTITION OF
      did not appear carefully chosen anyway (since it didn't match the
      location in which it appears in the synopsys either, which is what we
      normally do.)
      
      In the PARTITION BY stanza, add a link to the partitioning section in
      the DDL chapter, too.
      
      Suggested-by: David G. Johnston
      Discussion: https://postgr.es/m/CAKFQuwY4Ld7ecxL_KAmaxwt0FUu5VcPPN2L4dh+3BeYbrdBa5g@mail.gmail.com
      c6736ff7
    • Alvaro Herrera's avatar
      Revise BuildIndexValueDescription to simplify it · 1c04d4be
      Alvaro Herrera authored
      Getting a pg_index tuple from syscache when the open index relation is
      available is pointless -- just use the one from relcache.
      
      Noticed while reviewing code for cb9db2ab.
      
      No backpatch.
      1c04d4be
    • Alvaro Herrera's avatar
      Fix ALTER TABLE...SET STATS error message for included columns · cb9db2ab
      Alvaro Herrera authored
      The existing error message was complaining that the column is not an
      expression, which is not correct.  Introduce a suitable wording
      variation and a test.
      Co-authored-by: default avatarYugo Nagata <nagata@sraoss.co.jp>
      Discussion: https://postgr.es/m/20180628182803.e4632d5a.nagata@sraoss.co.jpReviewed-by: default avatarÁlvaro Herrera <alvherre@alvh.no-ip.org>
      cb9db2ab
  3. 16 Jul, 2018 4 commits
  4. 14 Jul, 2018 1 commit
    • Tom Lane's avatar
      Fix hashjoin costing mistake introduced with inner_unique optimization. · 1007b0a1
      Tom Lane authored
      In final_cost_hashjoin(), commit 9c7f5229 allowed inner_unique cases
      to follow a code path previously used only for SEMI/ANTI joins; but it
      neglected to fix an if-test within that path that assumed SEMI and ANTI
      were the only possible cases.  This resulted in a wrong value for
      hashjointuples, and an ensuing bad cost estimate, for inner_unique normal
      joins.  Fortunately, for inner_unique normal joins we can assume the number
      of joined tuples is the same as for a SEMI join; so there's no need for
      more code, we just have to invert the test to check for ANTI not SEMI.
      
      It turns out that in two contrib tests in which commit 9c7f5229
      changed the plan expected for a query, the change was actually wrong
      and induced by this estimation error, not by any real improvement.
      Hence this patch also reverts those changes.
      
      Per report from RK Korlapati.  Backpatch to v10 where the error was
      introduced.
      
      David Rowley
      
      Discussion: https://postgr.es/m/CA+SNy03bhq0fodsfOkeWDCreNjJVjsdHwUsb7AG=jpe0PtZc_g@mail.gmail.com
      1007b0a1
  5. 13 Jul, 2018 12 commits
  6. 12 Jul, 2018 12 commits
  7. 11 Jul, 2018 6 commits
    • Tom Lane's avatar
      Mark built-in btree comparison functions as leakproof where it's safe. · 39a96512
      Tom Lane authored
      Generally, if the comparison operators for a datatype or pair of datatypes
      are leakproof, the corresponding btree comparison support function can be
      considered so as well.  But we had not originally worried about marking
      support functions as leakproof, reasoning that they'd not likely be used in
      queries so the marking wouldn't matter.  It turns out there's at least one
      place where it does matter: calc_arraycontsel() finds the target datatype's
      default btree comparison function and tries to use that to estimate
      selectivity, but it will be blocked in some cases if the function isn't
      leakproof.  This leads to unnecessarily poor selectivity estimates and bad
      plans, as seen in bug #15251.
      
      Hence, run around and apply proleakproof markings where the corresponding
      btree comparison operators are leakproof.  (I did eyeball each function
      to verify that it wasn't doing anything surprising, too.)
      
      This isn't a full solution to bug #15251, and it's not back-patchable
      because of the need for a catversion bump.  A more useful response probably
      is to consider whether we can check permissions on the parent table instead
      of the child.  However, this change will help in some cases where that
      won't, and it's easy enough to do in HEAD, so let's do so.
      
      Discussion: https://postgr.es/m/3876.1531261875@sss.pgh.pa.us
      39a96512
    • Tom Lane's avatar
      Fix create_scan_plan's handling of sortgrouprefs for physical tlists. · 57cd2b6e
      Tom Lane authored
      We should only run apply_pathtarget_labeling_to_tlist if CP_LABEL_TLIST
      was specified, because only in that case has use_physical_tlist checked
      that the labeling will succeed; otherwise we may get an "ORDER/GROUP BY
      expression not found in targetlist" error.  (This subsumes the previous
      test about gating_clauses, because we reset "flags" to zero earlier
      if there are gating clauses to apply.)
      
      The only known case in which a failure can occur is with a ProjectSet
      path directly atop a table scan path, although it seems likely that there
      are other cases or will be such in future.  This means that the failure
      is currently only visible in the v10 branch: 9.6 didn't have ProjectSet,
      while in v11 and HEAD, apply_scanjoin_target_to_paths for some weird
      reason is using create_projection_path not apply_projection_to_path,
      masking the problem because there's a ProjectionPath in between.
      
      Nonetheless this code is clearly wrong on its own terms, so back-patch
      to 9.6 where this logic was introduced.
      
      Per report from Regina Obe.
      
      Discussion: https://postgr.es/m/001501d40f88$75186950$5f493bf0$@pcorp.us
      57cd2b6e
    • Tom Lane's avatar
      Fix bugs with degenerate window ORDER BY clauses in GROUPS/RANGE mode. · ff4f8891
      Tom Lane authored
      nodeWindowAgg.c failed to cope with the possibility that no ordering
      columns are defined in the window frame for GROUPS mode or RANGE OFFSET
      mode, leading to assertion failures or odd errors, as reported by Masahiko
      Sawada and Lukas Eder.  In RANGE OFFSET mode, an ordering column is really
      required, so add an Assert about that.  In GROUPS mode, the code would
      work, except that the node initialization code wasn't in sync with the
      execution code about when to set up tuplestore read pointers and spare
      slots.  Fix the latter for consistency's sake (even though I think the
      changes described below make the out-of-sync cases unreachable for now).
      
      Per SQL spec, a single ordering column is required for RANGE OFFSET mode,
      and at least one ordering column is required for GROUPS mode.  The parser
      enforced the former but not the latter; add a check for that.
      
      We were able to reach the no-ordering-column cases even with fully spec
      compliant queries, though, because the planner would drop partitioning
      and ordering columns from the generated plan if they were redundant with
      earlier columns according to the redundant-pathkey logic, for instance
      "PARTITION BY x ORDER BY y" in the presence of a "WHERE x=y" qual.
      While in principle that's an optimization that could save some pointless
      comparisons at runtime, it seems unlikely to be meaningful in the real
      world.  I think this behavior was not so much an intentional optimization
      as a side-effect of an ancient decision to construct the plan node's
      ordering-column info by reverse-engineering the PathKeys of the input
      path.  If we give up redundant-column removal then it takes very little
      code to generate the plan node info directly from the WindowClause,
      ensuring that we have the expected number of ordering columns in all
      cases.  (If anyone does complain about this, the planner could perhaps
      be taught to remove redundant columns only when it's safe to do so,
      ie *not* in RANGE OFFSET mode.  But I doubt anyone ever will.)
      
      With these changes, the WindowAggPath.winpathkeys field is not used for
      anything anymore, so remove it.
      
      The test cases added here are not actually very interesting given the
      removal of the redundant-column-removal logic, but they would represent
      important corner cases if anyone ever tries to put that back.
      
      Tom Lane and Masahiko Sawada.  Back-patch to v11 where RANGE OFFSET
      and GROUPS modes were added.
      
      Discussion: https://postgr.es/m/CAD21AoDrWqycq-w_+Bx1cjc+YUhZ11XTj9rfxNiNDojjBx8Fjw@mail.gmail.com
      Discussion: https://postgr.es/m/153086788677.17476.8002640580496698831@wrigleys.postgresql.org
      ff4f8891
    • Alexander Korotkov's avatar
      Fix more wrong paths in header comments · edf59c40
      Alexander Korotkov authored
      It appears that there are more files, whose header comment paths are
      wrong.  So, fix those paths.  No backpatching per proposal of Tom Lane.
      
      Discussion: https://postgr.es/m/CAPpHfdsJyYbOj59MOQL%2B4XxdcomLSLfLqBtAvwR%2BpsCqj3ELdQ%40mail.gmail.com
      edf59c40
    • Alvaro Herrera's avatar
      Rethink how to get float.h in old Windows API for isnan/isinf · f2c58706
      Alvaro Herrera authored
      We include <float.h> in every place that needs isnan(), because MSVC
      used to require it.  However, since MSVC 2013 that's no longer necessary
      (cf. commit cec8394b), so we can retire the inclusion to a
      version-specific stanza in win32_port.h, where it doesn't need to
      pollute random .c files.  The header is of course still needed in a few
      places for other reasons.
      
      I (Álvaro) removed float.h from a few more files than in Emre's original
      patch.  This doesn't break the build in my system, but we'll see what
      the buildfarm has to say about it all.
      
      Author: Emre Hasegeli
      Discussion: https://postgr.es/m/CAE2gYzyc0+5uG+Cd9-BSL7NKC8LSHLNg1Aq2=8ubjnUwut4_iw@mail.gmail.com
      f2c58706
    • Alexander Korotkov's avatar
      Fix wrong file path in header comment · a01d0fa1
      Alexander Korotkov authored
      Header comment of shm_mq.c was mistakenly specifying path to shm_mq.h.
      It was introduced in ec9037df.  So, theoretically it could be
      backpatched to 9.4, but it doesn't seem to worth it.
      a01d0fa1