]> granicus.if.org Git - postgresql/commit
Rationalize format-picture caching logic in formatting.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 28 Sep 2016 21:08:40 +0000 (17:08 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 28 Sep 2016 21:08:40 +0000 (17:08 -0400)
commit83bed06be4e808f3da30f99b6c91e9efda3961ad
tree4762afe4684f82fd63537a5c6617e642aea5c17c
parentd3cd36a133d96ad5578b6c10279b55fd5b538093
Rationalize format-picture caching logic in formatting.c.

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 ff783fbae in favor of a less fragile
solution; the problems with that approach are well illustrated by later
hacking such as 55f927a46.)

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>
src/backend/utils/adt/formatting.c