pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
14+ messages / 4 participants
[nested] [flat]
* BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
@ 2025-04-23 04:48 PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: PG Bug reporting form @ 2025-04-23 04:48 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: n.kalinin@postgrespro.ru
The following bug has been logged on the website:
Bug reference: 18902
Logged by: Nikita Kalinin
Email address: n.kalinin@postgrespro.ru
PostgreSQL version: 17.4
Operating system: ubuntu 22.04
Description:
Hello!
After I built PostgreSQL like this:
`./configure --enable-tap-tests --enable-debug --with-openssl
--enable-cassert --prefix=/tmp/pg && make -j8 && make install`
And executed the following script:
```
CREATE EXTENSION postgres_fdw;
CREATE SERVER testserver1 FOREIGN DATA WRAPPER postgres_fdw;
DO $d$
BEGIN
EXECUTE $$CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (dbname '$$||current_database()||$$',
port '$$||current_setting('port')||$$'
)$$;
EXECUTE $$CREATE SERVER loopback2 FOREIGN DATA WRAPPER
postgres_fdw
OPTIONS (dbname '$$||current_database()||$$',
port '$$||current_setting('port')||$$'
)$$;
EXECUTE $$CREATE SERVER loopback3 FOREIGN DATA WRAPPER
postgres_fdw
OPTIONS (dbname '$$||current_database()||$$',
port '$$||current_setting('port')||$$'
)$$;
END;
$d$;
CREATE USER MAPPING FOR public SERVER testserver1
OPTIONS (user 'value', password 'value');
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback2;
CREATE USER MAPPING FOR public SERVER loopback3;
CREATE TYPE user_enum AS ENUM ('foo', 'bar', 'buz');
CREATE FOREIGN TABLE ft1 (
c0 int,
c1 int NOT NULL,
c2 int NOT NULL,
c3 text,
c4 timestamptz,
c5 timestamp,
c6 varchar(10),
c7 char(10) default 'ft1',
c8 user_enum
) SERVER loopback;
ALTER FOREIGN TABLE ft1 DROP COLUMN c0;
CREATE FOREIGN TABLE ft2 (
c1 int NOT NULL,
c2 int NOT NULL,
cx int,
c3 text,
c4 timestamptz,
c5 timestamp,
c6 varchar(10),
c7 char(10) default 'ft2',
c8 user_enum
) SERVER loopback;
ALTER FOREIGN TABLE ft2 DROP COLUMN cx;
CREATE FOREIGN TABLE ft4 (
c1 int NOT NULL,
c2 int NOT NULL,
c3 text
) SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 3');
CREATE FOREIGN TABLE ft5 (
c1 int NOT NULL,
c2 int NOT NULL,
c3 text
) SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 4');
CREATE TABLE local_tbl (c1 int NOT NULL, c2 int NOT NULL, c3 text,
CONSTRAINT local_tbl_pkey PRIMARY KEY (c1));
INSERT INTO local_tbl SELECT id, id % 10, to_char(id, 'FM0000') FROM
generate_series(1, 1000) id;
ANALYZE local_tbl;
SET enable_nestloop TO false;
SET enable_hashjoin TO false;
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2
= ft4.c1
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND
ft2.c1 < 100 FOR UPDATE;
```
I got a coredump:
```
Core was generated by `postgres: test postgres [local] EXPLAIN
'.
Program terminated with signal SIGABRT, Aborted.
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized
out>) at ./nptl/pthread_kill.c:44
warning: 44 ./nptl/pthread_kill.c: No such file or directory
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized
out>) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=<optimized out>) at
./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at
./nptl/pthread_kill.c:89
#3 0x000073517d44527e in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#4 0x000073517d4288ff in __GI_abort () at ./stdlib/abort.c:79
#5 0x0000603f0b24f1af in ExceptionalCondition
(conditionName=conditionName@entry=0x603f0b2cd558 "!is_sorted",
fileName=fileName@entry=0x603f0b2cdce3 "createplan.c",
lineNumber=lineNumber@entry=4538) at assert.c:66
#6 0x0000603f0aff7e88 in create_mergejoin_plan (best_path=0x603f3881caa8,
root=0x603f3871c738) at createplan.c:4538
#7 create_join_plan (best_path=0x603f3881caa8, root=0x603f3871c738) at
createplan.c:1089
#8 create_plan_recurse (root=0x603f3871c738, best_path=0x603f3881caa8,
flags=<optimized out>) at createplan.c:418
#9 0x0000603f0aff94f1 in create_foreignscan_plan (scan_clauses=0x0,
tlist=0x603f3882d938, best_path=0x603f3881cef8,
root=0x603f3871c738) at createplan.c:4128
#10 create_scan_plan (root=0x603f3871c738, best_path=0x603f3881cef8,
flags=<optimized out>) at createplan.c:787
#11 0x0000603f0aff6c73 in create_mergejoin_plan (best_path=0x603f388289e8,
root=0x603f3871c738) at createplan.c:4468
#12 create_join_plan (best_path=0x603f388289e8, root=0x603f3871c738) at
createplan.c:1089
#13 create_plan_recurse (root=0x603f3871c738, best_path=0x603f388289e8,
flags=<optimized out>) at createplan.c:418
#14 0x0000603f0aff7916 in create_projection_plan (flags=1,
best_path=0x603f3882c558, root=0x603f3871c738) at createplan.c:2056
#15 create_plan_recurse (root=0x603f3871c738, best_path=0x603f3882c558,
flags=1) at createplan.c:434
#16 0x0000603f0aff5a93 in create_lockrows_plan (flags=1,
best_path=0x603f3882c988, root=0x603f3871c738) at createplan.c:2792
#17 create_plan_recurse (root=0x603f3871c738, best_path=0x603f3882c988,
flags=1) at createplan.c:527
#18 0x0000603f0aff8529 in create_plan (root=root@entry=0x603f3871c738,
best_path=<optimized out>) at createplan.c:349
#19 0x0000603f0b009dfa in standard_planner (parse=0x603f387c0818,
query_string=<optimized out>, cursorOptions=2048,
boundParams=0x0) at planner.c:436
#20 0x0000603f0b00a4b5 in planner (parse=parse@entry=0x603f387c0818,
--Type <RET> for more, q to quit, c to continue without paging--c
query_string=query_string@entry=0x603f386c3000 "EXPLAIN (VERBOSE, COSTS
OFF)\nSELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND
ft1.c2 = ft4.c1\n AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND
ft1.c1 < 100 AND ft2.c1 < 100 F"...,
cursorOptions=cursorOptions@entry=2048,
boundParams=boundParams@entry=0x0) at planner.c:294
#21 0x0000603f0b0fd2cb in pg_plan_query
(querytree=querytree@entry=0x603f387c0818,
query_string=query_string@entry=0x603f386c3000 "EXPLAIN (VERBOSE, COSTS
OFF)\nSELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND
ft1.c2 = ft4.c1\n AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND
ft1.c1 < 100 AND ft2.c1 < 100 F"...,
cursorOptions=cursorOptions@entry=2048,
boundParams=boundParams@entry=0x0) at postgres.c:900
#22 0x0000603f0aea1a74 in standard_ExplainOneQuery (query=0x603f387c0818,
cursorOptions=2048, into=0x0, es=0x603f387d3308,
queryString=0x603f386c3000 "EXPLAIN (VERBOSE, COSTS OFF)\nSELECT * FROM
ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = ft4.c1\n
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft2.c1 <
100 F"..., params=0x0,
queryEnv=0x0) at explain.c:353
#23 0x0000603f0aea1c4c in ExplainOneQuery (query=<optimized out>,
cursorOptions=<optimized out>, into=<optimized out>,
es=<optimized out>, pstate=<optimized out>, params=<optimized out>) at
explain.c:309
#24 0x0000603f0aea1d6e in ExplainQuery (pstate=0x603f387e67c8,
stmt=0x603f387c0658, params=0x0, dest=0x603f387e6738)
at ../../../src/include/nodes/nodes.h:178
#25 0x0000603f0b10342b in standard_ProcessUtility (pstmt=0x603f387c0708,
queryString=0x603f386c3000 "EXPLAIN (VERBOSE, COSTS OFF)\nSELECT * FROM
ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = ft4.c1\n
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft2.c1 <
100 F"...,
readOnlyTree=<optimized out>, context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x603f387e6738,
qc=0x7fffa93ebd50) at utility.c:866
#26 0x0000603f0b1016df in PortalRunUtility
(portal=portal@entry=0x603f38742a90, pstmt=0x603f387c0708,
isTopLevel=true,
setHoldSnapshot=setHoldSnapshot@entry=true,
dest=dest@entry=0x603f387e6738, qc=qc@entry=0x7fffa93ebd50) at
pquery.c:1185
#27 0x0000603f0b101bd4 in FillPortalStore
(portal=portal@entry=0x603f38742a90, isTopLevel=isTopLevel@entry=true)
at ../../../src/include/nodes/nodes.h:178
#28 0x0000603f0b101f0d in PortalRun (portal=portal@entry=0x603f38742a90,
count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=true, dest=dest@entry=0x603f387f5da8,
altdest=altdest@entry=0x603f387f5da8,
qc=qc@entry=0x7fffa93ebf30) at pquery.c:792
#29 0x0000603f0b0fd826 in exec_simple_query (
query_string=0x603f386c3000 "EXPLAIN (VERBOSE, COSTS OFF)\nSELECT * FROM
ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = ft4.c1\n
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft2.c1 <
100 F"...) at postgres.c:1274
#30 0x0000603f0b0ff44a in PostgresMain (dbname=<optimized out>,
username=<optimized out>) at postgres.c:4771
#31 0x0000603f0b0f97e3 in BackendMain (startup_data=<optimized out>,
startup_data_len=<optimized out>) at backend_startup.c:124
#32 0x0000603f0b048fb2 in postmaster_child_launch (child_type=<optimized
out>, child_slot=1,
startup_data=startup_data@entry=0x7fffa93ec3e0,
startup_data_len=startup_data_len@entry=24,
client_sock=client_sock@entry=0x7fffa93ec400) at launch_backend.c:290
#33 0x0000603f0b04ce0e in BackendStartup (client_sock=0x7fffa93ec400) at
postmaster.c:3580
#34 ServerLoop () at postmaster.c:1702
#35 0x0000603f0b04e7ee in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x603f3867d130) at postmaster.c:1400
#36 0x0000603f0ad1d76e in main (argc=3, argv=0x603f3867d130) at main.c:227
```
logfile:
```
2025-04-23 04:36:23.099 UTC [162772] LOG: database system is ready to
accept connections
TRAP: failed Assert("!is_sorted"), File: "createplan.c", Line: 4538, PID:
162868
postgres: test postgres [local]
EXPLAIN(ExceptionalCondition+0x70)[0x603f0b24f190]
postgres: test postgres [local] EXPLAIN(+0x45de88)[0x603f0aff7e88]
postgres: test postgres [local] EXPLAIN(+0x45f4f1)[0x603f0aff94f1]
postgres: test postgres [local] EXPLAIN(+0x45cc73)[0x603f0aff6c73]
postgres: test postgres [local] EXPLAIN(+0x45d916)[0x603f0aff7916]
postgres: test postgres [local] EXPLAIN(+0x45ba93)[0x603f0aff5a93]
postgres: test postgres [local] EXPLAIN(create_plan+0x29)[0x603f0aff8529]
postgres: test postgres [local]
EXPLAIN(standard_planner+0x15a)[0x603f0b009dfa]
postgres: test postgres [local] EXPLAIN(planner+0x35)[0x603f0b00a4b5]
postgres: test postgres [local]
EXPLAIN(pg_plan_query+0x4b)[0x603f0b0fd2cb]
postgres: test postgres [local]
EXPLAIN(standard_ExplainOneQuery+0x124)[0x603f0aea1a74]
postgres: test postgres [local] EXPLAIN(+0x307c4c)[0x603f0aea1c4c]
postgres: test postgres [local] EXPLAIN(ExplainQuery+0xfe)[0x603f0aea1d6e]
postgres: test postgres [local]
EXPLAIN(standard_ProcessUtility+0x65b)[0x603f0b10342b]
postgres: test postgres [local] EXPLAIN(+0x5676df)[0x603f0b1016df]
postgres: test postgres [local] EXPLAIN(+0x567bd4)[0x603f0b101bd4]
postgres: test postgres [local] EXPLAIN(PortalRun+0x2dd)[0x603f0b101f0d]
postgres: test postgres [local] EXPLAIN(+0x563826)[0x603f0b0fd826]
postgres: test postgres [local]
EXPLAIN(PostgresMain+0x17da)[0x603f0b0ff44a]
postgres: test postgres [local] EXPLAIN(BackendMain+0x53)[0x603f0b0f97e3]
postgres: test postgres [local]
EXPLAIN(postmaster_child_launch+0x102)[0x603f0b048fb2]
postgres: test postgres [local] EXPLAIN(+0x4b2e0e)[0x603f0b04ce0e]
postgres: test postgres [local]
EXPLAIN(PostmasterMain+0xd3e)[0x603f0b04e7ee]
postgres: test postgres [local] EXPLAIN(main+0x1ce)[0x603f0ad1d76e]
/lib/x86_64-linux-gnu/libc.so.6(+0x2a1ca)[0x73517d42a1ca]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x8b)[0x73517d42a28b]
postgres: test postgres [local] EXPLAIN(_start+0x25)[0x603f0ad1daf5]
2025-04-23 04:36:35.570 UTC [162772] LOG: client backend (PID 162868) was
terminated by signal 6: Aborted
```
The commit where this was reproduced:
e0f373ee42a40a41bdfc025a1641d351580991c4
```
postgres=# select version();
version
----------------------------------------------------------------------------------------------------------
PostgreSQL 18devel on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
13.3.0-6ubuntu2~24.04) 13.3.0, 64-bit
(1 row)
```
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
@ 2025-04-23 11:03 ` Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Tender Wang @ 2025-04-23 11:03 UTC (permalink / raw)
To: n.kalinin@postgrespro.ru, pgsql-bugs@lists.postgresql.org, Richard Guo <guofenglinux@gmail.com>
PG Bug reporting form <noreply@postgresql.org> 于2025年4月23日周三 17:37写道：
> The following bug has been logged on the website:
>
> Bug reference: 18902
> Logged by: Nikita Kalinin
> Email address: n.kalinin@postgrespro.ru
> PostgreSQL version: 17.4
> Operating system: ubuntu 22.04
> Description:
>
> Hello!
> After I built PostgreSQL like this:
> `./configure --enable-tap-tests --enable-debug --with-openssl
> --enable-cassert --prefix=/tmp/pg && make -j8 && make install`
> And executed the following script:
> ```
> CREATE EXTENSION postgres_fdw;
>
> CREATE SERVER testserver1 FOREIGN DATA WRAPPER postgres_fdw;
> DO $d$
> BEGIN
> EXECUTE $$CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
> OPTIONS (dbname '$$||current_database()||$$',
> port '$$||current_setting('port')||$$'
> )$$;
> EXECUTE $$CREATE SERVER loopback2 FOREIGN DATA WRAPPER
> postgres_fdw
> OPTIONS (dbname '$$||current_database()||$$',
> port '$$||current_setting('port')||$$'
> )$$;
> EXECUTE $$CREATE SERVER loopback3 FOREIGN DATA WRAPPER
> postgres_fdw
> OPTIONS (dbname '$$||current_database()||$$',
> port '$$||current_setting('port')||$$'
> )$$;
> END;
> $d$;
>
> CREATE USER MAPPING FOR public SERVER testserver1
> OPTIONS (user 'value', password 'value');
> CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
> CREATE USER MAPPING FOR CURRENT_USER SERVER loopback2;
> CREATE USER MAPPING FOR public SERVER loopback3;
>
> CREATE TYPE user_enum AS ENUM ('foo', 'bar', 'buz');
> CREATE FOREIGN TABLE ft1 (
> c0 int,
> c1 int NOT NULL,
> c2 int NOT NULL,
> c3 text,
> c4 timestamptz,
> c5 timestamp,
> c6 varchar(10),
> c7 char(10) default 'ft1',
> c8 user_enum
> ) SERVER loopback;
> ALTER FOREIGN TABLE ft1 DROP COLUMN c0;
>
> CREATE FOREIGN TABLE ft2 (
> c1 int NOT NULL,
> c2 int NOT NULL,
> cx int,
> c3 text,
> c4 timestamptz,
> c5 timestamp,
> c6 varchar(10),
> c7 char(10) default 'ft2',
> c8 user_enum
> ) SERVER loopback;
> ALTER FOREIGN TABLE ft2 DROP COLUMN cx;
>
> CREATE FOREIGN TABLE ft4 (
> c1 int NOT NULL,
> c2 int NOT NULL,
> c3 text
> ) SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 3');
>
> CREATE FOREIGN TABLE ft5 (
> c1 int NOT NULL,
> c2 int NOT NULL,
> c3 text
> ) SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 4');
>
> CREATE TABLE local_tbl (c1 int NOT NULL, c2 int NOT NULL, c3 text,
> CONSTRAINT local_tbl_pkey PRIMARY KEY (c1));
> INSERT INTO local_tbl SELECT id, id % 10, to_char(id, 'FM0000') FROM
> generate_series(1, 1000) id;
> ANALYZE local_tbl;
> SET enable_nestloop TO false;
> SET enable_hashjoin TO false;
> EXPLAIN (VERBOSE, COSTS OFF)
> SELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND
> ft1.c2
> = ft4.c1
> AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND
> ft2.c1 < 100 FOR UPDATE;
> ```
>
Yes, I can reproduce this crash on HEAD. This crash is related with below
commit:
commit 828e94c9d2fd87c06a75354361543119d9937068
Author: Richard Guo <rguo@postgresql.org>
Date: Wed Oct 9 17:14:42 2024 +0900
Consider explicit incremental sort for mergejoins
I remove the Assert(!is_sorted), its plan looks like as below after adding
below codes:
if (presorted_keys > 0 && presorted_keys <
list_length(best_path->outersortkeys))
use_incremental_sort = true;
QUERY PLAN
---------------------------------------------------------------------------------------------
LockRows
-> Merge Join
Merge Cond: (local_tbl.c1 = ft1.c2)
-> Index Scan using local_tbl_pkey on local_tbl
-> Sort
Sort Key: ft1.c2
-> Foreign Scan
Relations: (((ft1) INNER JOIN (ft2)) INNER JOIN (ft4))
INNER JOIN (ft5)
-> Merge Join
Merge Cond: (ft1.c2 = ft5.c1)
-> Sort
Sort Key: ft1.c2
-> Merge Join
Merge Cond: (ft1.c2 = ft4.c1)
-> Sort
Sort Key: ft1.c2
-> Merge Join
Merge Cond: (ft1.c1 =
ft2.c1)
-> Sort
Sort Key: ft1.c1
-> Foreign Scan
on ft1
-> Sort
Sort Key: ft2.c1
-> Foreign Scan
on ft2
-> Sort
Sort Key: ft4.c1
-> Foreign Scan on ft4
-> Sort
Sort Key: ft5.c1
-> Foreign Scan on ft5
The crash happened in the second mergejoin, the outerpath is already
sorted by ft1.c2, and the best_path->outersortkeys contains ft1.c2,
so the pathkeys_count_contained_in() return true. How about below fix:
if (is_sorted)
use_incremental_sort = false;
else if (presorted_keys > 0)
use_incremental_sort = true;
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
@ 2025-04-23 11:47 ` Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Tender Wang @ 2025-04-23 11:47 UTC (permalink / raw)
To: n.kalinin@postgrespro.ru, pgsql-bugs@lists.postgresql.org, Richard Guo <guofenglinux@gmail.com>
Tender Wang <tndrwang@gmail.com> 于2025年4月23日周三 19:03写道：
>
> I remove the Assert(!is_sorted), its plan looks like as below after adding
> below codes:
> if (presorted_keys > 0 && presorted_keys <
> list_length(best_path->outersortkeys))
> use_incremental_sort = true;
>
> QUERY PLAN
>
>
> ---------------------------------------------------------------------------------------------
> LockRows
> -> Merge Join
> Merge Cond: (local_tbl.c1 = ft1.c2)
> -> Index Scan using local_tbl_pkey on local_tbl
> -> Sort
> Sort Key: ft1.c2
> -> Foreign Scan
> Relations: (((ft1) INNER JOIN (ft2)) INNER JOIN
> (ft4)) INNER JOIN (ft5)
> -> Merge Join
> Merge Cond: (ft1.c2 = ft5.c1)
> -> Sort
> Sort Key: ft1.c2
> -> Merge Join
> Merge Cond: (ft1.c2 = ft4.c1)
> -> Sort
> Sort Key: ft1.c2
> -> Merge Join
> Merge Cond: (ft1.c1 =
> ft2.c1)
> -> Sort
> Sort Key: ft1.c1
> -> Foreign Scan
> on ft1
> -> Sort
> Sort Key: ft2.c1
> -> Foreign Scan
> on ft2
> -> Sort
> Sort Key: ft4.c1
> -> Foreign Scan on ft4
> -> Sort
> Sort Key: ft5.c1
> -> Foreign Scan on ft5
>
If is_sorted is true, it means that it's already fully sorted, the Sort
node doesn't need any more.
I suspect something went wrong somewhere else. I didn't look into the
details.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
@ 2025-04-24 08:40 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guo @ 2025-04-24 08:40 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Wed, Apr 23, 2025 at 8:47 PM Tender Wang <tndrwang@gmail.com> wrote:
>> -> Merge Join
>> Merge Cond: (ft1.c2 = ft5.c1)
>> -> Sort
>> Sort Key: ft1.c2
>> -> Merge Join
>> Merge Cond: (ft1.c2 = ft4.c1)
>> -> Sort
>> Sort Key: ft1.c2
This plan seems problematic to me. The Sort node above the MergeJoin
of ft1/ft4 is redundant, as the output of the MergeJoin is already
ordered by ft1.c2.
> If is_sorted is true, it means that it's already fully sorted, the Sort node doesn't need any more.
> I suspect something went wrong somewhere else. I didn't look into the details.
Yeah. Quoting the comments for outersortkeys/innersortkeys:
* outersortkeys (resp. innersortkeys) is NIL if the outer path
* (resp. inner path) is already ordered appropriately for the
* mergejoin. If it is not NIL then it is a PathKeys list describing
* the ordering that must be created by an explicit Sort node.
And try_mergejoin_path will detect whether the outer path (resp. inner
path) is already well enough ordered, and suppresses an explicit sort
step if so by setting outersortkeys (resp. innersortkeys) to NIL.
This reflects a basic assumption: if MergePath.outersortkeys is not
NIL, it means the outer path is not sufficiently ordered.
Therefore, I think the "Assert(!is_sorted)" when outersortkeys is not
NIL is reasonable. And I think some other code is violating this
assumption.
I tried this repro query on v17 and got a plan with a redundant Sort
node. It seems that this issue existed prior to commit 828e94c9d, and
the Assert introduced in that commit makes the issue easier to detect.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-04-24 09:20 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 13:29 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Richard Guo @ 2025-04-24 09:20 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, Apr 24, 2025 at 5:40 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Yeah. Quoting the comments for outersortkeys/innersortkeys:
>
> * outersortkeys (resp. innersortkeys) is NIL if the outer path
> * (resp. inner path) is already ordered appropriately for the
> * mergejoin. If it is not NIL then it is a PathKeys list describing
> * the ordering that must be created by an explicit Sort node.
>
> And try_mergejoin_path will detect whether the outer path (resp. inner
> path) is already well enough ordered, and suppresses an explicit sort
> step if so by setting outersortkeys (resp. innersortkeys) to NIL.
>
> This reflects a basic assumption: if MergePath.outersortkeys is not
> NIL, it means the outer path is not sufficiently ordered.
>
> Therefore, I think the "Assert(!is_sorted)" when outersortkeys is not
> NIL is reasonable. And I think some other code is violating this
> assumption.
>
> I tried this repro query on v17 and got a plan with a redundant Sort
> node. It seems that this issue existed prior to commit 828e94c9d, and
> the Assert introduced in that commit makes the issue easier to detect.
It seems the culprit is in GetExistingLocalJoinPath(), which is used
to get a local path for EPQ checks. In that function, if the outer or
inner path of the selected join path is a ForeignScan, it is replaced
with the fdw_outerpath.
The problem arises when the selected join path is a MergePath, and its
outer or inner path is a ForeignScan that is not already well enough
ordered. In this case, the MergePath will have non-NIL outersortkeys
or innersortkeys. If we then replace the outer or inner path with the
corresponding fdw_outerpath, and that path is already sufficiently
ordered, we end up in an inconsistent state: the MergePath has non-NIL
outersortkeys/innersortkeys, and its input path is already properly
ordered.
This inconsistency leads to a redundant Sort node prior to commit
828e94c9d, and triggers an Assert failure after that commit.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-04-24 09:43 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Richard Guo @ 2025-04-24 09:43 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, Apr 24, 2025 at 6:20 PM Richard Guo <guofenglinux@gmail.com> wrote:
> On Thu, Apr 24, 2025 at 5:40 PM Richard Guo <guofenglinux@gmail.com> wrote:
> > Yeah. Quoting the comments for outersortkeys/innersortkeys:
> >
> > * outersortkeys (resp. innersortkeys) is NIL if the outer path
> > * (resp. inner path) is already ordered appropriately for the
> > * mergejoin. If it is not NIL then it is a PathKeys list describing
> > * the ordering that must be created by an explicit Sort node.
> >
> > And try_mergejoin_path will detect whether the outer path (resp. inner
> > path) is already well enough ordered, and suppresses an explicit sort
> > step if so by setting outersortkeys (resp. innersortkeys) to NIL.
> >
> > This reflects a basic assumption: if MergePath.outersortkeys is not
> > NIL, it means the outer path is not sufficiently ordered.
> >
> > Therefore, I think the "Assert(!is_sorted)" when outersortkeys is not
> > NIL is reasonable. And I think some other code is violating this
> > assumption.
> >
> > I tried this repro query on v17 and got a plan with a redundant Sort
> > node. It seems that this issue existed prior to commit 828e94c9d, and
> > the Assert introduced in that commit makes the issue easier to detect.
>
> It seems the culprit is in GetExistingLocalJoinPath(), which is used
> to get a local path for EPQ checks. In that function, if the outer or
> inner path of the selected join path is a ForeignScan, it is replaced
> with the fdw_outerpath.
>
> The problem arises when the selected join path is a MergePath, and its
> outer or inner path is a ForeignScan that is not already well enough
> ordered. In this case, the MergePath will have non-NIL outersortkeys
> or innersortkeys. If we then replace the outer or inner path with the
> corresponding fdw_outerpath, and that path is already sufficiently
> ordered, we end up in an inconsistent state: the MergePath has non-NIL
> outersortkeys/innersortkeys, and its input path is already properly
> ordered.
>
> This inconsistency leads to a redundant Sort node prior to commit
> 828e94c9d, and triggers an Assert failure after that commit.
To fix, I think we can set outersortkeys/innersortkeys to NIL in
GetExistingLocalJoinPath() if we detect that the new outer/inner path
of the MergePath is already sorted properly, similar to what we do in
try_mergejoin_path().
if (IS_JOIN_REL(foreign_path->path.parent))
+ {
joinpath->outerjoinpath = foreign_path->fdw_outerpath;
+
+ if (joinpath->path.pathtype == T_MergeJoin)
+ {
+ MergePath *merge_path = (MergePath *) joinpath;
+
+ /*
+ * If the new outer path is already well enough ordered for
+ * mergejoin, we can skip doing an explicit sort.
+ */
+ if (merge_path->outersortkeys &&
+ pathkeys_contained_in(merge_path->outersortkeys,
+ joinpath->outerjoinpath->pathkeys))
+ merge_path->outersortkeys = NIL;
+ }
+ }
(We need to do the same to innersortkeys.)
One problem with this approach is that the cost of the explicit sort
has already been included in the MergePath. However, I'm not too
concerned about this, since the resulting plan is only used for EPQ
checks, where cost accuracy is not that important. After all, we also
don't adjust the join path's cost when replacing its outer or inner
subpath.
Any thoughts?
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-04-24 10:58 ` David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2025-04-24 10:58 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, 24 Apr 2025 at 21:44, Richard Guo <guofenglinux@gmail.com> wrote:
> One problem with this approach is that the cost of the explicit sort
> has already been included in the MergePath. However, I'm not too
> concerned about this, since the resulting plan is only used for EPQ
> checks, where cost accuracy is not that important. After all, we also
> don't adjust the join path's cost when replacing its outer or inner
> subpath.
>
> Any thoughts?
One thing I noticed when looking at this is the number of redundant
pathkey checks we have now due to 828e94c9d.
In try_mergejoin_path(), we first call pathkeys_contained_in() on
either side of the join and nullify the pathkeys if they're presorted.
If they're not presorted then initial_cost_mergejoin() checks again by
calling pathkeys_count_contained_in() so that we can figure out the
sort costs. If that MergePath wins the path battle, then
create_mergejoin_plan() will check the pathkeys for the 3rd time with
pathkeys_count_contained_in() to figure out what type of sort needs to
happen.
I think MergePath needs to do a bit more caching of sorted-ness
information. How about making MergePath remember how many leading
PathKeys the inputs paths are sorted on. If we had that then 0 could
mean "not sorted" rather than a NIL MergePath.outersortkeys /
innersortkeys field. In fact, is there any point in having MergePath
outersortkeys and innersortkeys at all? Couldn't
create_mergejoin_plan() look at the outer_path->pathkeys for
make_sort_from_pathkeys/make_incrementalsort_from_pathkeys? We could
probably get rid of a few dozen lines of code by doing this and
there'd be less concern that something gets out of sync as everything
would just trust that the npresorted_outerkeys is correct. That would
be calculated in try_partial_mergejoin_path() before calling
initial_cost_mergejoin().
Looking at the calendar... it's still April. I think we should make
this better for v18. I don't see the sense in v18 being the only
version where it works this way.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
@ 2025-04-25 02:16 ` Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guo @ 2025-04-25 02:16 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, Apr 24, 2025 at 7:58 PM David Rowley <dgrowleyml@gmail.com> wrote:
> I think MergePath needs to do a bit more caching of sorted-ness
> information. How about making MergePath remember how many leading
> PathKeys the inputs paths are sorted on. If we had that then 0 could
> mean "not sorted" rather than a NIL MergePath.outersortkeys /
> innersortkeys field. In fact, is there any point in having MergePath
> outersortkeys and innersortkeys at all? Couldn't
> create_mergejoin_plan() look at the outer_path->pathkeys for
> make_sort_from_pathkeys/make_incrementalsort_from_pathkeys? We could
> probably get rid of a few dozen lines of code by doing this and
> there'd be less concern that something gets out of sync as everything
> would just trust that the npresorted_outerkeys is correct. That would
> be calculated in try_partial_mergejoin_path() before calling
> initial_cost_mergejoin().
Hmm, well, I don't think we can get rid of MergePath.outersortkeys and
innersortkeys, even if we've cached the number of presorted keys for
the outer and inner paths. These fields represent the desired
ordering to be produced by an explicit Sort node — not the current
ordering of the outer or inner path. Without this information, how
would create_mergejoin_plan know what sort order to generate for the
outer and inner paths if they are not already sorted?
Maybe we could cache the number of presorted keys for the outer and
inner paths in MergePath to save some pathkeys_count_contained_in
calls. But I'm not too sure it's worthwhile. The pathkeys are
canonical, and can be checked for equality by simple pointer
comparison. So it does not seem to cost too much. Besides, the
"redundant" pathkey checks actually helped uncover the bug we're
discussing here — didn't they?
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-04-27 00:14 ` David Rowley <dgrowleyml@gmail.com>
2025-05-02 07:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2025-04-27 00:14 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Fri, 25 Apr 2025 at 14:16, Richard Guo <guofenglinux@gmail.com> wrote:
> Hmm, well, I don't think we can get rid of MergePath.outersortkeys and
> innersortkeys, even if we've cached the number of presorted keys for
> the outer and inner paths. These fields represent the desired
> ordering to be produced by an explicit Sort node — not the current
> ordering of the outer or inner path. Without this information, how
> would create_mergejoin_plan know what sort order to generate for the
> outer and inner paths if they are not already sorted?
I misspoke about getting rid of the two List fields.
> Maybe we could cache the number of presorted keys for the outer and
> inner paths in MergePath to save some pathkeys_count_contained_in
> calls. But I'm not too sure it's worthwhile. The pathkeys are
> canonical, and can be checked for equality by simple pointer
> comparison. So it does not seem to cost too much. Besides, the
> "redundant" pathkey checks actually helped uncover the bug we're
> discussing here — didn't they?
I don't see any reason why we couldn't keep an Assert to ensure the
sorted-ness of the Path matches our expectations. Calculating the
whole thing again in non-assert builds seems a bit silly. The
previous code took care to avoid recalculations by nullifying the
Lists when no sort was required, you've not followed that with the
incremental sort changes, and to me, that makes it feel a little
half-done.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
@ 2025-05-02 07:03 ` Richard Guo <guofenglinux@gmail.com>
2025-05-03 01:50 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Richard Guo @ 2025-05-02 07:03 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Sun, Apr 27, 2025 at 9:14 AM David Rowley <dgrowleyml@gmail.com> wrote:
> On Fri, 25 Apr 2025 at 14:16, Richard Guo <guofenglinux@gmail.com> wrote:
> > Maybe we could cache the number of presorted keys for the outer and
> > inner paths in MergePath to save some pathkeys_count_contained_in
> > calls. But I'm not too sure it's worthwhile. The pathkeys are
> > canonical, and can be checked for equality by simple pointer
> > comparison. So it does not seem to cost too much. Besides, the
> > "redundant" pathkey checks actually helped uncover the bug we're
> > discussing here — didn't they?
> I don't see any reason why we couldn't keep an Assert to ensure the
> sorted-ness of the Path matches our expectations. Calculating the
> whole thing again in non-assert builds seems a bit silly. The
> previous code took care to avoid recalculations by nullifying the
> Lists when no sort was required, you've not followed that with the
> incremental sort changes, and to me, that makes it feel a little
> half-done.
Fair point. Here is the patchset doing that. 0001 fixes this bug by
setting outersortkeys/innersortkeys to NIL in GetExistingLocalJoinPath
if we detect that the new outer/inner path of the MergePath is already
sorted properly. 0002 caches the number of presorted keys of the
outer path in MergePath, allowing us to save several calls to
pathkeys_count_contained_in.
(I'm a bit hesitant about whether we should apply the same caching to
the inner path of a mergejoin. I chose not to do that in this patch,
since incremental sort is not used for the inner path of a mergejoin.)
Thanks
Richard
Attachments:
[application/octet-stream] v1-0001-Suppress-unnecessary-explicit-sorting-for-EPQ-mer.patch (12.6K, ../../CAMbWs4-e7j6ivVnmpFh8w=BKs7ZLPq-nWW8vCMQf_0-ba1mhAg@mail.gmail.com/2-v1-0001-Suppress-unnecessary-explicit-sorting-for-EPQ-mer.patch)
download | inline diff:
From dfcac43990d2e0f67ee4d06b1d5110fa4b056050 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 1 May 2025 20:50:03 +0900
Subject: [PATCH v1 1/2] Suppress unnecessary explicit sorting for EPQ
mergejoin path
When building a ForeignPath for a joinrel, if there's a possibility
that EvalPlanQual will be executed, we must identify a suitable path
for EPQ checks. If the outer or inner path of the chosen path is a
ForeignPath representing a pushed-down join, we replace it with its
fdw_outerpath to ensure that the EPQ check path consists entirely of
local joins.
If the chosen path is a MergePath, and its outer or inner path is a
ForeignPath that is not already well enough ordered, the MergePath
will have non-NIL outersortkeys or innersortkeys indicating the
desired ordering to be created by an explicit Sort node. If we then
replace the outer or inner path with its corresponding fdw_outerpath,
and that path is already sufficiently ordered, we end up in an
inconsistent state: the MergePath has non-NIL outersortkeys or
innersortkeys, and its input path is already properly ordered. This
inconsistency can result in an Assert failure or the addition of a
redundant Sort node.
To fix, check if the new outer or inner path of a MergePath is already
properly sorted, and set its outersortkeys or innersortkeys to NIL if
so.
---
.../postgres_fdw/expected/postgres_fdw.out | 64 +++++++++++++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 5 ++
src/backend/foreign/foreign.c | 33 ++++++++++
3 files changed, 102 insertions(+)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 24ff5f70cce..2185b42bb4f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2565,6 +2565,70 @@ SELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = f
96 | 6 | 00096 | Tue Apr 07 00:00:00 1970 PST | Tue Apr 07 00:00:00 1970 | 6 | 6 | foo | 96 | 6 | 00096 | Tue Apr 07 00:00:00 1970 PST | Tue Apr 07 00:00:00 1970 | 6 | 6 | foo | 6 | 7 | AAA006 | 6 | 7 | AAA006 | 6 | 6 | 0006
(10 rows)
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM ft1, ft4, ft5, local_tbl WHERE ft1.c1 = ft4.c1 AND ft1.c1 = ft5.c1
+ AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft5.c1 < 100 FOR UPDATE;
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ LockRows
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3, local_tbl.c1, local_tbl.c2, local_tbl.c3, ft1.*, ft4.*, ft5.*, local_tbl.ctid
+ -> Merge Join
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3, local_tbl.c1, local_tbl.c2, local_tbl.c3, ft1.*, ft4.*, ft5.*, local_tbl.ctid
+ Merge Cond: (local_tbl.c1 = ft1.c2)
+ -> Index Scan using local_tbl_pkey on public.local_tbl
+ Output: local_tbl.c1, local_tbl.c2, local_tbl.c3, local_tbl.ctid
+ -> Sort
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*, ft4.c1, ft4.c2, ft4.c3, ft4.*, ft5.c1, ft5.c2, ft5.c3, ft5.*
+ Sort Key: ft1.c2
+ -> Foreign Scan
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*, ft4.c1, ft4.c2, ft4.c3, ft4.*, ft5.c1, ft5.c2, ft5.c3, ft5.*
+ Relations: ((public.ft1) INNER JOIN (public.ft4)) INNER JOIN (public.ft5)
+ Remote SQL: SELECT r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, r2.c1, r2.c2, r2.c3, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2.c1, r2.c2, r2.c3) END, r3.c1, r3.c2, r3.c3, CASE WHEN (r3.*)::text IS NOT NULL THEN ROW(r3.c1, r3.c2, r3.c3) END FROM (("S 1"."T 1" r1 INNER JOIN "S 1"."T 3" r2 ON (((r1."C 1" = r2.c1)) AND ((r1."C 1" < 100)))) INNER JOIN "S 1"."T 4" r3 ON (((r1."C 1" = r3.c1)) AND ((r3.c1 < 100)))) FOR UPDATE OF r1 FOR UPDATE OF r2 FOR UPDATE OF r3
+ -> Merge Join
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*, ft4.c1, ft4.c2, ft4.c3, ft4.*, ft5.c1, ft5.c2, ft5.c3, ft5.*
+ Merge Cond: (ft1.c1 = ft5.c1)
+ -> Merge Join
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*, ft4.c1, ft4.c2, ft4.c3, ft4.*
+ Merge Cond: (ft1.c1 = ft4.c1)
+ -> Sort
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*
+ Sort Key: ft1.c1
+ -> Foreign Scan on public.ft1
+ Output: ft1.c1, ft1.c2, ft1.c3, ft1.c4, ft1.c5, ft1.c6, ft1.c7, ft1.c8, ft1.*
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" < 100)) FOR UPDATE
+ -> Sort
+ Output: ft4.c1, ft4.c2, ft4.c3, ft4.*
+ Sort Key: ft4.c1
+ -> Foreign Scan on public.ft4
+ Output: ft4.c1, ft4.c2, ft4.c3, ft4.*
+ Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 3" FOR UPDATE
+ -> Sort
+ Output: ft5.c1, ft5.c2, ft5.c3, ft5.*
+ Sort Key: ft5.c1
+ -> Foreign Scan on public.ft5
+ Output: ft5.c1, ft5.c2, ft5.c3, ft5.*
+ Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 4" WHERE ((c1 < 100)) FOR UPDATE
+(38 rows)
+
+SELECT * FROM ft1, ft4, ft5, local_tbl WHERE ft1.c1 = ft4.c1 AND ft1.c1 = ft5.c1
+ AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft5.c1 < 100 FOR UPDATE;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c1 | c2 | c3 | c1 | c2 | c3 | c1 | c2 | c3
+----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+--------+----+----+--------+----+----+------
+ 12 | 2 | 00012 | Tue Jan 13 00:00:00 1970 PST | Tue Jan 13 00:00:00 1970 | 2 | 2 | foo | 12 | 13 | AAA012 | 12 | 13 | AAA012 | 2 | 2 | 0002
+ 42 | 2 | 00042 | Thu Feb 12 00:00:00 1970 PST | Thu Feb 12 00:00:00 1970 | 2 | 2 | foo | 42 | 43 | AAA042 | 42 | 43 | AAA042 | 2 | 2 | 0002
+ 72 | 2 | 00072 | Sat Mar 14 00:00:00 1970 PST | Sat Mar 14 00:00:00 1970 | 2 | 2 | foo | 72 | 73 | AAA072 | 72 | 73 | | 2 | 2 | 0002
+ 24 | 4 | 00024 | Sun Jan 25 00:00:00 1970 PST | Sun Jan 25 00:00:00 1970 | 4 | 4 | foo | 24 | 25 | AAA024 | 24 | 25 | AAA024 | 4 | 4 | 0004
+ 54 | 4 | 00054 | Tue Feb 24 00:00:00 1970 PST | Tue Feb 24 00:00:00 1970 | 4 | 4 | foo | 54 | 55 | AAA054 | 54 | 55 | | 4 | 4 | 0004
+ 84 | 4 | 00084 | Thu Mar 26 00:00:00 1970 PST | Thu Mar 26 00:00:00 1970 | 4 | 4 | foo | 84 | 85 | AAA084 | 84 | 85 | AAA084 | 4 | 4 | 0004
+ 96 | 6 | 00096 | Tue Apr 07 00:00:00 1970 PST | Tue Apr 07 00:00:00 1970 | 6 | 6 | foo | 96 | 97 | AAA096 | 96 | 97 | AAA096 | 6 | 6 | 0006
+ 36 | 6 | 00036 | Fri Feb 06 00:00:00 1970 PST | Fri Feb 06 00:00:00 1970 | 6 | 6 | foo | 36 | 37 | AAA036 | 36 | 37 | | 6 | 6 | 0006
+ 66 | 6 | 00066 | Sun Mar 08 00:00:00 1970 PST | Sun Mar 08 00:00:00 1970 | 6 | 6 | foo | 66 | 67 | AAA066 | 66 | 67 | AAA066 | 6 | 6 | 0006
+ 6 | 6 | 00006 | Wed Jan 07 00:00:00 1970 PST | Wed Jan 07 00:00:00 1970 | 6 | 6 | foo | 6 | 7 | AAA006 | 6 | 7 | AAA006 | 6 | 6 | 0006
+ 48 | 8 | 00048 | Wed Feb 18 00:00:00 1970 PST | Wed Feb 18 00:00:00 1970 | 8 | 8 | foo | 48 | 49 | AAA048 | 48 | 49 | AAA048 | 8 | 8 | 0008
+ 18 | 8 | 00018 | Mon Jan 19 00:00:00 1970 PST | Mon Jan 19 00:00:00 1970 | 8 | 8 | foo | 18 | 19 | AAA018 | 18 | 19 | | 8 | 8 | 0008
+ 78 | 8 | 00078 | Fri Mar 20 00:00:00 1970 PST | Fri Mar 20 00:00:00 1970 | 8 | 8 | foo | 78 | 79 | AAA078 | 78 | 79 | AAA078 | 8 | 8 | 0008
+(13 rows)
+
RESET enable_nestloop;
RESET enable_hashjoin;
-- test that add_paths_with_pathkeys_for_rel() arranges for the epq_path to
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 1f27260bafe..e534b40de3c 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -715,6 +715,11 @@ SELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = f
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft2.c1 < 100 FOR UPDATE;
SELECT * FROM ft1, ft2, ft4, ft5, local_tbl WHERE ft1.c1 = ft2.c1 AND ft1.c2 = ft4.c1
AND ft1.c2 = ft5.c1 AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft2.c1 < 100 FOR UPDATE;
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM ft1, ft4, ft5, local_tbl WHERE ft1.c1 = ft4.c1 AND ft1.c1 = ft5.c1
+ AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft5.c1 < 100 FOR UPDATE;
+SELECT * FROM ft1, ft4, ft5, local_tbl WHERE ft1.c1 = ft4.c1 AND ft1.c1 = ft5.c1
+ AND ft1.c2 = local_tbl.c1 AND ft1.c1 < 100 AND ft5.c1 < 100 FOR UPDATE;
RESET enable_nestloop;
RESET enable_hashjoin;
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index f0835fc3070..9c57f7a259c 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -22,6 +22,7 @@
#include "foreign/foreign.h"
#include "funcapi.h"
#include "miscadmin.h"
+#include "optimizer/paths.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
@@ -808,7 +809,23 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
foreign_path = (ForeignPath *) joinpath->outerjoinpath;
if (IS_JOIN_REL(foreign_path->path.parent))
+ {
joinpath->outerjoinpath = foreign_path->fdw_outerpath;
+
+ if (joinpath->path.pathtype == T_MergeJoin)
+ {
+ MergePath *merge_path = (MergePath *) joinpath;
+
+ /*
+ * If the new outer path is already well enough ordered
+ * for the mergejoin, we can skip doing an explicit sort.
+ */
+ if (merge_path->outersortkeys &&
+ pathkeys_contained_in(merge_path->outersortkeys,
+ joinpath->outerjoinpath->pathkeys))
+ merge_path->outersortkeys = NIL;
+ }
+ }
}
if (IsA(joinpath->innerjoinpath, ForeignPath))
@@ -817,7 +834,23 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
foreign_path = (ForeignPath *) joinpath->innerjoinpath;
if (IS_JOIN_REL(foreign_path->path.parent))
+ {
joinpath->innerjoinpath = foreign_path->fdw_outerpath;
+
+ if (joinpath->path.pathtype == T_MergeJoin)
+ {
+ MergePath *merge_path = (MergePath *) joinpath;
+
+ /*
+ * If the new inner path is already well enough ordered
+ * for the mergejoin, we can skip doing an explicit sort.
+ */
+ if (merge_path->innersortkeys &&
+ pathkeys_contained_in(merge_path->innersortkeys,
+ joinpath->innerjoinpath->pathkeys))
+ merge_path->innersortkeys = NIL;
+ }
+ }
}
return (Path *) joinpath;
--
2.43.0
[application/octet-stream] v1-0002-Track-the-number-of-presorted-outer-pathkeys-in-M.patch (15.2K, ../../CAMbWs4-e7j6ivVnmpFh8w=BKs7ZLPq-nWW8vCMQf_0-ba1mhAg@mail.gmail.com/3-v1-0002-Track-the-number-of-presorted-outer-pathkeys-in-M.patch)
download | inline diff:
From 7600156863934b579fe9526f79d231993aa7d164 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Fri, 2 May 2025 11:26:44 +0900
Subject: [PATCH v1 2/2] Track the number of presorted outer pathkeys in
MergePath
When creating an explicit Sort node for the outer path of a mergejoin,
we need to determine the number of presorted keys of the outer path to
decide whether explicit incremental sort can be applied. Currently,
this is done by repeatedly calling pathkeys_count_contained_in.
This patch caches the number of presorted outer pathkeys in MergePath,
allowing us to save several calls to pathkeys_count_contained_in.
---
src/backend/foreign/foreign.c | 5 +-
src/backend/optimizer/path/costsize.c | 59 ++++++++++++------------
src/backend/optimizer/path/joinpath.c | 28 ++++++++++--
src/backend/optimizer/plan/createplan.c | 61 +++++++++++++------------
src/backend/optimizer/util/pathnode.c | 5 +-
src/include/nodes/pathnodes.h | 8 ++++
src/include/optimizer/cost.h | 1 +
src/include/optimizer/pathnode.h | 3 +-
8 files changed, 104 insertions(+), 66 deletions(-)
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 9c57f7a259c..a57e59f27ea 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -821,8 +821,9 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
* for the mergejoin, we can skip doing an explicit sort.
*/
if (merge_path->outersortkeys &&
- pathkeys_contained_in(merge_path->outersortkeys,
- joinpath->outerjoinpath->pathkeys))
+ pathkeys_count_contained_in(merge_path->outersortkeys,
+ joinpath->outerjoinpath->pathkeys,
+ &merge_path->outer_presorted_keys))
merge_path->outersortkeys = NIL;
}
}
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 60b0fcfb6be..3d44815ed5a 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -3542,6 +3542,7 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
* 'inner_path' is the inner input to the join
* 'outersortkeys' is the list of sort keys for the outer path
* 'innersortkeys' is the list of sort keys for the inner path
+ * 'outer_presorted_keys' is the number of presorted keys of the outer path
* 'extra' contains miscellaneous information about the join
*
* Note: outersortkeys and innersortkeys should be NIL if no explicit
@@ -3553,6 +3554,7 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace,
List *mergeclauses,
Path *outer_path, Path *inner_path,
List *outersortkeys, List *innersortkeys,
+ int outer_presorted_keys,
JoinPathExtraData *extra)
{
int disabled_nodes;
@@ -3683,27 +3685,33 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace,
if (outersortkeys) /* do we need to sort outer? */
{
- bool use_incremental_sort = false;
- int presorted_keys;
+ /*
+ * We can assert that the outer path is not already ordered
+ * appropriately for the mergejoin; otherwise, outersortkeys would
+ * have been set to NIL.
+ */
+ Assert(!pathkeys_contained_in(outersortkeys, outer_path->pathkeys));
/*
* We choose to use incremental sort if it is enabled and there are
* presorted keys; otherwise we use full sort.
*/
- if (enable_incremental_sort)
+ if (enable_incremental_sort && outer_presorted_keys > 0)
{
- bool is_sorted PG_USED_FOR_ASSERTS_ONLY;
-
- is_sorted = pathkeys_count_contained_in(outersortkeys,
- outer_path->pathkeys,
- &presorted_keys);
- Assert(!is_sorted);
-
- if (presorted_keys > 0)
- use_incremental_sort = true;
+ cost_incremental_sort(&sort_path,
+ root,
+ outersortkeys,
+ outer_presorted_keys,
+ outer_path->disabled_nodes,
+ outer_path->startup_cost,
+ outer_path->total_cost,
+ outer_path_rows,
+ outer_path->pathtarget->width,
+ 0.0,
+ work_mem,
+ -1.0);
}
-
- if (!use_incremental_sort)
+ else
{
cost_sort(&sort_path,
root,
@@ -3716,21 +3724,7 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace,
work_mem,
-1.0);
}
- else
- {
- cost_incremental_sort(&sort_path,
- root,
- outersortkeys,
- presorted_keys,
- outer_path->disabled_nodes,
- outer_path->startup_cost,
- outer_path->total_cost,
- outer_path_rows,
- outer_path->pathtarget->width,
- 0.0,
- work_mem,
- -1.0);
- }
+
disabled_nodes += sort_path.disabled_nodes;
startup_cost += sort_path.startup_cost;
startup_cost += (sort_path.total_cost - sort_path.startup_cost)
@@ -3750,6 +3744,13 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace,
if (innersortkeys) /* do we need to sort inner? */
{
+ /*
+ * We can assert that the inner path is not already ordered
+ * appropriately for the mergejoin; otherwise, innersortkeys would
+ * have been set to NIL.
+ */
+ Assert(!pathkeys_contained_in(innersortkeys, inner_path->pathkeys));
+
/*
* We do not consider incremental sort for inner path, because
* incremental sort does not support mark/restore.
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 14ed9daeca8..e6276e3fa93 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -1042,6 +1042,7 @@ try_mergejoin_path(PlannerInfo *root,
bool is_partial)
{
Relids required_outer;
+ int outer_presorted_keys;
JoinCostWorkspace workspace;
if (is_partial)
@@ -1087,9 +1088,16 @@ try_mergejoin_path(PlannerInfo *root,
/*
* If the given paths are already well enough ordered, we can skip doing
* an explicit sort.
+ *
+ * We need to determine the number of presorted keys of the outer path to
+ * decide whether explicit incremental sort can be applied when
+ * outersortkeys is not NIL. We do not need to do the same for the inner
+ * path though, as incremental sort currently does not support
+ * mark/restore.
*/
if (outersortkeys &&
- pathkeys_contained_in(outersortkeys, outer_path->pathkeys))
+ pathkeys_count_contained_in(outersortkeys, outer_path->pathkeys,
+ &outer_presorted_keys))
outersortkeys = NIL;
if (innersortkeys &&
pathkeys_contained_in(innersortkeys, inner_path->pathkeys))
@@ -1101,6 +1109,7 @@ try_mergejoin_path(PlannerInfo *root,
initial_cost_mergejoin(root, &workspace, jointype, mergeclauses,
outer_path, inner_path,
outersortkeys, innersortkeys,
+ outer_presorted_keys,
extra);
if (add_path_precheck(joinrel, workspace.disabled_nodes,
@@ -1120,7 +1129,8 @@ try_mergejoin_path(PlannerInfo *root,
required_outer,
mergeclauses,
outersortkeys,
- innersortkeys));
+ innersortkeys,
+ outer_presorted_keys));
}
else
{
@@ -1146,6 +1156,7 @@ try_partial_mergejoin_path(PlannerInfo *root,
JoinType jointype,
JoinPathExtraData *extra)
{
+ int outer_presorted_keys;
JoinCostWorkspace workspace;
/*
@@ -1159,9 +1170,16 @@ try_partial_mergejoin_path(PlannerInfo *root,
/*
* If the given paths are already well enough ordered, we can skip doing
* an explicit sort.
+ *
+ * We need to determine the number of presorted keys of the outer path to
+ * decide whether explicit incremental sort can be applied when
+ * outersortkeys is not NIL. We do not need to do the same for the inner
+ * path though, as incremental sort currently does not support
+ * mark/restore.
*/
if (outersortkeys &&
- pathkeys_contained_in(outersortkeys, outer_path->pathkeys))
+ pathkeys_count_contained_in(outersortkeys, outer_path->pathkeys,
+ &outer_presorted_keys))
outersortkeys = NIL;
if (innersortkeys &&
pathkeys_contained_in(innersortkeys, inner_path->pathkeys))
@@ -1173,6 +1191,7 @@ try_partial_mergejoin_path(PlannerInfo *root,
initial_cost_mergejoin(root, &workspace, jointype, mergeclauses,
outer_path, inner_path,
outersortkeys, innersortkeys,
+ outer_presorted_keys,
extra);
if (!add_partial_path_precheck(joinrel, workspace.disabled_nodes,
@@ -1193,7 +1212,8 @@ try_partial_mergejoin_path(PlannerInfo *root,
NULL,
mergeclauses,
outersortkeys,
- innersortkeys));
+ innersortkeys,
+ outer_presorted_keys));
}
/*
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index a8f22a8c154..4ad30b7627e 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4521,48 +4521,41 @@ create_mergejoin_plan(PlannerInfo *root,
{
Relids outer_relids = outer_path->parent->relids;
Plan *sort_plan;
- bool use_incremental_sort = false;
- int presorted_keys;
+
+ /*
+ * We can assert that the outer path is not already ordered
+ * appropriately for the mergejoin; otherwise, outersortkeys would
+ * have been set to NIL.
+ */
+ Assert(!pathkeys_contained_in(best_path->outersortkeys,
+ outer_path->pathkeys));
/*
* We choose to use incremental sort if it is enabled and there are
* presorted keys; otherwise we use full sort.
*/
- if (enable_incremental_sort)
- {
- bool is_sorted PG_USED_FOR_ASSERTS_ONLY;
-
- is_sorted = pathkeys_count_contained_in(best_path->outersortkeys,
- outer_path->pathkeys,
- &presorted_keys);
- Assert(!is_sorted);
-
- if (presorted_keys > 0)
- use_incremental_sort = true;
- }
-
- if (!use_incremental_sort)
- {
- sort_plan = (Plan *)
- make_sort_from_pathkeys(outer_plan,
- best_path->outersortkeys,
- outer_relids);
-
- label_sort_with_costsize(root, (Sort *) sort_plan, -1.0);
- }
- else
+ if (enable_incremental_sort && best_path->outer_presorted_keys > 0)
{
sort_plan = (Plan *)
make_incrementalsort_from_pathkeys(outer_plan,
best_path->outersortkeys,
outer_relids,
- presorted_keys);
+ best_path->outer_presorted_keys);
label_incrementalsort_with_costsize(root,
(IncrementalSort *) sort_plan,
best_path->outersortkeys,
-1.0);
}
+ else
+ {
+ sort_plan = (Plan *)
+ make_sort_from_pathkeys(outer_plan,
+ best_path->outersortkeys,
+ outer_relids);
+
+ label_sort_with_costsize(root, (Sort *) sort_plan, -1.0);
+ }
outer_plan = sort_plan;
outerpathkeys = best_path->outersortkeys;
@@ -4578,9 +4571,19 @@ create_mergejoin_plan(PlannerInfo *root,
*/
Relids inner_relids = inner_path->parent->relids;
- Sort *sort = make_sort_from_pathkeys(inner_plan,
- best_path->innersortkeys,
- inner_relids);
+ Sort *sort;
+
+ /*
+ * We can assert that the inner path is not already ordered
+ * appropriately for the mergejoin; otherwise, innersortkeys would
+ * have been set to NIL.
+ */
+ Assert(!pathkeys_contained_in(best_path->innersortkeys,
+ inner_path->pathkeys));
+
+ sort = make_sort_from_pathkeys(inner_plan,
+ best_path->innersortkeys,
+ inner_relids);
label_sort_with_costsize(root, sort, -1.0);
inner_plan = (Plan *) sort;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..e0192d4a491 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2626,6 +2626,7 @@ create_nestloop_path(PlannerInfo *root,
* (this should be a subset of the restrict_clauses list)
* 'outersortkeys' are the sort varkeys for the outer relation
* 'innersortkeys' are the sort varkeys for the inner relation
+ * 'outer_presorted_keys' is the number of presorted keys of the outer path
*/
MergePath *
create_mergejoin_path(PlannerInfo *root,
@@ -2640,7 +2641,8 @@ create_mergejoin_path(PlannerInfo *root,
Relids required_outer,
List *mergeclauses,
List *outersortkeys,
- List *innersortkeys)
+ List *innersortkeys,
+ int outer_presorted_keys)
{
MergePath *pathnode = makeNode(MergePath);
@@ -2669,6 +2671,7 @@ create_mergejoin_path(PlannerInfo *root,
pathnode->path_mergeclauses = mergeclauses;
pathnode->outersortkeys = outersortkeys;
pathnode->innersortkeys = innersortkeys;
+ pathnode->outer_presorted_keys = outer_presorted_keys;
/* pathnode->skip_mark_restore will be set by final_cost_mergejoin */
/* pathnode->materialize_inner will be set by final_cost_mergejoin */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 011e5a811c3..1dd2d1560cb 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2253,6 +2253,12 @@ typedef struct NestPath
* mergejoin. If it is not NIL then it is a PathKeys list describing
* the ordering that must be created by an explicit Sort node.
*
+ * outer_presorted_keys is the number of presorted keys of the outer
+ * path that match outersortkeys. It is used to determine whether
+ * explicit incremental sort can be applied when outersortkeys is not
+ * NIL. We do not track the number of presorted keys of the inner
+ * path, as incremental sort currently does not support mark/restore.
+ *
* skip_mark_restore is true if the executor need not do mark/restore calls.
* Mark/restore overhead is usually required, but can be skipped if we know
* that the executor need find only one match per outer tuple, and that the
@@ -2270,6 +2276,8 @@ typedef struct MergePath
List *path_mergeclauses; /* join clauses to be used for merge */
List *outersortkeys; /* keys for explicit sort, if any */
List *innersortkeys; /* keys for explicit sort, if any */
+ int outer_presorted_keys; /* number of presorted keys of the
+ * outer path */
bool skip_mark_restore; /* can executor skip mark/restore? */
bool materialize_inner; /* add Materialize to inner? */
} MergePath;
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index c5987440817..d397fe27dc1 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -160,6 +160,7 @@ extern void initial_cost_mergejoin(PlannerInfo *root,
List *mergeclauses,
Path *outer_path, Path *inner_path,
List *outersortkeys, List *innersortkeys,
+ int outer_presorted_keys,
JoinPathExtraData *extra);
extern void final_cost_mergejoin(PlannerInfo *root, MergePath *path,
JoinCostWorkspace *workspace,
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 719be3897f6..60dcdb77e41 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -179,7 +179,8 @@ extern MergePath *create_mergejoin_path(PlannerInfo *root,
Relids required_outer,
List *mergeclauses,
List *outersortkeys,
- List *innersortkeys);
+ List *innersortkeys,
+ int outer_presorted_keys);
extern HashPath *create_hashjoin_path(PlannerInfo *root,
RelOptInfo *joinrel,
--
2.43.0
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-05-02 07:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-05-03 01:50 ` Tender Wang <tndrwang@gmail.com>
2025-05-08 10:26 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-05-08 10:28 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Tender Wang @ 2025-05-03 01:50 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> 于2025年5月2日周五 15:03写道：
> On Sun, Apr 27, 2025 at 9:14 AM David Rowley <dgrowleyml@gmail.com> wrote:
> > On Fri, 25 Apr 2025 at 14:16, Richard Guo <guofenglinux@gmail.com>
> wrote:
> > > Maybe we could cache the number of presorted keys for the outer and
> > > inner paths in MergePath to save some pathkeys_count_contained_in
> > > calls. But I'm not too sure it's worthwhile. The pathkeys are
> > > canonical, and can be checked for equality by simple pointer
> > > comparison. So it does not seem to cost too much. Besides, the
> > > "redundant" pathkey checks actually helped uncover the bug we're
> > > discussing here — didn't they?
>
> > I don't see any reason why we couldn't keep an Assert to ensure the
> > sorted-ness of the Path matches our expectations. Calculating the
> > whole thing again in non-assert builds seems a bit silly. The
> > previous code took care to avoid recalculations by nullifying the
> > Lists when no sort was required, you've not followed that with the
> > incremental sort changes, and to me, that makes it feel a little
> > half-done.
>
> Fair point. Here is the patchset doing that. 0001 fixes this bug by
> setting outersortkeys/innersortkeys to NIL in GetExistingLocalJoinPath
> if we detect that the new outer/inner path of the MergePath is already
> sorted properly.
The 0001 fixing this bug looks good to me.
> 0002 caches the number of presorted keys of the
> outer path in MergePath, allowing us to save several calls to
> pathkeys_count_contained_in.
>
I quickly look through the 0002, it seems good to me.
>
> (I'm a bit hesitant about whether we should apply the same caching to
> the inner path of a mergejoin. I chose not to do that in this patch,
> since incremental sort is not used for the inner path of a mergejoin.)
>
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-05-02 07:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-05-03 01:50 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
@ 2025-05-08 10:26 ` Richard Guo <guofenglinux@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Richard Guo @ 2025-05-08 10:26 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Sat, May 3, 2025 at 10:50 AM Tender Wang <tndrwang@gmail.com> wrote:
> Richard Guo <guofenglinux@gmail.com> 于2025年5月2日周五 15:03写道：
>> Fair point. Here is the patchset doing that. 0001 fixes this bug by
>> setting outersortkeys/innersortkeys to NIL in GetExistingLocalJoinPath
>> if we detect that the new outer/inner path of the MergePath is already
>> sorted properly.
> The 0001 fixing this bug looks good to me.
I've pushed this fix to master. I'm not quite sure whether it should
be backpatched. In the back branches, the issue leads to the addition
of a redundant Sort node in the plan that is only used for EPQ checks.
Since usually the efficiency of the EPQ plan is not a concern, this
doesn't seem like a big issue. Besides, this issue has existed in
back branches for a long time but we haven't received any field
reports.
That said, if anyone thinks there's a good reason to backpatch it, I'm
happy to do so.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" David Rowley <dgrowleyml@gmail.com>
2025-05-02 07:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-05-03 01:50 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
@ 2025-05-08 10:28 ` Richard Guo <guofenglinux@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Richard Guo @ 2025-05-08 10:28 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Sat, May 3, 2025 at 10:50 AM Tender Wang <tndrwang@gmail.com> wrote:
> Richard Guo <guofenglinux@gmail.com> 于2025年5月2日周五 15:03写道：
>> 0002 caches the number of presorted keys of the
>> outer path in MergePath, allowing us to save several calls to
>> pathkeys_count_contained_in.
> I quickly look through the 0002, it seems good to me.
I've also pushed 0002. It's not a bug fix, but it can be considered a
complement to the work of 828e94c9d. So it seems reasonable to
include it in v18 at this stage.
Thanks
Richard
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c"
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Re: BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" Richard Guo <guofenglinux@gmail.com>
@ 2025-04-24 13:29 ` Tender Wang <tndrwang@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Tender Wang @ 2025-04-24 13:29 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: n.kalinin@postgrespro.ru; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> 于2025年4月24日周四 17:20写道：
> On Thu, Apr 24, 2025 at 5:40 PM Richard Guo <guofenglinux@gmail.com>
> wrote:
> > Yeah. Quoting the comments for outersortkeys/innersortkeys:
> >
> > * outersortkeys (resp. innersortkeys) is NIL if the outer path
> > * (resp. inner path) is already ordered appropriately for the
> > * mergejoin. If it is not NIL then it is a PathKeys list describing
> > * the ordering that must be created by an explicit Sort node.
> >
> > And try_mergejoin_path will detect whether the outer path (resp. inner
> > path) is already well enough ordered, and suppresses an explicit sort
> > step if so by setting outersortkeys (resp. innersortkeys) to NIL.
> >
> > This reflects a basic assumption: if MergePath.outersortkeys is not
> > NIL, it means the outer path is not sufficiently ordered.
> >
> > Therefore, I think the "Assert(!is_sorted)" when outersortkeys is not
> > NIL is reasonable. And I think some other code is violating this
> > assumption.
> >
> > I tried this repro query on v17 and got a plan with a redundant Sort
> > node. It seems that this issue existed prior to commit 828e94c9d, and
> > the Assert introduced in that commit makes the issue easier to detect.
>
Yes, I tested this repro query on v17.4 and got the same plan.
>
> It seems the culprit is in GetExistingLocalJoinPath(), which is used
> to get a local path for EPQ checks. In that function, if the outer or
> inner path of the selected join path is a ForeignScan, it is replaced
> with the fdw_outerpath.
>
> The problem arises when the selected join path is a MergePath, and its
> outer or inner path is a ForeignScan that is not already well enough
> ordered. In this case, the MergePath will have non-NIL outersortkeys
> or innersortkeys. If we then replace the outer or inner path with the
> corresponding fdw_outerpath, and that path is already sufficiently
> ordered, we end up in an inconsistent state: the MergePath has non-NIL
> outersortkeys/innersortkeys, and its input path is already properly
> ordered.
>
> This inconsistency leads to a redundant Sort node prior to commit
> 828e94c9d, and triggers an Assert failure after that commit.
>
Yes, your analysis is correct. The commit 828e94c9d brought the issue to
light.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2025-05-08 10:28 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-04-23 04:48 BUG #18902: TRAP:: failed Assert("!is_sorted") in File: "createplan.c" PG Bug reporting form <noreply@postgresql.org>
2025-04-23 11:03 ` Tender Wang <tndrwang@gmail.com>
2025-04-23 11:47 ` Tender Wang <tndrwang@gmail.com>
2025-04-24 08:40 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:20 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 09:43 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 10:58 ` David Rowley <dgrowleyml@gmail.com>
2025-04-25 02:16 ` Richard Guo <guofenglinux@gmail.com>
2025-04-27 00:14 ` David Rowley <dgrowleyml@gmail.com>
2025-05-02 07:03 ` Richard Guo <guofenglinux@gmail.com>
2025-05-03 01:50 ` Tender Wang <tndrwang@gmail.com>
2025-05-08 10:26 ` Richard Guo <guofenglinux@gmail.com>
2025-05-08 10:28 ` Richard Guo <guofenglinux@gmail.com>
2025-04-24 13:29 ` Tender Wang <tndrwang@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox