1. 03 Oct, 2016 1 commit
    • Heikki Linnakangas's avatar
      Change the way pre-reading in external sort's merge phase works. · e94568ec
      Heikki Linnakangas authored
      Don't pre-read tuples into SortTuple slots during merge. Instead, use the
      memory for larger read buffers in logtape.c. We're doing the same number
      of READTUP() calls either way, but managing the pre-read SortTuple slots
      is much more complicated. Also, the on-tape representation is more compact
      than SortTuples, so we can fit more pre-read tuples into the same amount
      of memory this way. And we have better cache-locality, when we use just a
      small number of SortTuple slots.
      
      Now that we only hold one tuple from each tape in the SortTuple slots, we
      can greatly simplify the "batch memory" management. We now maintain a
      small set of fixed-sized slots, to hold the tuples, and fall back to
      palloc() for larger tuples. We use this method during all merge phases,
      not just the final merge, and also when randomAccess is requested, and
      also in the TSS_SORTEDONTAPE case. In other words, it's used whenever we
      do an external sort.
      
      Reviewed by Peter Geoghegan and Claudio Freire.
      
      Discussion: <CAM3SWZTpaORV=yQGVCG8Q4axcZ3MvF-05xe39ZvORdU9JcD6hQ@mail.gmail.com>
      e94568ec
  2. 02 Oct, 2016 2 commits
    • Tom Lane's avatar
      Add ALTER EXTENSION ADD/DROP ACCESS METHOD, and use it in pg_upgrade. · e8bdee27
      Tom Lane authored
      Without this, an extension containing an access method is not properly
      dumped/restored during pg_upgrade --- the AM ends up not being a member
      of the extension after upgrading.
      
      Another oversight in commit 473b9328, reported by Andrew Dunstan.
      
      Report: <f7ac29f3-515c-2a44-21c5-ec925053265f@dunslane.net>
      e8bdee27
    • Tom Lane's avatar
      Avoid leaking FDs after an fsync failure. · 728ceba9
      Tom Lane authored
      Fixes errors introduced in commit bc34223b, as detected by Coverity.
      
      In passing, report ENOSPC for a short write while padding a new wal file in
      open_walfile, make certain that close_walfile closes walfile in all cases,
      and improve a couple of comments.
      
      Michael Paquier and Tom Lane
      728ceba9
  3. 01 Oct, 2016 8 commits
    • Tom Lane's avatar
      Do ClosePostmasterPorts() earlier in SubPostmasterMain(). · 3b90e38c
      Tom Lane authored
      In standard Unix builds, postmaster child processes do ClosePostmasterPorts
      immediately after InitPostmasterChild, that is almost immediately after
      being spawned.  This is important because we don't want children holding
      open the postmaster's end of the postmaster death watch pipe.
      
      However, in EXEC_BACKEND builds, SubPostmasterMain was postponing this
      responsibility significantly, in order to make it slightly more convenient
      to pass the right flag value to ClosePostmasterPorts.  This is bad,
      particularly seeing that process_shared_preload_libraries() might invoke
      nearly-arbitrary code.  Rearrange so that we do it as soon as we've
      fetched the socket FDs via read_backend_variables().
      
      Also move the comment explaining about randomize_va_space to before the
      call of PGSharedMemoryReAttach, which is where it's relevant.  The old
      placement was appropriate when the reattach happened inside
      CreateSharedMemoryAndSemaphores, but that was a long time ago.
      
      Back-patch to 9.3; the patch doesn't apply cleanly before that, and
      it doesn't seem worth a lot of effort given that we've had no actual
      field complaints traceable to this.
      
      Discussion: <4157.1475178360@sss.pgh.pa.us>
      3b90e38c
    • Tom Lane's avatar
      Fix bugs in contrib/pg_visibility. · 9a109452
      Tom Lane authored
      collect_corrupt_items() failed to initialize tuple.t_self.  While
      HeapTupleSatisfiesVacuum() doesn't actually use that value, it does
      Assert that it's valid, so that the code would dump core if ip_posid
      chanced to be zero.  (That's somewhat unlikely, which probably explains
      how this got missed.  In any case it wouldn't matter for field use.)
      
      Also, collect_corrupt_items was returning the wrong TIDs, that is the
      contents of t_ctid rather than the tuple's own location.  This would
      be the same thing in simple cases, but it could be wrong if, for
      example, a past update attempt had been rolled back, leaving a live
      tuple whose t_ctid doesn't point at itself.
      
      Also, in pg_visibility(), guard against trying to read a page past
      the end of the rel.  The VM code handles inquiries beyond the end
      of the map by silently returning zeroes, and it seems like we should
      do the same thing here.
      
      I ran into the assertion failure while using pg_visibility to check
      pg_upgrade's behavior, and then noted the other problems while
      reading the code.
      
      Report: <29043.1475288648@sss.pgh.pa.us>
      9a109452
    • Tom Lane's avatar
      Copy-editing for contrib/pg_visibility documentation. · 33596edf
      Tom Lane authored
      Add omitted names for some function parameters.
      Fix some minor grammatical issues.
      33596edf
    • Tom Lane's avatar
      Fix misstatement in comment in Makefile.shlib. · ea046f08
      Tom Lane authored
      There is no need for "all: all-lib" to be placed before inclusion of
      Makefile.shlib.  Makefile.global is what ensures that "all" is the
      default target, and we already document that that has to be included
      first.  Per comment from Pavel Raiskup.
      
      Discussion: <1925924.izSMJEZO3x@unused-4-107.brq.redhat.com>
      ea046f08
    • Tom Lane's avatar
      Fix misplacement of submake-generated-headers prerequisites. · 7107d58e
      Tom Lane authored
      The sequence "configure; cd src/pl/plpython; make -j" failed due to
      trying to compile plpython's .o files before the generated headers
      finished building.  (This is an important real-world case, since it's
      the typical second step when building both plpython2 and plpython3.)
      This happens because the submake-generated-headers target is not
      placed in a way to make it a prerequisite to compiling the .o files.
      Fix that.
      
      Checking other uses of submake-generated-headers, I noted that the one
      attached to pg_regress was similarly misplaced; but it's actually not
      needed at all for pg_regress.o, rather regress.o, so move it to be a
      prerequisite of that.
      
      Back-patch to 9.6 where submake-generated-headers was introduced
      (by commit 548af97f).  It's not immediately clear to me why the
      previous coding didn't have the same issue; but since we've not
      had field reports of plpython make failing, leave it alone in the
      older branches.
      
      Pavel Raiskup and Tom Lane
      
      Discussion: <1925924.izSMJEZO3x@unused-4-107.brq.redhat.com>
      7107d58e
    • Peter Eisentraut's avatar
      Set log_line_prefix and application name in test drivers · a4327296
      Peter Eisentraut authored
      Before pg_regress runs psql, set the application name to the test name.
      Similarly, set the application name to the test file name in the TAP
      tests.  Also, set a default log_line_prefix that show the application
      name, as well as the PID and a time stamp.
      
      That way, the server log output can be correlated to the test input
      files, making debugging a bit easier.
      a4327296
    • Tom Lane's avatar
      Improve error reporting in pg_upgrade's file copying/linking/rewriting. · f002ed2b
      Tom Lane authored
      The previous design for this had copyFile(), linkFile(), and
      rewriteVisibilityMap() returning strerror strings, with the caller
      producing one-size-fits-all error messages based on that.  This made it
      impossible to produce messages that described the failures with any degree
      of precision, especially not short-read problems since those don't set
      errno at all.
      
      Since pg_upgrade has no intention of continuing after any error in this
      area, let's fix this by just letting these functions call pg_fatal() for
      themselves, making it easy for each point of failure to have a suitable
      error message.  Taking this approach also allows dropping cleanup code
      that was unnecessary and was often rather sloppy about preserving errno.
      To not lose relevant info that was reported before, pass in the schema name
      and table name of the current table so that they can be included in the
      error reports.
      
      An additional problem was the use of getErrorText(), which was flat out
      wrong for all but a couple of call sites, because it unconditionally did
      "_dosmaperr(GetLastError())" on Windows.  That's only appropriate when
      reporting an error from a Windows-native API, which only a couple of
      the callers were actually doing.  Thus, even the reported strerror string
      would be unrelated to the actual failure in many cases on Windows.
      To fix, get rid of getErrorText() altogether, and just have call sites
      do strerror(errno) instead, since that's the way all the rest of our
      frontend programs do it.  Add back the _dosmaperr() calls in the two
      places where that's actually appropriate.
      
      In passing, make assorted messages hew more closely to project style
      guidelines, notably by removing initial capitals in not-complete-sentence
      primary error messages.  (I didn't make any effort to clean up places
      I didn't have another reason to touch, though.)
      
      Per discussion of a report from Thomas Kellerer.  Back-patch to 9.6,
      but no further; given the relative infrequency of reports of problems
      here, it's not clear it's worth adapting the patch to older branches.
      
      Patch by me, but with credit to Alvaro Herrera for spotting the issue
      with getErrorText's misuse of _dosmaperr().
      
      Discussion: <nsjrbh$8li$1@blaine.gmane.org>
      f002ed2b
    • Tom Lane's avatar
      Fix multiple portability issues in pg_upgrade's rewriteVisibilityMap(). · 5afcd2aa
      Tom Lane authored
      This is new code in 9.6, and evidently we missed out testing it as
      thoroughly as it should have been.  Bugs fixed here:
      
      1. Use binary not text mode to open the files on Windows.  Before, if
      the visibility map chanced to contain two bytes that looked like \r\n,
      Windows' read() would convert that to \n, which both corrupts the map
      data and causes the file to look shorter than it should.  Unless you
      were *very* unlucky and had an exact multiple of 8K such occurrences
      in each VM file, this would cause pg_upgrade to report a failure,
      though with a rather obscure error message.
      
      2. The code for copying rebuilt bytes into the output was simply wrong.
      It chanced to work okay on little-endian machines but would emit the
      bytes in the wrong order on big-endian, leading to silent corruption
      of the visibility map data.
      
      3. The code was careless about alignment of the working buffers.  Given
      all three of an alignment-picky architecture, a compiler that chooses
      to put the new_vmbuf[] local variable at an odd starting address, and
      a checksum-enabled database, pg_upgrade would dump core.
      
      Point one was reported by Thomas Kellerer, the other two detected by
      code-reading.
      
      Point two is much the nastiest of these issues from an impact standpoint,
      though fortunately it affects only a minority of users.  The Windows issue
      will definitely bite people, but it seems quite unlikely that there would
      be undetected corruption from that.
      
      In addition, I failed to resist the temptation to do some minor cosmetic
      adjustments, mostly improving the comments.
      
      It would be a good idea to try to improve the error reporting here, but
      that seems like material for a separate patch.
      
      Discussion: <nsjrbh$8li$1@blaine.gmane.org>
      5afcd2aa
  4. 30 Sep, 2016 8 commits
  5. 29 Sep, 2016 7 commits
    • Peter Eisentraut's avatar
      Fix compiler warnings · f2af8dc5
      Peter Eisentraut authored
      This was missed in bf5bb2e8, because the
      code is only visible under PG_FLUSH_DATA_WORKS.
      f2af8dc5
    • Tom Lane's avatar
      Allow contrib/file_fdw to read from a program, like COPY FROM PROGRAM. · 8e91e12b
      Tom Lane authored
      This patch just exposes COPY's FROM PROGRAM option in contrib/file_fdw.
      There don't seem to be any security issues with that that are any worse
      than what already exist with file_fdw and COPY; as in the existing cases,
      only superusers are allowed to control what gets executed.
      
      A regression test case might be nice here, but choosing a 100% portable
      command to run is hard.  (We haven't got a test for COPY FROM PROGRAM
      itself, either.)
      
      Corey Huinker and Adam Gomaa, reviewed by Amit Langote
      
      Discussion: <CADkLM=dGDGmaEiZ=UDepzumWg-CVn7r8MHPjr2NArj8S3TsROQ@mail.gmail.com>
      8e91e12b
    • Peter Eisentraut's avatar
      Switch pg_basebackup commands in Postgres.pm to use --nosync · 728a3e73
      Peter Eisentraut authored
      On slow machines, this greatly reduces the I/O pressure induced by the
      tests.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      728a3e73
    • Peter Eisentraut's avatar
      pg_basebackup: Add --nosync option · 6ed2d858
      Peter Eisentraut authored
      This is useful for testing, similar to initdb's --nosync.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      6ed2d858
    • Peter Eisentraut's avatar
      pg_basebackup pg_receivexlog: Issue fsync more carefully · bc34223b
      Peter Eisentraut authored
      Several places weren't careful about fsyncing in the way.  See 1d4a0ab1
      and 606e0f98 for details about required fsyncs.
      
      This adds a couple of functions in src/common/ that have an equivalent
      in the backend: durable_rename(), fsync_parent_path()
      
      From: Michael Paquier <michael.paquier@gmail.com>
      bc34223b
    • Peter Eisentraut's avatar
      Move fsync routines of initdb into src/common/ · bf5bb2e8
      Peter Eisentraut authored
      The intention is to used those in other utilities such as pg_basebackup
      and pg_receivexlog.
      
      From: Michael Paquier <michael.paquier@gmail.com>
      bf5bb2e8
    • Heikki Linnakangas's avatar
      Don't bother to lock bufmgr partitions in pg_buffercache. · 6e654546
      Heikki Linnakangas authored
      That makes the view a lot less disruptive to use on a production system.
      Without the locks, you don't get a consistent snapshot across all buffers,
      but that's OK. It wasn't a very useful guarantee in practice.
      
      Ivan Kartyshov, reviewed by Tomas Vondra and Robert Haas.
      
      Discusssion: <f9d6cab2-73a7-7a84-55a8-07dcb8516ae5@postgrespro.ru>
      6e654546
  6. 28 Sep, 2016 9 commits
    • Peter Eisentraut's avatar
      Exclude additional directories in pg_basebackup · 6ad8ac60
      Peter Eisentraut authored
      The list of files and directories that pg_basebackup excludes from the
      backup was somewhat incomplete and unorganized.  Change that with having
      the exclusion driven from tables.  Clean up some code around it.  Also
      document the exclusions in more detail so that users of pg_start_backup
      can make use of it as well.
      
      The contents of these directories are now excluded from the backup:
      pg_dynshmem, pg_notify, pg_serial, pg_snapshots, pg_subtrans
      
      Also fix a bug that a pg_repl_slot or pg_stat_tmp being a symlink would
      cause a corrupt tar header to be created.  Now such symlinks are
      included in the backup as empty directories.  Bug found by Ashutosh
      Sharma <ashu.coek88@gmail.com>.
      
      From: David Steele <david@pgmasters.net>
      Reviewed-by: default avatarMichael Paquier <michael.paquier@gmail.com>
      6ad8ac60
    • Alvaro Herrera's avatar
      Silence compiler warnings · b82d5a2c
      Alvaro Herrera authored
      Reported by Peter Eisentraut.  Coding suggested by Tom Lane.
      b82d5a2c
    • Tom Lane's avatar
      Rationalize format-picture caching logic in formatting.c. · 83bed06b
      Tom Lane authored
      Add a validity flag to DCHCacheEntry and NUMCacheEntry entries, and
      do not set it true until after we've parsed the supplied format string.
      This allows dealing with possible errors while parsing the format
      without the baroque hack that was there before (which only covered
      errors within NUMDesc_prepare, anyway).  We can get rid of the PG_TRY in
      NUMDesc_prepare, as well as last_NUMCacheEntry and NUM_cache_remove.
      (Essentially, this reverts commit ff783fba in favor of a less fragile
      solution; the problems with that approach are well illustrated by later
      hacking such as 55f927a4.)
      
      In passing, define the size of these caches as DCH_CACHE_ENTRIES not
      DCH_CACHE_FIELDS + 1 (whoever thought that was a good definition?)
      and likewise for the NUM cache.  Also const-ify format string parameters
      where convenient, and merge duplicated cache lookup logic.
      
      This is primarily driven by a proposed patch from Artur Zakirov,
      which introduced some ereport's into format string parsing for
      the datetime case.  He proposed preventing the creation of invalid
      cache entries by parsing the format string first into a local-variable
      array, and then copying that to a cache entry.  That seemed a bit
      ugly to me, and anyway randomly different from the way the identical
      problem had been solved for the numeric case.  Let's make the two
      sets of code more similar not less so.
      
      I'm not sure whether we'll adopt the new error conditions Artur proposes,
      but this patch seems like good code cleanup and future-proofing in any
      case.  The existing code is critically (and undocumented-ly) dependent on
      no elog being thrown out of several nontrivial functions, which is trouble
      waiting to happen, though it doesn't seem to be actively broken today.
      
      Discussion: <b2a39359-3282-b402-f4a3-057aae500ee7@postgrespro.ru>
      83bed06b
    • Tom Lane's avatar
      Make to_timestamp() and to_date() range-check fields of their input. · d3cd36a1
      Tom Lane authored
      Historically, something like to_date('2009-06-40','YYYY-MM-DD') would
      return '2009-07-10' because there was no prohibition on out-of-range
      month or day numbers.  This has been widely panned, and it also turns
      out that Oracle throws an error in such cases.  Since these functions
      are nominally Oracle-compatibility features, let's change that.
      
      There's no particular restriction on year (modulo the fact that the
      scanner may not believe that more than 4 digits are year digits,
      a matter to be addressed separately if at all).  But we now check month,
      day, hour, minute, second, and fractional-second fields, as well as
      day-of-year and second-of-day fields if those are used.
      
      Currently, no checks are made on ISO-8601-style week numbers or day
      numbers; it's not very clear what the appropriate rules would be there,
      and they're probably so little used that it's not worth sweating over.
      
      Artur Zakirov, reviewed by Amul Sul, further adjustments by me
      
      Discussion: <1873520224.1784572.1465833145330.JavaMail.yahoo@mail.yahoo.com>
      See-Also: <57786490.9010201@wars-nicht.de>
      d3cd36a1
    • Peter Eisentraut's avatar
      Remove dead line of code · 967ed920
      Peter Eisentraut authored
      967ed920
    • Robert Haas's avatar
      worker_spi: Call pgstat_report_stat. · 4929704a
      Robert Haas authored
      Without this, statistics changes accumulated by the worker never get
      reported to the stats collector, which is bad.
      
      Julien Rouhaud
      4929704a
    • Peter Eisentraut's avatar
      Fix CRC check handling in get_controlfile · e79e6c4d
      Peter Eisentraut authored
      The previous patch broke this by returning NULL for a failed CRC check,
      which pg_controldata would then try to read.  Fix by returning the
      result of the CRC check in a separate argument.
      
      Michael Paquier and myself
      e79e6c4d
    • Robert Haas's avatar
      Fix dangling pointer problem in ReorderBufferSerializeChange. · 308985b0
      Robert Haas authored
      Commit 3fe3511d introduced a new
      case into this function, but neglected to ensure that the "ondisk"
      pointer got updated after a possible reallocation as the code does
      in other cases.
      
      Stas Kelvich, per diagnosis by Konstantin Knizhnik.
      308985b0
    • Heikki Linnakangas's avatar
      Turn password_encryption GUC into an enum. · babe05bc
      Heikki Linnakangas authored
      This makes the parameter easier to extend, to support other password-based
      authentication protocols than MD5. (SCRAM is being worked on.)
      
      The GUC still accepts on/off as aliases for "md5" and "plain", although
      we may want to remove those once we actually add support for another
      password hash type.
      
      Michael Paquier, reviewed by David Steele, with some further edits by me.
      
      Discussion: <CAB7nPqSMXU35g=W9X74HVeQp0uvgJxvYOuA4A-A3M+0wfEBv-w@mail.gmail.com>
      babe05bc
  7. 27 Sep, 2016 5 commits
    • Tom Lane's avatar
      Disallow pushing volatile quals past set-returning functions. · 72daabc7
      Tom Lane authored
      Pushing an upper-level restriction clause into an unflattened
      subquery-in-FROM is okay when the subquery contains no SRFs in its
      targetlist, or when it does but the SRFs are unreferenced by the clause
      *and the clause is not volatile*.  Otherwise, we're changing the number
      of times the clause is evaluated, which is bad for volatile quals, and
      possibly changing the result, since a volatile qual might succeed for some
      SRF output rows and not others despite not referencing any of the changing
      columns.  (Indeed, if the clause is something like "random() > 0.5", the
      user is probably expecting exactly that behavior.)
      
      We had most of these restrictions down, but not the one about the upper
      clause not being volatile.  Fix that, and add a regression test to
      illustrate the expected behavior.
      
      Although this is definitely a bug, it doesn't seem like back-patch
      material, since possibly some users don't realize that the broken
      behavior is broken and are relying on what happens now.  Also, while
      the added test is quite cheap in the wake of commit a4c35ea1, it would
      be much more expensive (or else messier) in older branches.
      
      Per report from Tom van Tilburg.
      
      Discussion: <CAP3PPDiucxYCNev52=YPVkrQAPVF1C5PFWnrQPT7iMzO1fiKFQ@mail.gmail.com>
      72daabc7
    • Tom Lane's avatar
      Make struct ParallelSlot private within pg_dump/parallel.c. · 0109ab27
      Tom Lane authored
      The only field of this struct that other files have any need to touch
      is the pointer to the TocEntry a worker is working on.  (Well,
      pg_backup_archiver.c is actually looking at workerStatus too, but that
      can be finessed by specifying that the TocEntry pointer is NULL for a
      non-busy worker.)
      
      Hence, move out the TocEntry pointers to a separate array within
      struct ParallelState, and then we can make struct ParallelSlot private.
      
      I noted the possibility of this previously, but hadn't got round to
      actually doing it.
      
      Discussion: <1188.1464544443@sss.pgh.pa.us>
      0109ab27
    • Tom Lane's avatar
      Rationalize parallel dump/restore's handling of worker cmd/status messages. · fb03d08a
      Tom Lane authored
      The existing APIs for creating and parsing command and status messages are
      rather messy; for example, archive-format modules have to provide code
      for constructing command messages, which is entirely pointless since
      the code to read them is hard-wired in WaitForCommands() and hence
      no format-specific variation is actually possible.  But there's little
      foreseeable reason to need format-specific variation anyway.
      
      The situation for status messages is no better; at least those are both
      constructed and parsed by format-specific code, but said code is quite
      redundant since there's no actual need for format-specific variation.
      
      To add insult to injury, the first API involves returning pointers to
      static buffers, which is bad, while the second involves returning pointers
      to malloc'd strings, which is safer but randomly inconsistent.
      
      Hence, get rid of the MasterStartParallelItem and MasterEndParallelItem
      APIs, and instead write centralized functions that construct and parse
      command and status messages.  If we ever do need more flexibility, these
      functions can be the standard implementations of format-specific
      callback methods, but that's a long way off if it ever happens.
      
      Tom Lane, reviewed by Kevin Grittner
      
      Discussion: <17340.1464465717@sss.pgh.pa.us>
      fb03d08a
    • Tom Lane's avatar
      Redesign parallel dump/restore's wait-for-workers logic. · b7b8cc0c
      Tom Lane authored
      The ListenToWorkers/ReapWorkerStatus APIs were messy and hard to use.
      Instead, make DispatchJobForTocEntry register a callback function that
      will take care of state cleanup, doing whatever had been done by the caller
      of ReapWorkerStatus in the old design.  (This callback is essentially just
      the old mark_work_done function in the restore case, and a trivial test for
      worker failure in the dump case.)  Then we can have ListenToWorkers call
      the callback immediately on receipt of a status message, and return the
      worker to WRKR_IDLE state; so the WRKR_FINISHED state goes away.
      
      This allows us to design a unified wait-for-worker-messages loop:
      WaitForWorkers replaces EnsureIdleWorker and EnsureWorkersFinished as well
      as the mess in restore_toc_entries_parallel.  Also, we no longer need the
      fragile API spec that the caller of DispatchJobForTocEntry is responsible
      for ensuring there's an idle worker, since DispatchJobForTocEntry can just
      wait until there is one.
      
      In passing, I got rid of the ParallelArgs struct, which was a net negative
      in terms of notational verboseness, and didn't seem to be providing any
      noticeable amount of abstraction either.
      
      Tom Lane, reviewed by Kevin Grittner
      
      Discussion: <1188.1464544443@sss.pgh.pa.us>
      b7b8cc0c
    • Tom Lane's avatar
      Improve contrib/cube's handling of zero-D cubes, infinities, and NaNs. · f31a931f
      Tom Lane authored
      It's always been possible to create a zero-dimensional cube by converting
      from a zero-length float8 array, but cube_in failed to accept the '()'
      representation that cube_out produced for that case, resulting in a
      dump/reload hazard.  Make it accept the case.  Also fix a couple of
      other places that didn't behave sanely for zero-dimensional cubes:
      cube_size would produce 1.0 when surely the answer should be 0.0,
      and g_cube_distance risked a divide-by-zero failure.
      
      Likewise, it's always been possible to create cubes containing float8
      infinity or NaN coordinate values, but cube_in couldn't parse such input,
      and cube_out produced platform-dependent spellings of the values.  Convert
      them to use float8in_internal and float8out_internal so that the behavior
      will be the same as for float8, as we recently did for the core geometric
      types (cf commit 50861cd6).  As in that commit, I don't pretend that this
      patch fixes all insane corner-case behaviors that may exist for NaNs, but
      it's a step forward.
      
      (This change allows removal of the separate cube_1.out and cube_3.out
      expected-files, as the platform dependency that previously required them
      is now gone: an underflowing coordinate value will now produce an error
      not plus or minus zero.)
      
      Make errors from cube_in follow project conventions as to spelling
      ("invalid input syntax for cube" not "bad cube representation")
      and errcode (INVALID_TEXT_REPRESENTATION not SYNTAX_ERROR).
      
      Also a few marginal code cleanups and comment improvements.
      
      Tom Lane, reviewed by Amul Sul
      
      Discussion: <15085.1472494782@sss.pgh.pa.us>
      f31a931f