Tom Lane <[email protected]> writes:
> Still, we should consider whether the right answer is to fix that logic
> to allow hex input. A (weak) argument for going that way is we'd stay
> bug-compatible with the existing behavior for octal input.
I ran a differential sweep to put numbers on that choice: unpatched master
(base 0) against master + Marko's patch (base 10), same server binaries,
only _int.so differs, ~264k parsed inputs. Full write-up and repro are
attached (evidence.txt); the parts that bear on the decision:
Hex and 0b are unreachable, measured. gettoken() copies only '0'-'9' and a
leading '-' into the operand buffer, so strtol never sees 'x' or 'b'. On
unpatched master every one of 0x10, 0X1F, 0b1, 0o7 is already a syntax
error. (_int.so here links __isoc23_strtol, so base 0 *would* take 0x and
0b if either reached it -- it doesn't.) So there is no existing hex
behavior to stay compatible with: allowing hex would be adding a feature,
not preserving one. The only thing base 0 does today is read a
leading-zero operand as octal.
And that octal reading is the harmful part, not something worth keeping. A
leading-zero operand containing an 8 or 9 is truncated silently, not
rejected: '08' and '09' both parse as 0, '-08' as 0 with the sign dropped,
'018' as 1, and '02147483647' as 9020. That is worse, and quieter, than
the '010' -> 8 in Marko's report. Every
behavior change between the two builds falls in this leading-zero class
(33,883 of ~264k inputs; none outside it). The change is per operand and
does not touch the operators, NOT, parentheses or nesting (4,007 random
boolean queries, exact match in both directions), and @@/~~ stay
consistent across seqscan, gist__int_ops, gist__intbig_ops and
gin__int_ops (0 disagreements). Output carries no leading zero, so values
already stored in parsed form survive dump/restore unchanged; only query
text parsed again after the upgrade picks up the decimal reading.
One compatibility point worth a line in the commit message: base 10 does
reject a small set of inputs base 0 accepted -- leading-zero values whose
octal reading fits in int32 but whose decimal reading does not, e.g.
'017777777777' (octal INT_MAX) and '-020000000000' (octal INT_MIN); 5,178
such inputs in the sweep. Nothing goes the other way.
So the measurement lines up with your and Marko's read: base 10 is the
fix, and the hex route would be new behavior rather than kept behavior. If
it helps, tests-suggested.diff (attached, on top of 0001) adds the
silent-truncation and now-rejected cases; it passes on the patched build
and fails on master.
regards,
Manu
query_int: base 0 -> base 10 (Marko Grujic's 0001) -- adversarial differential
sweep
====================================================================================
Setup
-----
master 42e96cf2fe0 (2026-10-01), non-assert build (--prefix only), gcc 16.2.1,
glibc 2.44.
contrib/intarray/_int_bool.c is blob 4382f882939, i.e. exactly the patch
pre-image.
Two installs with the same server binaries; only _int.so differs:
b0 = master strtol(nnn, NULL, 0)
b10 = master + 0001 strtol(nnn, NULL, 10)
Every query below ran on both. Scripts: barrido/run.sh, barrido/sweep.py; raw
output in
barrido/out/.
1. Scope: one parser, one call site
-----------------------------------
- strtol appears once in contrib/intarray/*.c: _int_bool.c:96. No other integer
parsing
in _int_gist.c, _intbig_gist.c, _int_gin.c, _int_op.c, _int_tool.c,
_int_selfuncs.c.
- query_int has INPUT = bqarr_in, OUTPUT = bqarr_out and no receive/send, so
bqarr_in is
the only text-to-query_int path.
2. Hex (and binary) are unreachable, confirmed empirically
----------------------------------------------------------
gettoken() only copies '0'-'9' (and a leading '-') into nnn, so strtol never
sees 'x' or
'b'. _int.so links __isoc23_strtol@GLIBC_2.38, so base 0 here would accept both
0x and the
C23 0b prefix if either reached it. On unpatched master all of these are
already errors:
'0x10' '0X1F' '0x' '0b1' '0B1' '0o7' '+1' '1e3' '1.0' '1_000' ->
syntax error
So no input that parses today depends on base 0 accepting a prefix. The only
difference
between base 0 and base 10 is that a leading zero selects octal.
3. Single operands: 259,599 inputs, checked against a model
-----------------------------------------------------------
Inputs: every digit string of length 1-5, with and without '-'; zero-padded
forms of 19
values near the octal/decimal/int32/long limits, padded up to and past the
15-char nnn
buffer; 40,000 seeded random operands of 6-17 digits, about half with leading
zeros.
Each result was compared with a model of gettoken() + glibc strtol + the int32
check.
model mismatches b0: 0 b10: 0
unchanged 220,538
value changed 33,883
valid on b0 -> error b10 5,178
error on b0 -> valid b10 0
changed, no leading zero 0
leading-zero inputs 41,590 (2,529 of them unchanged: octal ==
decimal, e.g.
'00'..'07', or both are errors)
Every changed input has a leading zero. Leading-zero inputs whose octal and
decimal
readings agree are unchanged.
Selected cases (full table: out/A_named.tsv, 94 cases, 43 changed):
input master (base 0) patched (base 10)
'07' 7 7
'08' 0 8 <- silent: strtol stops at '8'
'09' 0 9 <- silent
'-08' 0 -8 <- silent, sign lost
'010' 8 10
'-010' -8 -10
'018' 1 18 <- octal prefix '01', rest
dropped
'0100' 64 100
'02147483647' 9020 2147483647 <- octal prefix '021474'
'02147483648' 9020 error
'017777777777' 2147483647 error <- octal INT_MAX
'-020000000000' -2147483648 error <- octal INT_MIN
'2147483648' error error
'-2147483648' -2147483648 -2147483648
'000000000000010' (15 chars) 8 10
'0000000000000010' (16 chars) error error <- nnn[16] overrun, unchanged
In base 0, any leading-zero operand containing an 8 or 9 is truncated silently
at that
digit, with no error (26,473 of the inputs above). '08' and '09' give 0.
The only inputs that go from accepted to rejected are leading-zero values whose
octal
reading fits in int32 and whose decimal reading does not (5,178 here, e.g.
'017777777777'). No input goes from rejected to accepted.
4. Boolean structure: the change is per operand
-----------------------------------------------
4,007 random queries (depth <= 5; !, (), &, |; operands zero-padded and/or
negative).
b0(q) == b10(q with every operand replaced by its octal value) 4,007 /
4,007
b10(q) == b0(q with every operand replaced by its decimal value) 4,007 /
4,007
The patch does not interact with the operators, NOT, parentheses or nesting.
Examples:
'!(010&!08)' master: !( 8 & !0 ) patched: !( 10 & !8 )
'(010|8)&(10|012)' master: ( 8 | 8 ) & ( 10 | 10 ) patched: ( 10 | 8 ) & (
10 | 12 )
5. @@ / ~~ consistency: seqscan vs GiST (int, intbig) vs GIN
-----------------------------------------------------------
30,000-row table (int[] values 0..71, some empty arrays). 24
leading-zero/boolean queries
plus their canonical forms. Each was run with a@@q and q~~a under four access
paths: Seq
Scan, gist__int_ops, gist__intbig_ops and gin__int_ops. Plans were checked as
"Seq Scan"
or "Bitmap Index Scan on ti".
Within one build, a path or operator gives a different row set: 0
Cross-check: master's rows for q == patched rows for master's
output text of q (e.g. master '010' vs patched '8'), every path: 0 failures
Index paths agree with seqscan on both builds. Only the value of the parsed
operand
changes. Example, rows matched:
'08' master 1294 (rows containing 0) patched 1117 (rows containing 8)
'010' master 1117 (rows containing 8) patched 1472 (rows containing 10)
6. Output and dump/restore
--------------------------
bqarr_out prints "%d", so output never has a leading zero and query_int text
round-trips
on both builds. Dumped from master with pg_dump, restored into patched with 0
errors:
stored query_int values '010' -> 8, '08' -> 0 on master; still 8 / 0 after
restore
view constant dumped as '8'::query_int; unchanged
BEGIN ATOMIC function parsed at CREATE; unchanged (true)
column DEFAULT / CHECK dumped as '8'::query_int; unchanged
string-bodied SQL function body keeps '010'; re-parsed: true on master, false
patched
text column cast at runtime '010'::query_int: 8 on master, 10 patched
Anything stored in parsed form keeps its old value across dump/restore. Only
query text
that is parsed again after the upgrade picks up the decimal reading.
7. Regression suite
-------------------
patched tests on patched build pass
master tests on patched build pass (no other expected output changes)
master tests on master build pass
patched tests on master build fail, only in the two new queries (8 / f
instead of
10 / t)
8. Verdict
----------
We found no new broken edge in the patch. Over about 264,000 parsed inputs,
every behaviour
change belongs to the leading-zero class. Hex and 0b are unreachable from the
tokenizer
today. The change is per operand and does not depend on the boolean structure.
Seqscan/GiST/intbig/GIN stay consistent, and parsed stored values are
unaffected by
dump/restore. Effects the commit message or tests could mention:
- '08'/'09' (and any leading-zero operand containing 8/9) silently became 0
or a
truncated octal prefix; that is the worst current case, worse than '010' ->
8.
- a small set of formerly valid inputs now error: leading-zero values whose
octal
reading fits in int32 but whose decimal reading does not ('017777777777').
Suggested extra tests (barrido/tests-suggested.diff, applies on top of 0001;
passes on the
patched build, fails on master):
SELECT '08'::query_int, '-010'::query_int, '!(010|09)'::query_int; -- 8 |
-10 | !( 10 | 9 )
SELECT '02147483647'::query_int; --
2147483647
SELECT '017777777777'::query_int; -- ERROR:
syntax error
Side note, pre-existing and unrelated to the patch (same on both builds): a
lone '-' is
accepted as 0, since strtol("-") converts nothing and does not set errno:
'-' -> 0 '1&-' -> 1 & 0 '!-' -> !0 '(-)' -> 0 ('- 1', '--1' are
errors)
--- a/contrib/intarray/sql/_int.sql 2026-10-17 21:15:02.494272800 -0300
+++ b/contrib/intarray/sql/_int.sql 2026-10-17 21:19:40.670412273 -0300
@@ -78,6 +78,9 @@
-- Leading zeros do not make an octal number.
SELECT '010'::query_int;
SELECT '{10}'::int[] @@ '010'::query_int;
+SELECT '08'::query_int, '-010'::query_int, '!(010|09)'::query_int;
+SELECT '02147483647'::query_int;
+SELECT '017777777777'::query_int;
-- Test for overflow of the int16 "left" field in findoprnd().
-- This query uses a balanced binary tree to avoid using too much stack.
--- a/contrib/intarray/expected/_int.out 2026-10-17 21:15:02.494206846 -0300
+++ b/contrib/intarray/expected/_int.out 2026-10-17 21:19:42.422411677 -0300
@@ -411,6 +411,22 @@
t
(1 row)
+SELECT '08'::query_int, '-010'::query_int, '!(010|09)'::query_int;
+ query_int | query_int | query_int
+-----------+-----------+-------------
+ 8 | -10 | !( 10 | 9 )
+(1 row)
+
+SELECT '02147483647'::query_int;
+ query_int
+------------
+ 2147483647
+(1 row)
+
+SELECT '017777777777'::query_int;
+ERROR: syntax error
+LINE 1: SELECT '017777777777'::query_int;
+ ^
-- Test for overflow of the int16 "left" field in findoprnd().
-- This query uses a balanced binary tree to avoid using too much stack.
DO $$
