public inbox for [email protected]help / color / mirror / Atom feed
Re-add recently-removed tests for ltree and intarray 7+ messages / 3 participants [nested] [flat]
* Re-add recently-removed tests for ltree and intarray @ 2026-05-14 23:38 Michael Paquier <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Michael Paquier @ 2026-05-14 23:38 UTC (permalink / raw) To: Postgres hackers <[email protected]>; +Cc: Tom Lane <[email protected]> Hi all, Some of you may have noticed that some regression tests have been removed due to some noise in the buildfarm, as of commit 906ea101d0d5. We did not have time to do something for this release, unfortunately. It is possible to reproduce the incompatibility by setting max_stack_depth to a low value, where the first new query of ltree and intarray would fail, when written in their original shape. Tom had the idea to switch these two unstable tests to use a balanced binary tree instead, so as they don't eat the stack still are able to cover the recent fixes pushed into the tree. And this investigation has led me to the attached, to-be-backpatched down to v14. Even under a low max_stack_depth, these new tests are stable. I could not see an issue for the two tests added at the bottom of ltree. Opinions or comments? Thanks, -- Michael From d11a1dc0e8dc8d4838ca81fe8d4aa935cc1ed2df Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 14 May 2026 13:40:00 +0900 Subject: [PATCH] Re-add regression tests for ltree and intarray These tests have been removed by 906ea101d0d5, due to some of them being unstable in the buildfarm with low max_stack_depth values. These tests are reworked to be more portable. The tests to cover the findoprnd() overflows use a balanced tree to avoid using too much stack, per a suggestion by Tom Lane. Backpatch-through: 14 --- contrib/intarray/expected/_int.out | 15 +++++++++++++++ contrib/intarray/sql/_int.sql | 13 +++++++++++++ contrib/ltree/expected/ltree.out | 24 ++++++++++++++++++++++++ contrib/ltree/sql/ltree.sql | 20 ++++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out index d0e68d0447fb..fb4086a95caf 100644 --- a/contrib/intarray/expected/_int.out +++ b/contrib/intarray/expected/_int.out @@ -398,6 +398,21 @@ SELECT '1&(2&(4&(5|!6)))'::query_int; 1 & 2 & 4 & ( 5 | !6 ) (1 row) +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := '1'; +BEGIN + FOR i IN 1..15 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('0|' || e)::query_int; +END; +$$; +ERROR: query_int expression is too complex +CONTEXT: SQL statement "SELECT ('0|' || e)::query_int" +PL/pgSQL function inline_code_block line 8 at PERFORM -- test non-error-throwing input SELECT str as "query_int", pg_input_is_valid(str,'query_int') as ok, diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql index 5668ab407045..0d30914725e1 100644 --- a/contrib/intarray/sql/_int.sql +++ b/contrib/intarray/sql/_int.sql @@ -75,6 +75,19 @@ SELECT '1&2&4&5&6'::query_int; SELECT '1&(2&(4&(5|6)))'::query_int; SELECT '1&(2&(4&(5|!6)))'::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 $$ +DECLARE + e text := '1'; +BEGIN + FOR i IN 1..15 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('0|' || e)::query_int; +END; +$$; + -- test non-error-throwing input SELECT str as "query_int", diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index d2a566284755..15b9131a7506 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1283,6 +1283,21 @@ SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; f (1 row) +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := 'a'; +BEGIN + FOR i IN 1..14 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('b|' || e)::ltxtquery; +END; +$$; +ERROR: ltxtquery is too large +CONTEXT: SQL statement "SELECT ('b|' || e)::ltxtquery" +PL/pgSQL function inline_code_block line 8 at PERFORM --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; ?column? @@ -8202,3 +8217,12 @@ FROM (VALUES ('.2.3', 'ltree'), !tree & aWdf@* | ltxtquery | t | | | | (8 rows) +-- Test for overflow of lquery_level.totallen. +SELECT (repeat('x', 255) || repeat('|' || repeat('x', 255), 256))::lquery; +ERROR: lquery level is too large +DETAIL: Total size of level exceeds the maximum allowed (65535 bytes). +--- Test for overflow of lquery_level.numvar, with a set of single-char +--- variants in one level. +SELECT (repeat('a|', 65535) || 'a')::lquery; +ERROR: lquery level has too many variants +DETAIL: Number of variants exceeds the maximum allowed (65535). diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 77e6958c62a7..d0fade9d17d8 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -253,6 +253,19 @@ SELECT 'tree.awdfg'::ltree @ 'tree & aWdfg@'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_qw%*'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := 'a'; +BEGIN + FOR i IN 1..14 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('b|' || e)::ltxtquery; +END; +$$; + --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; @@ -457,3 +470,10 @@ FROM (VALUES ('.2.3', 'ltree'), ('!tree & aWdf@*','ltxtquery')) AS a(str,typ), LATERAL pg_input_error_info(a.str, a.typ) as errinfo; + +-- Test for overflow of lquery_level.totallen. +SELECT (repeat('x', 255) || repeat('|' || repeat('x', 255), 256))::lquery; + +--- Test for overflow of lquery_level.numvar, with a set of single-char +--- variants in one level. +SELECT (repeat('a|', 65535) || 'a')::lquery; -- 2.54.0 Attachments: [text/plain] 0001-Re-add-regression-tests-for-ltree-and-intarray.patch (5.0K, 2-0001-Re-add-regression-tests-for-ltree-and-intarray.patch) download | inline diff: From d11a1dc0e8dc8d4838ca81fe8d4aa935cc1ed2df Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 14 May 2026 13:40:00 +0900 Subject: [PATCH] Re-add regression tests for ltree and intarray These tests have been removed by 906ea101d0d5, due to some of them being unstable in the buildfarm with low max_stack_depth values. These tests are reworked to be more portable. The tests to cover the findoprnd() overflows use a balanced tree to avoid using too much stack, per a suggestion by Tom Lane. Backpatch-through: 14 --- contrib/intarray/expected/_int.out | 15 +++++++++++++++ contrib/intarray/sql/_int.sql | 13 +++++++++++++ contrib/ltree/expected/ltree.out | 24 ++++++++++++++++++++++++ contrib/ltree/sql/ltree.sql | 20 ++++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out index d0e68d0447fb..fb4086a95caf 100644 --- a/contrib/intarray/expected/_int.out +++ b/contrib/intarray/expected/_int.out @@ -398,6 +398,21 @@ SELECT '1&(2&(4&(5|!6)))'::query_int; 1 & 2 & 4 & ( 5 | !6 ) (1 row) +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := '1'; +BEGIN + FOR i IN 1..15 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('0|' || e)::query_int; +END; +$$; +ERROR: query_int expression is too complex +CONTEXT: SQL statement "SELECT ('0|' || e)::query_int" +PL/pgSQL function inline_code_block line 8 at PERFORM -- test non-error-throwing input SELECT str as "query_int", pg_input_is_valid(str,'query_int') as ok, diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql index 5668ab407045..0d30914725e1 100644 --- a/contrib/intarray/sql/_int.sql +++ b/contrib/intarray/sql/_int.sql @@ -75,6 +75,19 @@ SELECT '1&2&4&5&6'::query_int; SELECT '1&(2&(4&(5|6)))'::query_int; SELECT '1&(2&(4&(5|!6)))'::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 $$ +DECLARE + e text := '1'; +BEGIN + FOR i IN 1..15 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('0|' || e)::query_int; +END; +$$; + -- test non-error-throwing input SELECT str as "query_int", diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index d2a566284755..15b9131a7506 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1283,6 +1283,21 @@ SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; f (1 row) +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := 'a'; +BEGIN + FOR i IN 1..14 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('b|' || e)::ltxtquery; +END; +$$; +ERROR: ltxtquery is too large +CONTEXT: SQL statement "SELECT ('b|' || e)::ltxtquery" +PL/pgSQL function inline_code_block line 8 at PERFORM --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; ?column? @@ -8202,3 +8217,12 @@ FROM (VALUES ('.2.3', 'ltree'), !tree & aWdf@* | ltxtquery | t | | | | (8 rows) +-- Test for overflow of lquery_level.totallen. +SELECT (repeat('x', 255) || repeat('|' || repeat('x', 255), 256))::lquery; +ERROR: lquery level is too large +DETAIL: Total size of level exceeds the maximum allowed (65535 bytes). +--- Test for overflow of lquery_level.numvar, with a set of single-char +--- variants in one level. +SELECT (repeat('a|', 65535) || 'a')::lquery; +ERROR: lquery level has too many variants +DETAIL: Number of variants exceeds the maximum allowed (65535). diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 77e6958c62a7..d0fade9d17d8 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -253,6 +253,19 @@ SELECT 'tree.awdfg'::ltree @ 'tree & aWdfg@'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_qw%*'::ltxtquery; SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery; +-- Test for overflow of the int16 "left" field in findoprnd(). +-- This query uses a balanced binary tree to avoid using too much stack. +DO $$ +DECLARE + e text := 'a'; +BEGIN + FOR i IN 1..14 LOOP + e := '(' || e || '&' || e || ')'; + END LOOP; + PERFORM ('b|' || e)::ltxtquery; +END; +$$; + --arrays SELECT '{1.2.3}'::ltree[] @> '1.2.3.4'; @@ -457,3 +470,10 @@ FROM (VALUES ('.2.3', 'ltree'), ('!tree & aWdf@*','ltxtquery')) AS a(str,typ), LATERAL pg_input_error_info(a.str, a.typ) as errinfo; + +-- Test for overflow of lquery_level.totallen. +SELECT (repeat('x', 255) || repeat('|' || repeat('x', 255), 256))::lquery; + +--- Test for overflow of lquery_level.numvar, with a set of single-char +--- variants in one level. +SELECT (repeat('a|', 65535) || 'a')::lquery; -- 2.54.0 [application/pgp-signature] signature.asc (833B, 3-signature.asc) download ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 02:09 Tom Lane <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Tom Lane @ 2026-05-15 02:09 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]> Michael Paquier <[email protected]> writes: > Some of you may have noticed that some regression tests have been > removed due to some noise in the buildfarm, as of commit 906ea101d0d5. > We did not have time to do something for this release, unfortunately. > It is possible to reproduce the incompatibility by setting > max_stack_depth to a low value, where the first new query of ltree and > intarray would fail, when written in their original shape. Just to add a little more color to this --- what we discovered after there was time for some investigation was that: (a) the stack-overflow failure occurred in the findoprnd() function of intarray/_int_bool.c or ltree/ltxtquery_io.c. (b) the failure only appeared on buildfarm members running on ppc64 or s390x. I determined by examining assembly code that ppc64 uses about 3X as much stack per call level in this function as x86_64; probably s390x is similar. That was enough to overrun our default max_stack_depth on these architectures, even though the same case passed on the machines we'd tested on. (c) even with minimum max_stack_depth, the test passed using gcc but not clang. Again examining assembly code, gcc is smart enough to collapse the tail-recursion calls in findoprnd() into looping, causing the original test case's right-deep query tree to consume essentially zero stack space. clang doesn't do that, at least not on those arches at default optimization level. You can make gcc fail too with -O0. So it'd be good to verify on a few oddball platforms that Michael's new attempt is OK. It should theoretically work, but ... regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 02:47 John Naylor <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: John Naylor @ 2026-05-15 02:47 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Postgres hackers <[email protected]> On Fri, May 15, 2026 at 9:09 AM Tom Lane <[email protected]> wrote: > (b) the failure only appeared on buildfarm members running on ppc64 > or s390x. I determined by examining assembly code that ppc64 uses > about 3X as much stack per call level in this function as x86_64; > probably s390x is similar. That was enough to overrun our default > max_stack_depth on these architectures, even though the same case > passed on the machines we'd tested on. FWIW, I tried to reproduce with the former new tests un-reverted, and didn't see stack overflow on the following, so unless I fat-fingered that I wonder if there's something more specific on the previously failing members: ppc64le / gcc 8.5 / Linux kernel 4.18 S390X / gcc 13.3 / Linux kernel 6.8 -- John Naylor Amazon Web Services ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 02:49 Tom Lane <[email protected]> parent: John Naylor <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Tom Lane @ 2026-05-15 02:49 UTC (permalink / raw) To: John Naylor <[email protected]>; +Cc: Michael Paquier <[email protected]>; Postgres hackers <[email protected]> John Naylor <[email protected]> writes: > FWIW, I tried to reproduce with the former new tests un-reverted, and > didn't see stack overflow on the following, so unless I fat-fingered > that I wonder if there's something more specific on the previously > failing members: > ppc64le / gcc 8.5 / Linux kernel 4.18 > S390X / gcc 13.3 / Linux kernel 6.8 Hm, did you use -O0 ? regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 04:59 Michael Paquier <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Michael Paquier @ 2026-05-15 04:59 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: John Naylor <[email protected]>; Postgres hackers <[email protected]> On Thu, May 14, 2026 at 10:49:38PM -0400, Tom Lane wrote: > John Naylor <[email protected]> writes: >> FWIW, I tried to reproduce with the former new tests un-reverted, and >> didn't see stack overflow on the following, so unless I fat-fingered >> that I wonder if there's something more specific on the previously >> failing members: > >> ppc64le / gcc 8.5 / Linux kernel 4.18 >> S390X / gcc 13.3 / Linux kernel 6.8 > > Hm, did you use -O0 ? Yeah, that should matter. I don't immediately see why the new tests should fail at hand.. And unfortunately I don't have these environments at hand to double-check things, so I think that I am going to take a bet on HEAD. Then if things work, do a backpatch. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 06:33 John Naylor <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: John Naylor @ 2026-05-15 06:33 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Postgres hackers <[email protected]> On Fri, May 15, 2026 at 9:49 AM Tom Lane <[email protected]> wrote: > > John Naylor <[email protected]> writes: > > FWIW, I tried to reproduce with the former new tests un-reverted, and > > didn't see stack overflow on the following, so unless I fat-fingered > > that I wonder if there's something more specific on the previously > > failing members: > > > ppc64le / gcc 8.5 / Linux kernel 4.18 > > S390X / gcc 13.3 / Linux kernel 6.8 > > Hm, did you use -O0 ? I just now tried -O0 on yesterday's master with ppc64le, with the previous new tests re-added, and it did fail. Then, pulled in master with the tests just now committed, and it passed. -- John Naylor Amazon Web Services ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re-add recently-removed tests for ltree and intarray @ 2026-05-15 06:54 Michael Paquier <[email protected]> parent: John Naylor <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Michael Paquier @ 2026-05-15 06:54 UTC (permalink / raw) To: John Naylor <[email protected]>; +Cc: Tom Lane <[email protected]>; Postgres hackers <[email protected]> On Fri, May 15, 2026 at 01:33:53PM +0700, John Naylor wrote: > I just now tried -O0 on yesterday's master with ppc64le, with the > previous new tests re-added, and it did fail. Then, pulled in master > with the tests just now committed, and it passed. イエイ。 Thanks for checking. 3 animals running on ppc have currently passed with the new tests on HEAD, all passing. They are not the ones that have failed previously, so I'm still holding a bit longer.. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-05-15 06:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-14 23:38 Re-add recently-removed tests for ltree and intarray Michael Paquier <[email protected]> 2026-05-15 02:09 ` Tom Lane <[email protected]> 2026-05-15 02:47 ` John Naylor <[email protected]> 2026-05-15 02:49 ` Tom Lane <[email protected]> 2026-05-15 04:59 ` Michael Paquier <[email protected]> 2026-05-15 06:33 ` John Naylor <[email protected]> 2026-05-15 06:54 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox