agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode
2+ messages / 2 participants
[nested] [flat]
* BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode
@ 2026-08-01 15:05 PG Bug reporting form <noreply@postgresql.org>
2026-08-01 16:50 ` Re: BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 2+ messages in thread
From: PG Bug reporting form @ 2026-08-01 15:05 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: malis@pgrust.com
The following bug has been logged on the website:
Bug reference: 19594
Logged by: Michael Malis
Email address: malis@pgrust.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.
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode
2026-08-01 15:05 BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode PG Bug reporting form <noreply@postgresql.org>
@ 2026-08-01 16:50 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 0 replies; 2+ messages in thread
From: Andrey Rachitskiy @ 2026-08-01 16:50 UTC (permalink / raw)
To: malis@pgrust.com; pgsql-bugs@lists.postgresql.org; +Cc: Alexander Korotkov <akorotkov@postgresql.org>
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@postgresql.org>:
> The following bug has been logged on the website:
>
> Bug reference: 19594
> Logged by: Michael Malis
> Email address: malis@pgrust.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
Attachments:
[text/x-patch] 0001-Fix-DCH-cache-recycle-to-update-std-flag.patch (3.3K, ../../CAB8bMit-UFnAz0C+i-0DCo3O7Dzq-+0C75z78W-MpJMyW+cztA@mail.gmail.com/3-0001-Fix-DCH-cache-recycle-to-update-std-flag.patch)
download | inline diff:
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Sat, 1 Aug 2026 21:23:00 +0500
Subject: [PATCH] Fix DCH format-cache recycle to update std flag
DCH_cache_getnew() set ent->std only when allocating a new entry, not
when recycling an old one. DCH_cache_search() matches on (str, std), so
a format tree parsed in one mode could be reused under the other.
That let jsonpath .datetime() (SQL/JSON standard mode) reuse a format
tree built by lenient to_char(), once enough distinct format strings
had filled the cache and forced an eviction.
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: Michael Malis <malis@pgrust.com>
Discussion: https://www.postgresql.org/message-id/19594-5d9bdc019e3f7f6e%40postgresql.org
---
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index d52d71b0a8c..93a2336d951 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -3814,6 +3814,7 @@ DCH_cache_getnew(const char *str, bool std)
#endif
old->valid = false;
strlcpy(old->str, str, DCH_CACHE_SIZE + 1);
+ old->std = std;
old->age = (++DCHCounter);
/* caller is expected to fill format, then set valid */
return old;
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c7b8c36c842..d512328e836 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -1782,6 +1782,25 @@ select jsonb_path_query('"10-03-2017t12:34:56"', '$.datetime("dd-mm-yyyy\"T\"HH2
ERROR: unmatched format character "T"
select jsonb_path_query('"10-03-2017 12:34:56"', '$.datetime("dd-mm-yyyy\"T\"HH24:MI:SS")');
ERROR: unmatched format character "T"
+-- DCH cache recycle must preserve std vs lenient mode
+SELECT count(*) FROM (
+ SELECT jsonb_path_query(to_jsonb('x' || g::text || '1234'),
+ format('$.datetime("\"x%s\"HH24MI")', g)::jsonpath)
+ FROM generate_series(1, 20) g
+) s;
+ count
+-------
+ 20
+(1 row)
+
+SELECT to_char(timestamp '2000-01-01 12:34:00', 'HH24zMI');
+ to_char
+---------
+ 12z34
+(1 row)
+
+SELECT jsonb_path_query('"12z34"'::jsonb, '$.datetime("HH24zMI")');
+ERROR: invalid datetime format separator: "z"
-- Test .bigint()
select jsonb_path_query('null', '$.bigint()');
ERROR: jsonpath item method .bigint() can only be applied to a string or numeric value
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index c37dc3817ff..209ca470a9f 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -383,6 +383,15 @@ select jsonb_path_query('"10-03-2017T12:34:56"', '$.datetime("dd-mm-yyyy\"T\"HH2
select jsonb_path_query('"10-03-2017t12:34:56"', '$.datetime("dd-mm-yyyy\"T\"HH24:MI:SS")');
select jsonb_path_query('"10-03-2017 12:34:56"', '$.datetime("dd-mm-yyyy\"T\"HH24:MI:SS")');
+-- DCH cache recycle must preserve std vs lenient mode
+SELECT count(*) FROM (
+ SELECT jsonb_path_query(to_jsonb('x' || g::text || '1234'),
+ format('$.datetime("\"x%s\"HH24MI")', g)::jsonpath)
+ FROM generate_series(1, 20) g
+) s;
+SELECT to_char(timestamp '2000-01-01 12:34:00', 'HH24zMI');
+SELECT jsonb_path_query('"12z34"'::jsonb, '$.datetime("HH24zMI")');
+
-- Test .bigint()
select jsonb_path_query('null', '$.bigint()');
select jsonb_path_query('true', '$.bigint()');
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-08-01 16:50 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-01 15:05 BUG #19594: to_char/jsonpath format cache serves a format tree parsed in the wrong strict-mode PG Bug reporting form <noreply@postgresql.org>
2026-08-01 16:50 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox