Re: BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode

Hi, Michael!

Thanks for the report!

The more you find, the more we fix. Together we'll make the codebase much
better.

I looked at this and agree with the analysis.

The std flag on DCHCacheEntry was added in 1a950f37d0a (Implement
standard datetime parsing mode, Alexander Korotkov, 2019). That commit
correctly threaded std through DCH_cache_getnew / search / fetch and set
ent->std on the allocation path, but the recycle path only refreshed
str / age / valid and left the previous occupant's std in place.
DCH_cache_search() keys on (str, std), so after an eviction a tree
parsed in one mode can be reused under the other — exactly the
session-dependent .datetime() behaviour in the report.

The fix is to set old->std = std when recycling, matching the new-entry
branch. Added a regress test for the cross-mode eviction case.

Alexander — this is in code you introduced, would you mind reviewing and
pushing if it looks right?

Patch with tests attached.

сб, 1 авг. 2026 г. в 21:14, PG Bug reporting form <noreply(at)postgresql(dot)org>:

> The following bug has been logged on the website:
>
> Bug reference: 19594
> Logged by: Michael Malis
> Email address: malis(at)pgrust(dot)com
> PostgreSQL version: 18.3
> Operating system: Debian (docker postgres:18.3, aarch64)
> Description:
>
> Hey. This is the 9th bug I've found in a couple of days. I'm maybe 10% of
> the way through the codebase so I expect to find a lot more. Should I be
> submitting bugs in a different way to make it easier for you?
>
> DCH_cache_getnew() in src/backend/utils/adt/formatting.c fails to reset the
> per-entry "std" (SQL/JSON standard mode) flag when it recycles a cache
> entry.
> Because DCH_cache_search() matches on (str, std), a format tree that was
> parsed in one strict-mode becomes reachable from the other.
>
> The user-visible effect is that the same query, in the same session,
> returns
> a different answer depending on what else that session has formatted
> earlier. In particular jsonpath's .datetime(), which is required to use
> SQL/JSON standard mode, can be handed a leniently-parsed tree and will then
> accept format pictures the standard forbids.
>
> All of the following runs in a single fresh session against a stock 18.3
> server. No configuration changes are required.
>
> -- 1. Control: in a fresh session, standard mode correctly rejects "z"
> -- as a datetime format separator.
> SELECT jsonb_path_query('"12z34"'::jsonb, '$.datetime("HH24zMI")');
> ERROR: invalid datetime format separator: "z"
>
> -- 2. Seed the cache with the picture "HH24MI" parsed in STANDARD mode
> -- (std = true), via jsonpath.
> SELECT jsonb_path_query('"1234"'::jsonb, '$.datetime("HH24MI")');
> jsonb_path_query
> ------------------
> "12:34:00"
>
> -- 3. Fill the remaining cache slots. DCH_CACHE_ENTRIES is 20, so
> exactly
> -- 19 further distinct pictures are needed to make the next miss
> evict.
> -- (The to_char() result must actually be consumed, or the planner may
> -- elide the calls and no cache entries are created.)
> SELECT count(*) FROM generate_series(1,19) g
> WHERE to_char(now(), 'HH24MI'||g) IS NOT NULL;
> count
> -------
> 19
>
> -- 4. A to_char() call, i.e. LENIENT mode (std = false), with a new
> -- picture. This misses, and evicts the entry created in step 2.
> SELECT to_char(now(), 'HH24zMI');
> to_char
> ---------
> 14z51
>
> -- 5. Exactly the query from step 1. It now succeeds.
> SELECT jsonb_path_query('"12z34"'::jsonb, '$.datetime("HH24zMI")');
> jsonb_path_query
> ------------------
> "12:34:00"
>
> Step 5 is the defect. jsonpath .datetime() is standard mode and must
> reject
> "z" as a separator, exactly as it did in step 1, but it is served the
> lenient
> tree left behind by step 4.
>
>
> EXPECTED
> ========
>
> Step 5 raises the same error as step 1:
>
> ERROR: invalid datetime format separator: "z"
>
> The result of a format operation must not depend on the session's cache
> history.
>
>
> ANALYSIS
> ========
>
> src/backend/utils/adt/formatting.c. The cache entry carries the mode:
>
> 394 typedef struct
> 395 {
> 396 FormatNode format[DCH_CACHE_SIZE + 1];
> 397 char str[DCH_CACHE_SIZE + 1];
> 398 bool std;
> 399 bool valid;
> 400 int age;
> 401 } DCHCacheEntry;
>
> DCH_cache_getnew() has two branches. The allocation branch sets std:
>
> 3867 DCHCache[n_DCHCache] = ent = (DCHCacheEntry *)
> 3868 MemoryContextAllocZero(TopMemoryContext,
> sizeof(DCHCacheEntry));
> 3869 ent->valid = false;
> 3870 strlcpy(ent->str, str, DCH_CACHE_SIZE + 1);
> 3871 ent->std = std; <-- set here
> 3872 ent->age = (++DCHCounter);
>
> The recycle branch does not:
>
> 3855 old->valid = false;
> 3856 strlcpy(old->str, str, DCH_CACHE_SIZE + 1);
> 3857 old->age = (++DCHCounter); <-- old->std is never updated
> 3858 /* caller is expected to fill format, then set valid */
> 3859 return old;
>
> So a recycled entry keeps the std value of its previous occupant, while its
> str and format are those of the new picture. DCH_cache_search() then
> matches
> on the stale flag:
>
> 3890 if (ent->valid && strcmp(ent->str, str) == 0 && ent->std ==
> std)
>
> DCH_cache_fetch() parses with (std ? STD_FLAG : 0), so the tree stored in
> the
> recycled slot is parsed in the *requested* mode but filed under the
> *previous* occupant's mode. A later lookup in the previous occupant's mode
> finds it and reuses it; a later lookup in the mode it was actually parsed
> under misses and re-parses. Both directions are wrong; the reproducer
> above
> shows the first.
>
>
>
>
>

--
Regards,
Rachitskiy Andrey