From 7d2a5be3127d121033ae9dcfb8f278d69affcb03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Mon, 19 Aug 2024 19:20:00 +0200 Subject: [PATCH 1/7] Add more test for target list indirections Adding similar indirection test to existing ones but using a sublink. i.e. INSERT INTO .... SELECT and UPDATE ... SELECT are added. Queries of the form: ``` ... SET (a, b) = (SELECT '1', '2') ... SET (b, a) = (SELECT '2', '1') ``` Should do the same thing, but currently the order of the attributes in `SET (...)` as rewriten for pushdown is only based on the physical ordering of the attributes in the relation. This leads to several subtle problems including situation where a DROPped then reADDed attributes will change its placement in the attribute list. There are maybe more tests to add in other situation where a SET (MULTIEXPR) is possible, though some alternatives are not supported yet by citus, for example: `(INSERT .. ON CONFLICT SET (a,b).....` --- .../regress/expected/distributed_types.out | 1 + .../regress/expected/multi_modifications.out | 30 +++++++++++++++++++ .../expected/multi_mx_modifications.out | 21 +++++++++++++ src/test/regress/sql/distributed_types.sql | 4 +++ src/test/regress/sql/multi_modifications.sql | 18 +++++++++++ .../regress/sql/multi_mx_modifications.sql | 15 ++++++++++ 6 files changed, 89 insertions(+) diff --git a/src/test/regress/expected/distributed_types.out b/src/test/regress/expected/distributed_types.out index 703614d61..68b952aae 100644 --- a/src/test/regress/expected/distributed_types.out +++ b/src/test/regress/expected/distributed_types.out @@ -444,6 +444,7 @@ HINT: Use the column name to insert or update the composite type as a single va UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value +HINT: Use the column name to insert or update the composite type as a single value -- below are supported as we don't do any field indirection INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) VALUES ('(1, "text1", 2)', 3, '(4, 5)'), ('(6, "text2", 7)', 8, '(9, 10)'); diff --git a/src/test/regress/expected/multi_modifications.out b/src/test/regress/expected/multi_modifications.out index 93f6c8c45..1a2c01a4d 100644 --- a/src/test/regress/expected/multi_modifications.out +++ b/src/test/regress/expected/multi_modifications.out @@ -330,6 +330,12 @@ UPDATE limit_orders SET (kind, limit_price) = ('buy', 999) WHERE id = 246 RETURN 246 | GM | 30 | Mon Jul 02 16:32:15 2007 | buy | 999 (1 row) +UPDATE limit_orders SET (kind, limit_price) = (SELECT 'buy'::order_side, 999) WHERE id = 246 RETURNING *; + id | symbol | bidder_id | placed_at | kind | limit_price +--------------------------------------------------------------------- + 246 | GM | 30 | Mon Jul 02 16:32:15 2007 | buy | 999 +(1 row) + -- Test that on unique contraint violations, we fail fast \set VERBOSITY terse INSERT INTO limit_orders VALUES (275, 'ADR', 140, '2007-07-02 16:32:15', 'sell', 43.67); @@ -435,6 +441,8 @@ UPDATE limit_orders SET limit_price = 0.00 FROM bidders -- should succeed with a CTE WITH deleted_orders AS (INSERT INTO limit_orders VALUES (399, 'PDR', 14, '2017-07-02 16:32:15', 'sell', 43)) UPDATE limit_orders SET symbol = 'GM'; +WITH deleted_orders AS (INSERT INTO limit_orders SELECT 400, 'PDR', 14, '2017-07-02 16:32:15', 'sell', 43) +UPDATE limit_orders SET symbol = 'GM'; SELECT symbol, bidder_id FROM limit_orders WHERE id = 246; symbol | bidder_id --------------------------------------------------------------------- @@ -927,6 +935,17 @@ SELECT * FROM summary_table ORDER BY id; 2 | 400 | 450.0000000000000000 | | (2 rows) +--- TODO this one is a silent corruption: +-- UPDATE summary_table SET (average_value, min_value) = +-- (SELECT avg(value), min(value) FROM raw_table WHERE id = 2) +-- WHERE id = 2; +SELECT * FROM summary_table ORDER BY id; + id | min_value | average_value | count | uniques +--------------------------------------------------------------------- + 1 | | 200.0000000000000000 | | + 2 | 400 | 450.0000000000000000 | | +(2 rows) + UPDATE summary_table SET min_value = 100 WHERE id IN (SELECT id FROM raw_table WHERE id = 1 and value > 100) AND id = 1; SELECT * FROM summary_table ORDER BY id; @@ -1103,6 +1122,17 @@ SELECT * FROM reference_summary_table ORDER BY id; 2 | 400 | 450.0000000000000000 | | (2 rows) +--- TODO this one is a silent corruption: +-- UPDATE reference_summary_table SET (average_value, min_value) = +-- (SELECT avg(value), min(value) FROM reference_raw_table WHERE id = 2) +-- WHERE id = 2; +SELECT * FROM reference_summary_table ORDER BY id; + id | min_value | average_value | count | uniques +--------------------------------------------------------------------- + 1 | | 200.0000000000000000 | | + 2 | 400 | 450.0000000000000000 | | +(2 rows) + -- no need partition colum equalities on reference tables UPDATE reference_summary_table SET (count) = (SELECT id AS inner_id FROM reference_raw_table WHERE value = 500) diff --git a/src/test/regress/expected/multi_mx_modifications.out b/src/test/regress/expected/multi_mx_modifications.out index 9e053d3f2..b401a67e3 100644 --- a/src/test/regress/expected/multi_mx_modifications.out +++ b/src/test/regress/expected/multi_mx_modifications.out @@ -91,6 +91,8 @@ SET client_min_messages TO DEFAULT; -- commands with non-constant partition values are unsupported INSERT INTO limit_orders_mx VALUES (random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', 'sell', 0.58); +INSERT INTO limit_orders_mx SELECT random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', + 'sell', 0.58; -- values for other columns are totally fine INSERT INTO limit_orders_mx VALUES (2036, 'GOOG', 5634, now(), 'buy', random()); -- commands with mutable functions in their quals @@ -103,6 +105,22 @@ DELETE FROM limit_orders_mx WHERE id = 246 AND placed_at = current_timestamp::ti INSERT INTO limit_orders_mx VALUES (2037, 'GOOG', 5634, now(), 'buy', random()), (2038, 'GOOG', 5634, now(), 'buy', random()), (2039, 'GOOG', 5634, now(), 'buy', random()); +SELECT count(distinct placed_at) FROM limit_orders_mx WHERE id BETWEEN 2037 AND 2039; + count +--------------------------------------------------------------------- + 1 +(1 row) + +INSERT INTO limit_orders_mx SELECT * FROM (VALUES + (2040, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random()), + (2041, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random()), + (2042, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random())); +SELECT count(distinct placed_at) FROM limit_orders_mx WHERE id BETWEEN 2040 AND 2042; + count +--------------------------------------------------------------------- + 1 +(1 row) + -- connect back to the other node \c - - - :worker_1_port -- commands containing a CTE are supported @@ -216,9 +234,12 @@ INSERT INTO limit_orders_mx VALUES (275, 'ADR', 140, '2007-07-02 16:32:15', 'sel ERROR: duplicate key value violates unique constraint "limit_orders_mx_pkey_1220093" -- multi shard update is supported UPDATE limit_orders_mx SET limit_price = 0.00; +UPDATE limit_orders_mx SET limit_price = (SELECT 0.00); -- attempting to change the partition key is unsupported UPDATE limit_orders_mx SET id = 0 WHERE id = 246; ERROR: modifying the partition value of rows is not allowed +UPDATE limit_orders_mx SET id = (SELECT 0) WHERE id = 246; +ERROR: modifying the partition value of rows is not allowed -- UPDATEs with a FROM clause are unsupported UPDATE limit_orders_mx SET limit_price = 0.00 FROM bidders WHERE limit_orders_mx.id = 246 AND diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index 205faf9c7..ac44457da 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -270,10 +270,14 @@ SELECT create_distributed_table('domain_indirection_test', 'f1'); -- not supported (field indirection to underlying composite type) INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; +-- INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; +-- INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; +-- ERROR: could not find a conversion path from type 23 to 17619 UPDATE domain_indirection_test SET domain_array[0].if2 = 5; UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); +UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); -- below are supported as we don't do any field indirection INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) diff --git a/src/test/regress/sql/multi_modifications.sql b/src/test/regress/sql/multi_modifications.sql index 2a00e7992..eeab5536f 100644 --- a/src/test/regress/sql/multi_modifications.sql +++ b/src/test/regress/sql/multi_modifications.sql @@ -234,6 +234,7 @@ SELECT kind, limit_price FROM limit_orders WHERE id = 246; -- multi-column UPDATE with RETURNING UPDATE limit_orders SET (kind, limit_price) = ('buy', 999) WHERE id = 246 RETURNING *; +UPDATE limit_orders SET (kind, limit_price) = (SELECT 'buy'::order_side, 999) WHERE id = 246 RETURNING *; -- Test that on unique contraint violations, we fail fast \set VERBOSITY terse @@ -337,6 +338,9 @@ UPDATE limit_orders SET limit_price = 0.00 FROM bidders WITH deleted_orders AS (INSERT INTO limit_orders VALUES (399, 'PDR', 14, '2017-07-02 16:32:15', 'sell', 43)) UPDATE limit_orders SET symbol = 'GM'; +WITH deleted_orders AS (INSERT INTO limit_orders SELECT 400, 'PDR', 14, '2017-07-02 16:32:15', 'sell', 43) +UPDATE limit_orders SET symbol = 'GM'; + SELECT symbol, bidder_id FROM limit_orders WHERE id = 246; -- updates referencing just a var are supported @@ -584,6 +588,13 @@ WHERE id = 2; SELECT * FROM summary_table ORDER BY id; +--- TODO this one is a silent corruption: +-- UPDATE summary_table SET (average_value, min_value) = +-- (SELECT avg(value), min(value) FROM raw_table WHERE id = 2) +-- WHERE id = 2; + +SELECT * FROM summary_table ORDER BY id; + UPDATE summary_table SET min_value = 100 WHERE id IN (SELECT id FROM raw_table WHERE id = 1 and value > 100) AND id = 1; @@ -712,6 +723,13 @@ WHERE id = 2; SELECT * FROM reference_summary_table ORDER BY id; +--- TODO this one is a silent corruption: +-- UPDATE reference_summary_table SET (average_value, min_value) = +-- (SELECT avg(value), min(value) FROM reference_raw_table WHERE id = 2) +-- WHERE id = 2; + +SELECT * FROM reference_summary_table ORDER BY id; + -- no need partition colum equalities on reference tables UPDATE reference_summary_table SET (count) = (SELECT id AS inner_id FROM reference_raw_table WHERE value = 500) diff --git a/src/test/regress/sql/multi_mx_modifications.sql b/src/test/regress/sql/multi_mx_modifications.sql index 852bf3a42..185fef451 100644 --- a/src/test/regress/sql/multi_mx_modifications.sql +++ b/src/test/regress/sql/multi_mx_modifications.sql @@ -67,6 +67,8 @@ SET client_min_messages TO DEFAULT; -- commands with non-constant partition values are unsupported INSERT INTO limit_orders_mx VALUES (random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', 'sell', 0.58); +INSERT INTO limit_orders_mx SELECT random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', + 'sell', 0.58; -- values for other columns are totally fine INSERT INTO limit_orders_mx VALUES (2036, 'GOOG', 5634, now(), 'buy', random()); @@ -83,6 +85,17 @@ INSERT INTO limit_orders_mx VALUES (2037, 'GOOG', 5634, now(), 'buy', random()), (2038, 'GOOG', 5634, now(), 'buy', random()), (2039, 'GOOG', 5634, now(), 'buy', random()); +SELECT count(distinct placed_at) FROM limit_orders_mx WHERE id BETWEEN 2037 AND 2039; + +INSERT INTO limit_orders_mx SELECT * FROM (VALUES + (2040, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random()), + (2041, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random()), + (2042, 'GOOG', 5634, now(), 'buy'::citus_mx_test_schema.order_side_mx, random())); + +SELECT count(distinct placed_at) FROM limit_orders_mx WHERE id BETWEEN 2040 AND 2042; + + + -- connect back to the other node \c - - - :worker_1_port @@ -153,9 +166,11 @@ INSERT INTO limit_orders_mx VALUES (275, 'ADR', 140, '2007-07-02 16:32:15', 'sel -- multi shard update is supported UPDATE limit_orders_mx SET limit_price = 0.00; +UPDATE limit_orders_mx SET limit_price = (SELECT 0.00); -- attempting to change the partition key is unsupported UPDATE limit_orders_mx SET id = 0 WHERE id = 246; +UPDATE limit_orders_mx SET id = (SELECT 0) WHERE id = 246; -- UPDATEs with a FROM clause are unsupported UPDATE limit_orders_mx SET limit_price = 0.00 FROM bidders From dfc26f9a2a6be14b22254be73ad9a3eca0c7aa12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Thu, 27 Feb 2025 05:05:12 +0100 Subject: [PATCH 2/7] Fix insert planner crash -- TODO crash Citus --- INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; +INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; +ERROR: cannot plan distributed INSERT INTO ... SELECT query +HINT: Do not use array references and field stores on the INSERT target list. INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value -- TODO here: not the expected ERROR -- INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; -- ERROR: could not find a conversion path from type 23 to 17619 UPDATE domain_indirection_test SET domain_array[0].if2 = 5; ERROR: inserting or modifying composite type fields is not supported HINT: Use the column name to insert or update the composite type as a single value -- TODO here: not the expected ERROR --- src/backend/distributed/planner/insert_select_planner.c | 5 +++++ src/test/regress/sql/distributed_types.sql | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index db5c3d4ff..f8f0b8b8c 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -1082,6 +1082,11 @@ ReorderInsertSelectTargetLists(Query *originalQuery, RangeTblEntry *insertRte, TargetEntry *newSubqueryTargetEntry = NULL; AttrNumber originalAttrNo = get_attnum(insertRelationId, oldInsertTargetEntry->resname); + Node *expr; + + /* we need to explore the underlying expression */ + expr = (Node *) oldInsertTargetEntry->expr; + expr = strip_implicit_coercions(expr); /* we need to explore the underlying expression */ Node *expr = strip_implicit_coercions((Node *) oldInsertTargetEntry->expr); diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index ac44457da..205faf9c7 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -270,14 +270,10 @@ SELECT create_distributed_table('domain_indirection_test', 'f1'); -- not supported (field indirection to underlying composite type) INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; --- INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) SELECT 0, 1, 2; INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; --- INSERT INTO domain_indirection_test (f1,f3.if1) SELECT 0, 1; --- ERROR: could not find a conversion path from type 23 to 17619 UPDATE domain_indirection_test SET domain_array[0].if2 = 5; UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); -UPDATE domain_indirection_test SET domain_array[0].if2 = (SELECT 5); -- below are supported as we don't do any field indirection INSERT INTO field_indirection_test_2 (ct2_col, int_col, ct1_col) From 7ceb49068e1395ed0ac3a5fc47517f10b094239f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Wed, 26 Feb 2025 16:04:45 +0100 Subject: [PATCH 3/7] WIP new test Add more testing for UPDATE with indirection It'll probably be interesting to add more test related to indirection here. --- src/test/regress/expected/indirections.out | 358 +++++++++++++++++++++ src/test/regress/sql/indirections.sql | 206 ++++++++++++ 2 files changed, 564 insertions(+) create mode 100644 src/test/regress/expected/indirections.out create mode 100644 src/test/regress/sql/indirections.sql diff --git a/src/test/regress/expected/indirections.out b/src/test/regress/expected/indirections.out new file mode 100644 index 000000000..a9ef5078a --- /dev/null +++ b/src/test/regress/expected/indirections.out @@ -0,0 +1,358 @@ +SET citus.shard_count TO 2; +SET citus.next_shard_id TO 750000; +SET citus.next_placement_id TO 750000; +CREATE SCHEMA indirections; +SET search_path TO indirections; +-- specific tests related to get_update_query_targetlist_def +-- we test only queries with sublinks, like: +-- ( ... SET (...) = (SELECT ...)) +-- Reference tables +CREATE TABLE test_ref_indirection ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_reference_table('indirections.test_ref_indirection'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE test_ref_indirection_new ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_reference_table('indirections.test_ref_indirection_new'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +-- Distributed tables +CREATE TABLE test_dist_indirection ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_distributed_table('indirections.test_dist_indirection', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE TABLE test_dist_indirection_new ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_distributed_table('indirections.test_dist_indirection_new', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Local tables required ? +-- those should work: +INSERT INTO test_ref_indirection (id, col_bool, col_date, col_int, col_text) + SELECT 1, true, '1970-01-01'::date, 1, 'one'; +INSERT INTO test_dist_indirection (id, col_bool, col_date, col_int, col_text) + SELECT 1, true, '1970-01-01'::date, 1, 'one'; +INSERT INTO test_ref_indirection (id, col_text, col_bool, col_date, col_int) + SELECT 2, 'two', false, '1970-01-01'::date, 2; +INSERT INTO test_dist_indirection (id, col_text, col_bool, col_date, col_int) + SELECT 2, 'two', false, '1970-01-01'::date, 2; +INSERT INTO test_ref_indirection SELECT 3, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_dist_indirection SELECT 3, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_ref_indirection SELECT 4, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_dist_indirection SELECT 4, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_ref_indirection_new SELECT * FROM test_ref_indirection; +INSERT INTO test_dist_indirection_new SELECT * FROM test_dist_indirection; +SELECT * FROM test_ref_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1970 | 1 | one + 2 | f | 01-01-1970 | 2 | two + 3 | f | 01-01-1970 | 0 | empty + 4 | f | 01-01-1970 | 0 | empty +(4 rows) + +SELECT * FROM test_dist_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1970 | 1 | one + 2 | f | 01-01-1970 | 2 | two + 3 | f | 01-01-1970 | 0 | empty + 4 | f | 01-01-1970 | 0 | empty +(4 rows) + +SELECT * FROM test_ref_indirection_new ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1970 | 1 | one + 2 | f | 01-01-1970 | 2 | two + 3 | f | 01-01-1970 | 0 | empty + 4 | f | 01-01-1970 | 0 | empty +(4 rows) + +SELECT * FROM test_dist_indirection_new ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1970 | 1 | one + 2 | f | 01-01-1970 | 2 | two + 3 | f | 01-01-1970 | 0 | empty + 4 | f | 01-01-1970 | 0 | empty +(4 rows) + +-- now UPDATEs +UPDATE test_ref_indirection + SET (col_bool, col_date, col_int, col_text) + = (SELECT true, '1970-12-31'::date, 1, 'ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 12-31-1970 | 1 | ok + 2 | t | 12-31-1970 | 1 | ok + 3 | t | 12-31-1970 | 1 | ok + 4 | t | 12-31-1970 | 1 | ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_bool, col_date, col_int, col_text) + = (SELECT true, '1970-12-31'::date, 1, 'ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 12-31-1970 | 1 | ok + 2 | t | 12-31-1970 | 1 | ok + 3 | t | 12-31-1970 | 1 | ok + 4 | t | 12-31-1970 | 1 | ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_bool, col_date) = (select true, '1970-06-06'::date) + , (col_int, col_text) = (select 1, 'still ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | still ok + 2 | t | 06-06-1970 | 1 | still ok + 3 | t | 06-06-1970 | 1 | still ok + 4 | t | 06-06-1970 | 1 | still ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_bool, col_date) = (select true, '1970-06-06'::date) + , (col_int, col_text) = (select 1, 'still ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | still ok + 2 | t | 06-06-1970 | 1 | still ok + 3 | t | 06-06-1970 | 1 | still ok + 4 | t | 06-06-1970 | 1 | still ok +(4 rows) + +SELECT * FROM test_ref_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | still ok + 2 | t | 06-06-1970 | 1 | still ok + 3 | t | 06-06-1970 | 1 | still ok + 4 | t | 06-06-1970 | 1 | still ok +(4 rows) + +SELECT * FROM test_dist_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | still ok + 2 | t | 06-06-1970 | 1 | still ok + 3 | t | 06-06-1970 | 1 | still ok + 4 | t | 06-06-1970 | 1 | still ok +(4 rows) + +-- but those should not: +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_date, col_text, col_int, col_bool) + = (SELECT '1970-12-31'::date, 'not ok', 2, false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type date +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_date, col_text, col_int, col_bool) + = (SELECT '1970-12-31'::date, 'not ok', 2, false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type date +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_int, col_date) = (select 2, '1970-06-06'::date) + , (col_text, col_bool) = (select 'not ok', false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type integer +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_int, col_date) = (select 2, '1970-06-06'::date) + , (col_text, col_bool) = (select 'not ok', false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type integer +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- +-- more restrictive ones, just in case we miss a wrong value +-- +-- those should work +UPDATE test_ref_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok + 2 | t | 06-06-1970 | 1 | ok + 3 | t | 06-06-1970 | 1 | ok + 4 | t | 06-06-1970 | 1 | ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok + 2 | t | 06-06-1970 | 1 | ok + 3 | t | 06-06-1970 | 1 | ok + 4 | t | 06-06-1970 | 1 | ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +WHERE id = 1 +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok +(1 row) + +UPDATE test_dist_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +WHERE id = 1 +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok +(1 row) + +SELECT * FROM test_ref_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok + 2 | t | 06-06-1970 | 1 | ok + 3 | t | 06-06-1970 | 1 | ok + 4 | t | 06-06-1970 | 1 | ok +(4 rows) + +SELECT * FROM test_dist_indirection ORDER BY id; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 06-06-1970 | 1 | ok + 2 | t | 06-06-1970 | 1 | ok + 3 | t | 06-06-1970 | 1 | ok + 4 | t | 06-06-1970 | 1 | ok +(4 rows) + +-- those should not +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +WHERE id = 2 +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +WHERE id = 2 +RETURNING *; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- several updates in CTE shoult not work +-- TODO wrong ERROR +with qq3 as ( + update test_ref_indirection + SET (col_text, col_bool) + = (SELECT 'full', true) + where id = 3 + returning * +), +qq4 as ( + update test_ref_indirection + SET (col_text, col_bool) + = (SELECT 'fully', true) + where id = 4 + returning * +) +select * from qq3 union all select * from qq4; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +-- TODO wrong ERROR +with qq3 as ( + update test_dist_indirection + SET (col_text, col_bool) + = (SELECT 'full', true) + where id = 3 + returning * +), +qq4 as ( + update test_dist_indirection + SET (col_text, col_bool) + = (SELECT 'fully', true) + where id = 4 + returning * +) +select * from qq3 union all select * from qq4; +ERROR: column "col_bool" is of type boolean but expression is of type text +HINT: You will need to rewrite or cast the expression. +CONTEXT: while executing command on localhost:xxxxx +DROP TABLE test_dist_indirection; +DROP TABLE test_dist_indirection_new; +DROP TABLE test_ref_indirection; +DROP TABLE test_ref_indirection_new; +-- https://github.com/citusdata/citus/issues/4092 +CREATE TABLE update_test ( + a INT DEFAULT 10, + b INT, + c TEXT +); +SELECT create_reference_table('indirections.update_test'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +UPDATE update_test +SET (b,a) = (select a,b from update_test where b = 41 and c = 'car') +WHERE a = 100 AND b = 20; +-- https://github.com/citusdata/citus/pull/5692 +DROP SCHEMA indirections CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table update_test +drop cascades to table update_test_750006 diff --git a/src/test/regress/sql/indirections.sql b/src/test/regress/sql/indirections.sql new file mode 100644 index 000000000..fd8119ff5 --- /dev/null +++ b/src/test/regress/sql/indirections.sql @@ -0,0 +1,206 @@ +SET citus.shard_count TO 2; +SET citus.next_shard_id TO 750000; +SET citus.next_placement_id TO 750000; + +CREATE SCHEMA indirections; +SET search_path TO indirections; + +-- specific tests related to get_update_query_targetlist_def +-- we test only queries with sublinks, like: +-- ( ... SET (...) = (SELECT ...)) + +-- Reference tables +CREATE TABLE test_ref_indirection ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_reference_table('indirections.test_ref_indirection'); + +CREATE TABLE test_ref_indirection_new ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_reference_table('indirections.test_ref_indirection_new'); + +-- Distributed tables +CREATE TABLE test_dist_indirection ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_distributed_table('indirections.test_dist_indirection', 'id'); + +CREATE TABLE test_dist_indirection_new ( + id bigint primary key + , col_bool bool , col_date date , col_int integer , col_text text + ); +SELECT create_distributed_table('indirections.test_dist_indirection_new', 'id'); + +-- Local tables required ? + +-- those should work: +INSERT INTO test_ref_indirection (id, col_bool, col_date, col_int, col_text) + SELECT 1, true, '1970-01-01'::date, 1, 'one'; +INSERT INTO test_dist_indirection (id, col_bool, col_date, col_int, col_text) + SELECT 1, true, '1970-01-01'::date, 1, 'one'; + +INSERT INTO test_ref_indirection (id, col_text, col_bool, col_date, col_int) + SELECT 2, 'two', false, '1970-01-01'::date, 2; +INSERT INTO test_dist_indirection (id, col_text, col_bool, col_date, col_int) + SELECT 2, 'two', false, '1970-01-01'::date, 2; + +INSERT INTO test_ref_indirection SELECT 3, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_dist_indirection SELECT 3, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_ref_indirection SELECT 4, false, '1970-01-01'::date, 0, 'empty'; +INSERT INTO test_dist_indirection SELECT 4, false, '1970-01-01'::date, 0, 'empty'; + +INSERT INTO test_ref_indirection_new SELECT * FROM test_ref_indirection; +INSERT INTO test_dist_indirection_new SELECT * FROM test_dist_indirection; + +SELECT * FROM test_ref_indirection ORDER BY id; +SELECT * FROM test_dist_indirection ORDER BY id; + +SELECT * FROM test_ref_indirection_new ORDER BY id; +SELECT * FROM test_dist_indirection_new ORDER BY id; + +-- now UPDATEs +UPDATE test_ref_indirection + SET (col_bool, col_date, col_int, col_text) + = (SELECT true, '1970-12-31'::date, 1, 'ok') +RETURNING *; +UPDATE test_dist_indirection + SET (col_bool, col_date, col_int, col_text) + = (SELECT true, '1970-12-31'::date, 1, 'ok') +RETURNING *; + +UPDATE test_ref_indirection + SET (col_bool, col_date) = (select true, '1970-06-06'::date) + , (col_int, col_text) = (select 1, 'still ok') +RETURNING *; +UPDATE test_dist_indirection + SET (col_bool, col_date) = (select true, '1970-06-06'::date) + , (col_int, col_text) = (select 1, 'still ok') +RETURNING *; + +SELECT * FROM test_ref_indirection ORDER BY id; +SELECT * FROM test_dist_indirection ORDER BY id; + +-- but those should not: +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_date, col_text, col_int, col_bool) + = (SELECT '1970-12-31'::date, 'not ok', 2, false) +RETURNING *; +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_date, col_text, col_int, col_bool) + = (SELECT '1970-12-31'::date, 'not ok', 2, false) +RETURNING *; + +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_int, col_date) = (select 2, '1970-06-06'::date) + , (col_text, col_bool) = (select 'not ok', false) +RETURNING *; +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_int, col_date) = (select 2, '1970-06-06'::date) + , (col_text, col_bool) = (select 'not ok', false) +RETURNING *; + +-- +-- more restrictive ones, just in case we miss a wrong value +-- +-- those should work +UPDATE test_ref_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +RETURNING *; +UPDATE test_dist_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +RETURNING *; + +UPDATE test_ref_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +WHERE id = 1 +RETURNING *; +UPDATE test_dist_indirection + SET (col_bool, col_text) = (SELECT true, 'ok') +WHERE id = 1 +RETURNING *; + +SELECT * FROM test_ref_indirection ORDER BY id; +SELECT * FROM test_dist_indirection ORDER BY id; + +-- those should not +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +RETURNING *; +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +RETURNING *; + +-- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +WHERE id = 2 +RETURNING *; +-- TODO wrong ERROR +UPDATE test_dist_indirection + SET (col_text, col_bool) = (SELECT 'not ok', false) +WHERE id = 2 +RETURNING *; + +-- several updates in CTE shoult not work +-- TODO wrong ERROR +with qq3 as ( + update test_ref_indirection + SET (col_text, col_bool) + = (SELECT 'full', true) + where id = 3 + returning * +), +qq4 as ( + update test_ref_indirection + SET (col_text, col_bool) + = (SELECT 'fully', true) + where id = 4 + returning * +) +select * from qq3 union all select * from qq4; +-- TODO wrong ERROR +with qq3 as ( + update test_dist_indirection + SET (col_text, col_bool) + = (SELECT 'full', true) + where id = 3 + returning * +), +qq4 as ( + update test_dist_indirection + SET (col_text, col_bool) + = (SELECT 'fully', true) + where id = 4 + returning * +) +select * from qq3 union all select * from qq4; + +DROP TABLE test_dist_indirection; +DROP TABLE test_dist_indirection_new; +DROP TABLE test_ref_indirection; +DROP TABLE test_ref_indirection_new; + +-- https://github.com/citusdata/citus/issues/4092 +CREATE TABLE update_test ( + a INT DEFAULT 10, + b INT, + c TEXT +); +SELECT create_reference_table('indirections.update_test'); +UPDATE update_test +SET (b,a) = (select a,b from update_test where b = 41 and c = 'car') +WHERE a = 100 AND b = 20; + +-- https://github.com/citusdata/citus/pull/5692 + +DROP SCHEMA indirections CASCADE; From 2a6b9ffd39408287e34e503bacbcfff107eacdc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Wed, 26 Feb 2025 10:09:47 +0100 Subject: [PATCH 4/7] Check more carrefuly UPDATE SET ()=(SELECT) query Produce an ERROR when attribute in the target list have been reordered by PostgreSQL rewritter. --- .../distributed/deparser/ruleutils_15.c | 40 ++++ .../distributed/deparser/ruleutils_16.c | 40 ++++ .../distributed/deparser/ruleutils_17.c | 40 ++++ src/test/regress/expected/indirections.out | 176 +++++++++--------- src/test/regress/sql/indirections.sql | 31 ++- 5 files changed, 223 insertions(+), 104 deletions(-) diff --git a/src/backend/distributed/deparser/ruleutils_15.c b/src/backend/distributed/deparser/ruleutils_15.c index 9004f7bbc..88db9662f 100644 --- a/src/backend/distributed/deparser/ruleutils_15.c +++ b/src/backend/distributed/deparser/ruleutils_15.c @@ -3509,6 +3509,9 @@ get_update_query_targetlist_def(Query *query, List *targetList, SubLink *cur_ma_sublink; List *ma_sublinks; + AttrNumber previous_attnum = InvalidAttrNumber; + int paramid_increment = 0; + /* * Prepare to deal with MULTIEXPR assignments: collect the source SubLinks * into a list. We expect them to appear, in ID order, in resjunk tlist @@ -3631,11 +3634,48 @@ get_update_query_targetlist_def(Query *query, List *targetList, */ if (cur_ma_sublink != NULL) { + AttrNumber attnum = InvalidAttrNumber; + if (IsA(expr, Param)) + { + Param *param = (Param *) expr; + attnum = param->paramid + paramid_increment; + } + else if (IsA(expr, FuncExpr)) + { + FuncExpr *func = (FuncExpr *) expr; + ListCell *lc; + + /* Iterate through the arguments of the FuncExpr */ + foreach(lc, func->args) + { + Node *arg = (Node *) lfirst(lc); + + /* Check if the argument is a PARAM node */ + if (IsA(arg, Param)) + { + Param *param = (Param *) arg; + attnum = param->paramid + paramid_increment; + + break; /* Exit loop once we find the PARAM node */ + } + } + } + + if (previous_attnum >= attnum) + ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg( + "cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order"), + errhint("Sort the columns on the left side by physical order."))); + + previous_attnum = attnum; + if (--remaining_ma_columns > 0) continue; /* not the last column of multiassignment */ + appendStringInfoChar(buf, ')'); expr = (Node *) cur_ma_sublink; cur_ma_sublink = NULL; + paramid_increment = previous_attnum; } appendStringInfoString(buf, " = "); diff --git a/src/backend/distributed/deparser/ruleutils_16.c b/src/backend/distributed/deparser/ruleutils_16.c index 65bbd1720..027fe3242 100644 --- a/src/backend/distributed/deparser/ruleutils_16.c +++ b/src/backend/distributed/deparser/ruleutils_16.c @@ -3525,6 +3525,9 @@ get_update_query_targetlist_def(Query *query, List *targetList, SubLink *cur_ma_sublink; List *ma_sublinks; + AttrNumber previous_attnum = InvalidAttrNumber; + int paramid_increment = 0; + /* * Prepare to deal with MULTIEXPR assignments: collect the source SubLinks * into a list. We expect them to appear, in ID order, in resjunk tlist @@ -3647,11 +3650,48 @@ get_update_query_targetlist_def(Query *query, List *targetList, */ if (cur_ma_sublink != NULL) { + AttrNumber attnum = InvalidAttrNumber; + if (IsA(expr, Param)) + { + Param *param = (Param *) expr; + attnum = param->paramid + paramid_increment; + } + else if (IsA(expr, FuncExpr)) + { + FuncExpr *func = (FuncExpr *) expr; + ListCell *lc; + + /* Iterate through the arguments of the FuncExpr */ + foreach(lc, func->args) + { + Node *arg = (Node *) lfirst(lc); + + /* Check if the argument is a PARAM node */ + if (IsA(arg, Param)) + { + Param *param = (Param *) arg; + attnum = param->paramid + paramid_increment; + + break; /* Exit loop once we find the PARAM node */ + } + } + } + + if (previous_attnum >= attnum) + ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg( + "cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order"), + errhint("Sort the columns on the left side by physical order."))); + + previous_attnum = attnum; + if (--remaining_ma_columns > 0) continue; /* not the last column of multiassignment */ + appendStringInfoChar(buf, ')'); expr = (Node *) cur_ma_sublink; cur_ma_sublink = NULL; + paramid_increment = previous_attnum; } appendStringInfoString(buf, " = "); diff --git a/src/backend/distributed/deparser/ruleutils_17.c b/src/backend/distributed/deparser/ruleutils_17.c index f0710e684..64ac077cb 100644 --- a/src/backend/distributed/deparser/ruleutils_17.c +++ b/src/backend/distributed/deparser/ruleutils_17.c @@ -3542,6 +3542,9 @@ get_update_query_targetlist_def(Query *query, List *targetList, SubLink *cur_ma_sublink; List *ma_sublinks; + AttrNumber previous_attnum = InvalidAttrNumber; + int paramid_increment = 0; + /* * Prepare to deal with MULTIEXPR assignments: collect the source SubLinks * into a list. We expect them to appear, in ID order, in resjunk tlist @@ -3664,11 +3667,48 @@ get_update_query_targetlist_def(Query *query, List *targetList, */ if (cur_ma_sublink != NULL) { + AttrNumber attnum = InvalidAttrNumber; + if (IsA(expr, Param)) + { + Param *param = (Param *) expr; + attnum = param->paramid + paramid_increment; + } + else if (IsA(expr, FuncExpr)) + { + FuncExpr *func = (FuncExpr *) expr; + ListCell *lc; + + /* Iterate through the arguments of the FuncExpr */ + foreach(lc, func->args) + { + Node *arg = (Node *) lfirst(lc); + + /* Check if the argument is a PARAM node */ + if (IsA(arg, Param)) + { + Param *param = (Param *) arg; + attnum = param->paramid + paramid_increment; + + break; /* Exit loop once we find the PARAM node */ + } + } + } + + if (previous_attnum >= attnum) + ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg( + "cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order"), + errhint("Sort the columns on the left side by physical order."))); + + previous_attnum = attnum; + if (--remaining_ma_columns > 0) continue; /* not the last column of multiassignment */ + appendStringInfoChar(buf, ')'); expr = (Node *) cur_ma_sublink; cur_ma_sublink = NULL; + paramid_increment = previous_attnum; } appendStringInfoString(buf, " = "); diff --git a/src/test/regress/expected/indirections.out b/src/test/regress/expected/indirections.out index a9ef5078a..20bd73d2b 100644 --- a/src/test/regress/expected/indirections.out +++ b/src/test/regress/expected/indirections.out @@ -103,103 +103,113 @@ SELECT * FROM test_dist_indirection_new ORDER BY id; -- now UPDATEs UPDATE test_ref_indirection SET (col_bool, col_date, col_int, col_text) - = (SELECT true, '1970-12-31'::date, 1, 'ok') + = (SELECT true, '1970-01-01'::date, 1, 'ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 12-31-1970 | 1 | ok - 2 | t | 12-31-1970 | 1 | ok - 3 | t | 12-31-1970 | 1 | ok - 4 | t | 12-31-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) UPDATE test_dist_indirection SET (col_bool, col_date, col_int, col_text) - = (SELECT true, '1970-12-31'::date, 1, 'ok') + = (SELECT true, '1970-01-01'::date, 1, 'ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 12-31-1970 | 1 | ok - 2 | t | 12-31-1970 | 1 | ok - 3 | t | 12-31-1970 | 1 | ok - 4 | t | 12-31-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) UPDATE test_ref_indirection - SET (col_bool, col_date) = (select true, '1970-06-06'::date) + SET (col_bool, col_date) = (select true, '1970-01-01'::date) , (col_int, col_text) = (select 1, 'still ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | still ok - 2 | t | 06-06-1970 | 1 | still ok - 3 | t | 06-06-1970 | 1 | still ok - 4 | t | 06-06-1970 | 1 | still ok + 1 | t | 01-01-1970 | 1 | still ok + 2 | t | 01-01-1970 | 1 | still ok + 3 | t | 01-01-1970 | 1 | still ok + 4 | t | 01-01-1970 | 1 | still ok (4 rows) UPDATE test_dist_indirection - SET (col_bool, col_date) = (select true, '1970-06-06'::date) + SET (col_bool, col_date) = (select true, '1970-01-01'::date) , (col_int, col_text) = (select 1, 'still ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | still ok - 2 | t | 06-06-1970 | 1 | still ok - 3 | t | 06-06-1970 | 1 | still ok - 4 | t | 06-06-1970 | 1 | still ok + 1 | t | 01-01-1970 | 1 | still ok + 2 | t | 01-01-1970 | 1 | still ok + 3 | t | 01-01-1970 | 1 | still ok + 4 | t | 01-01-1970 | 1 | still ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_bool, col_int) = (select true, 1) + , (col_text) = (select 'ok') +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) SELECT * FROM test_ref_indirection ORDER BY id; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | still ok - 2 | t | 06-06-1970 | 1 | still ok - 3 | t | 06-06-1970 | 1 | still ok - 4 | t | 06-06-1970 | 1 | still ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) SELECT * FROM test_dist_indirection ORDER BY id; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | still ok - 2 | t | 06-06-1970 | 1 | still ok - 3 | t | 06-06-1970 | 1 | still ok - 4 | t | 06-06-1970 | 1 | still ok + 1 | t | 01-01-1970 | 1 | still ok + 2 | t | 01-01-1970 | 1 | still ok + 3 | t | 01-01-1970 | 1 | still ok + 4 | t | 01-01-1970 | 1 | still ok (4 rows) -- but those should not: --- TODO wrong ERROR UPDATE test_ref_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-12-31'::date, 'not ok', 2, false) + = (SELECT '1970-06-06'::date, 'not ok', 2, false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type date -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_dist_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-12-31'::date, 'not ok', 2, false) + = (SELECT '1970-06-06'::date, 'not ok', 2, false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type date -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. +UPDATE test_ref_indirection + SET (col_int, col_text) = (select 2, 'not ok') + , (col_bool) = (select false) +RETURNING *; +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_ref_indirection SET (col_int, col_date) = (select 2, '1970-06-06'::date) , (col_text, col_bool) = (select 'not ok', false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type integer -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_dist_indirection SET (col_int, col_date) = (select 2, '1970-06-06'::date) , (col_text, col_bool) = (select 'not ok', false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type integer -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. -- -- more restrictive ones, just in case we miss a wrong value -- @@ -209,10 +219,10 @@ UPDATE test_ref_indirection RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok - 2 | t | 06-06-1970 | 1 | ok - 3 | t | 06-06-1970 | 1 | ok - 4 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) UPDATE test_dist_indirection @@ -220,10 +230,10 @@ UPDATE test_dist_indirection RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok - 2 | t | 06-06-1970 | 1 | ok - 3 | t | 06-06-1970 | 1 | ok - 4 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) UPDATE test_ref_indirection @@ -232,7 +242,7 @@ WHERE id = 1 RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok (1 row) UPDATE test_dist_indirection @@ -241,60 +251,51 @@ WHERE id = 1 RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok (1 row) SELECT * FROM test_ref_indirection ORDER BY id; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok - 2 | t | 06-06-1970 | 1 | ok - 3 | t | 06-06-1970 | 1 | ok - 4 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) SELECT * FROM test_dist_indirection ORDER BY id; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 06-06-1970 | 1 | ok - 2 | t | 06-06-1970 | 1 | ok - 3 | t | 06-06-1970 | 1 | ok - 4 | t | 06-06-1970 | 1 | ok + 1 | t | 01-01-1970 | 1 | ok + 2 | t | 01-01-1970 | 1 | ok + 3 | t | 01-01-1970 | 1 | ok + 4 | t | 01-01-1970 | 1 | ok (4 rows) -- those should not --- TODO wrong ERROR UPDATE test_ref_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_dist_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_ref_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) WHERE id = 2 RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. UPDATE test_dist_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) WHERE id = 2 RETURNING *; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. -- several updates in CTE shoult not work --- TODO wrong ERROR with qq3 as ( update test_ref_indirection SET (col_text, col_bool) @@ -310,10 +311,8 @@ qq4 as ( returning * ) select * from qq3 union all select * from qq4; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx --- TODO wrong ERROR +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. with qq3 as ( update test_dist_indirection SET (col_text, col_bool) @@ -329,9 +328,8 @@ qq4 as ( returning * ) select * from qq3 union all select * from qq4; -ERROR: column "col_bool" is of type boolean but expression is of type text -HINT: You will need to rewrite or cast the expression. -CONTEXT: while executing command on localhost:xxxxx +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. DROP TABLE test_dist_indirection; DROP TABLE test_dist_indirection_new; DROP TABLE test_ref_indirection; @@ -351,6 +349,8 @@ SELECT create_reference_table('indirections.update_test'); UPDATE update_test SET (b,a) = (select a,b from update_test where b = 41 and c = 'car') WHERE a = 100 AND b = 20; +ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order +HINT: Sort the columns on the left side by physical order. -- https://github.com/citusdata/citus/pull/5692 DROP SCHEMA indirections CASCADE; NOTICE: drop cascades to 2 other objects diff --git a/src/test/regress/sql/indirections.sql b/src/test/regress/sql/indirections.sql index fd8119ff5..73ff78b71 100644 --- a/src/test/regress/sql/indirections.sql +++ b/src/test/regress/sql/indirections.sql @@ -65,43 +65,48 @@ SELECT * FROM test_dist_indirection_new ORDER BY id; -- now UPDATEs UPDATE test_ref_indirection SET (col_bool, col_date, col_int, col_text) - = (SELECT true, '1970-12-31'::date, 1, 'ok') + = (SELECT true, '1970-01-01'::date, 1, 'ok') RETURNING *; UPDATE test_dist_indirection SET (col_bool, col_date, col_int, col_text) - = (SELECT true, '1970-12-31'::date, 1, 'ok') + = (SELECT true, '1970-01-01'::date, 1, 'ok') RETURNING *; UPDATE test_ref_indirection - SET (col_bool, col_date) = (select true, '1970-06-06'::date) + SET (col_bool, col_date) = (select true, '1970-01-01'::date) , (col_int, col_text) = (select 1, 'still ok') RETURNING *; UPDATE test_dist_indirection - SET (col_bool, col_date) = (select true, '1970-06-06'::date) + SET (col_bool, col_date) = (select true, '1970-01-01'::date) , (col_int, col_text) = (select 1, 'still ok') RETURNING *; +UPDATE test_ref_indirection + SET (col_bool, col_int) = (select true, 1) + , (col_text) = (select 'ok') +RETURNING *; + SELECT * FROM test_ref_indirection ORDER BY id; SELECT * FROM test_dist_indirection ORDER BY id; -- but those should not: --- TODO wrong ERROR UPDATE test_ref_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-12-31'::date, 'not ok', 2, false) + = (SELECT '1970-06-06'::date, 'not ok', 2, false) RETURNING *; --- TODO wrong ERROR UPDATE test_dist_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-12-31'::date, 'not ok', 2, false) + = (SELECT '1970-06-06'::date, 'not ok', 2, false) RETURNING *; --- TODO wrong ERROR +UPDATE test_ref_indirection + SET (col_int, col_text) = (select 2, 'not ok') + , (col_bool) = (select false) +RETURNING *; UPDATE test_ref_indirection SET (col_int, col_date) = (select 2, '1970-06-06'::date) , (col_text, col_bool) = (select 'not ok', false) RETURNING *; --- TODO wrong ERROR UPDATE test_dist_indirection SET (col_int, col_date) = (select 2, '1970-06-06'::date) , (col_text, col_bool) = (select 'not ok', false) @@ -131,28 +136,23 @@ SELECT * FROM test_ref_indirection ORDER BY id; SELECT * FROM test_dist_indirection ORDER BY id; -- those should not --- TODO wrong ERROR UPDATE test_ref_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) RETURNING *; --- TODO wrong ERROR UPDATE test_dist_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) RETURNING *; --- TODO wrong ERROR UPDATE test_ref_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) WHERE id = 2 RETURNING *; --- TODO wrong ERROR UPDATE test_dist_indirection SET (col_text, col_bool) = (SELECT 'not ok', false) WHERE id = 2 RETURNING *; -- several updates in CTE shoult not work --- TODO wrong ERROR with qq3 as ( update test_ref_indirection SET (col_text, col_bool) @@ -168,7 +168,6 @@ qq4 as ( returning * ) select * from qq3 union all select * from qq4; --- TODO wrong ERROR with qq3 as ( update test_dist_indirection SET (col_text, col_bool) From dcdaaaa88dfd4bdc9b31cf308a2626fa5e47c7cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= Date: Sun, 2 Mar 2025 08:00:02 +0100 Subject: [PATCH 5/7] WIP reorder target list All OK for basic UPDATE multi SELECT --- .../distributed/deparser/citus_ruleutils.c | 70 ++++ .../distributed/deparser/ruleutils_16.c | 72 ++--- src/include/distributed/citus_ruleutils.h | 3 + src/test/regress/expected/indirections.out | 301 ++++++++++++------ src/test/regress/sql/indirections.sql | 95 +++--- 5 files changed, 359 insertions(+), 182 deletions(-) diff --git a/src/backend/distributed/deparser/citus_ruleutils.c b/src/backend/distributed/deparser/citus_ruleutils.c index d590495a6..a7f19c869 100644 --- a/src/backend/distributed/deparser/citus_ruleutils.c +++ b/src/backend/distributed/deparser/citus_ruleutils.c @@ -1715,3 +1715,73 @@ RoleSpecString(RoleSpec *spec, bool withQuoteIdentifier) } } } + + +/* + * list_sort comparator to sort target list by paramid (in MULTIEXPR) + * Intended for indirection management: UPDATE SET () = (SELECT ) + */ +int +target_list_cmp(const ListCell *a, const ListCell *b) +{ + TargetEntry *tleA = lfirst(a); + TargetEntry *tleB = lfirst(b); + + if (IsA(tleA->expr, Param) && IsA(tleB->expr, Param)) + { + int la = ((Param *) tleA->expr)->paramid; + int lb = ((Param *) tleB->expr)->paramid; + return (la > lb) - (la < lb); + } + else if ((IsA(tleA->expr, Param) && IsA(tleB->expr, SubLink)) || + (IsA(tleA->expr, SubLink) && IsA(tleB->expr, Param)) || + (IsA(tleA->expr, SubLink) && IsA(tleB->expr, SubLink))) + { + return -1; + } + else + { + elog(ERROR, "unexpected nodes"); + } +} + + +/* + * Recursively search an expression for a Param and return its paramid + * Intended for indirection management: UPDATE SET () = (SELECT ) + * Does not cover all options but those supported by Citus. + */ +int +GetParamId(Node *expr) +{ + int paramid = 0; + + if (expr == NULL) + { + return paramid; + } + + /* If it's a Param, return its attnum */ + if (IsA(expr, Param)) + { + Param *param = (Param *) expr; + paramid = param->paramid; + } + /* If it's a FuncExpr, search in arguments */ + else if (IsA(expr, FuncExpr)) + { + FuncExpr *func = (FuncExpr *) expr; + ListCell *lc; + + foreach(lc, func->args) + { + paramid = GetParamId((Node *) lfirst(lc)); + if (paramid != 0) + { + break; /* Stop at the first valid paramid */ + } + } + } + + return paramid; +} diff --git a/src/backend/distributed/deparser/ruleutils_16.c b/src/backend/distributed/deparser/ruleutils_16.c index 027fe3242..91eeb4b76 100644 --- a/src/backend/distributed/deparser/ruleutils_16.c +++ b/src/backend/distributed/deparser/ruleutils_16.c @@ -3525,9 +3525,6 @@ get_update_query_targetlist_def(Query *query, List *targetList, SubLink *cur_ma_sublink; List *ma_sublinks; - AttrNumber previous_attnum = InvalidAttrNumber; - int paramid_increment = 0; - /* * Prepare to deal with MULTIEXPR assignments: collect the source SubLinks * into a list. We expect them to appear, in ID order, in resjunk tlist @@ -3536,13 +3533,21 @@ get_update_query_targetlist_def(Query *query, List *targetList, ma_sublinks = NIL; if (query->hasSubLinks) /* else there can't be any */ { + bool saw_junk = false; + bool need_to_sort_target_list = false; + int previous_paramid = 0; + foreach(l, targetList) { TargetEntry *tle = (TargetEntry *) lfirst(l); + // elog(WARNING, "TOP node to string: %s", nodeToString(tle->expr)); + // elog(WARNING, "TOP node type: %d", (int) nodeTag(tle->expr)); + if (tle->resjunk && IsA(tle->expr, SubLink)) { SubLink *sl = (SubLink *) tle->expr; + saw_junk = true; if (sl->subLinkType == MULTIEXPR_SUBLINK) { @@ -3550,7 +3555,30 @@ get_update_query_targetlist_def(Query *query, List *targetList, Assert(sl->subLinkId == list_length(ma_sublinks)); } } + else if (!tle->resjunk) + { + int paramid = 0; + if (saw_junk) + elog(ERROR, "out of order target list"); + + paramid = GetParamId((Node *) tle->expr); + if (paramid < previous_paramid) + need_to_sort_target_list = true; + + previous_paramid = paramid; + } } + + /* + * reorder the target list on left side of the update: + * SET () = (SELECT ) + * reordering the SELECT side only does not work, consider a case like: + * SET (col_1, col3) = (SELECT 1, 3), (col_2) = (SELECT 2) + * Then default order will lead to: + * SET (col_1, col2) = (SELECT 1, 3), (col_3) = (SELECT 2) + */ + if (need_to_sort_target_list) + list_sort(targetList, target_list_cmp); } next_ma_cell = list_head(ma_sublinks); cur_ma_sublink = NULL; @@ -3650,54 +3678,18 @@ get_update_query_targetlist_def(Query *query, List *targetList, */ if (cur_ma_sublink != NULL) { - AttrNumber attnum = InvalidAttrNumber; - if (IsA(expr, Param)) - { - Param *param = (Param *) expr; - attnum = param->paramid + paramid_increment; - } - else if (IsA(expr, FuncExpr)) - { - FuncExpr *func = (FuncExpr *) expr; - ListCell *lc; - - /* Iterate through the arguments of the FuncExpr */ - foreach(lc, func->args) - { - Node *arg = (Node *) lfirst(lc); - - /* Check if the argument is a PARAM node */ - if (IsA(arg, Param)) - { - Param *param = (Param *) arg; - attnum = param->paramid + paramid_increment; - - break; /* Exit loop once we find the PARAM node */ - } - } - } - - if (previous_attnum >= attnum) - ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg( - "cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order"), - errhint("Sort the columns on the left side by physical order."))); - - previous_attnum = attnum; - if (--remaining_ma_columns > 0) continue; /* not the last column of multiassignment */ - appendStringInfoChar(buf, ')'); expr = (Node *) cur_ma_sublink; cur_ma_sublink = NULL; - paramid_increment = previous_attnum; } appendStringInfoString(buf, " = "); get_rule_expr(expr, context, false); } + elog(DEBUG4, "rewriten query: %s", buf->data); } /* ---------- diff --git a/src/include/distributed/citus_ruleutils.h b/src/include/distributed/citus_ruleutils.h index 3a9c36482..9089ca3a7 100644 --- a/src/include/distributed/citus_ruleutils.h +++ b/src/include/distributed/citus_ruleutils.h @@ -60,5 +60,8 @@ extern char * generate_operator_name(Oid operid, Oid arg1, Oid arg2); extern List * getOwnedSequences_internal(Oid relid, AttrNumber attnum, char deptype); extern void AppendOptionListToString(StringInfo stringData, List *options); +extern int GetParamId(Node *expr); +extern int target_list_cmp(const ListCell *a, const ListCell *b); + #endif /* CITUS_RULEUTILS_H */ diff --git a/src/test/regress/expected/indirections.out b/src/test/regress/expected/indirections.out index 20bd73d2b..115f5a7b4 100644 --- a/src/test/regress/expected/indirections.out +++ b/src/test/regress/expected/indirections.out @@ -126,210 +126,303 @@ RETURNING *; (4 rows) UPDATE test_ref_indirection - SET (col_bool, col_date) = (select true, '1970-01-01'::date) - , (col_int, col_text) = (select 1, 'still ok') + SET (col_bool, col_date) = (select false, '1971-01-01'::date) + , (col_int, col_text) = (select 2, '2 ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | still ok - 2 | t | 01-01-1970 | 1 | still ok - 3 | t | 01-01-1970 | 1 | still ok - 4 | t | 01-01-1970 | 1 | still ok + 1 | f | 01-01-1971 | 2 | 2 ok + 2 | f | 01-01-1971 | 2 | 2 ok + 3 | f | 01-01-1971 | 2 | 2 ok + 4 | f | 01-01-1971 | 2 | 2 ok (4 rows) UPDATE test_dist_indirection - SET (col_bool, col_date) = (select true, '1970-01-01'::date) - , (col_int, col_text) = (select 1, 'still ok') + SET (col_bool, col_date) = (select false, '1971-01-01'::date) + , (col_int, col_text) = (select 2, '2 ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | still ok - 2 | t | 01-01-1970 | 1 | still ok - 3 | t | 01-01-1970 | 1 | still ok - 4 | t | 01-01-1970 | 1 | still ok + 1 | f | 01-01-1971 | 2 | 2 ok + 2 | f | 01-01-1971 | 2 | 2 ok + 3 | f | 01-01-1971 | 2 | 2 ok + 4 | f | 01-01-1971 | 2 | 2 ok (4 rows) UPDATE test_ref_indirection - SET (col_bool, col_int) = (select true, 1) - , (col_text) = (select 'ok') + SET (col_bool, col_int) = (select true, 3) + , (col_text) = (select '3 ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1971 | 3 | 3 ok + 2 | t | 01-01-1971 | 3 | 3 ok + 3 | t | 01-01-1971 | 3 | 3 ok + 4 | t | 01-01-1971 | 3 | 3 ok (4 rows) -SELECT * FROM test_ref_indirection ORDER BY id; +UPDATE test_dist_indirection + SET (col_bool, col_int) = (select true, 3) + , (col_text) = (select '3 ok') +RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1971 | 3 | 3 ok + 2 | t | 01-01-1971 | 3 | 3 ok + 3 | t | 01-01-1971 | 3 | 3 ok + 4 | t | 01-01-1971 | 3 | 3 ok (4 rows) -SELECT * FROM test_dist_indirection ORDER BY id; - id | col_bool | col_date | col_int | col_text ---------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | still ok - 2 | t | 01-01-1970 | 1 | still ok - 3 | t | 01-01-1970 | 1 | still ok - 4 | t | 01-01-1970 | 1 | still ok -(4 rows) - --- but those should not: +-- but those should work since 13.X UPDATE test_ref_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-06-06'::date, 'not ok', 2, false) + = (SELECT '1972-01-01'::date, '4 ok', 4, false) RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1972 | 4 | 4 ok + 2 | f | 01-01-1972 | 4 | 4 ok + 3 | f | 01-01-1972 | 4 | 4 ok + 4 | f | 01-01-1972 | 4 | 4 ok +(4 rows) + UPDATE test_dist_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-06-06'::date, 'not ok', 2, false) + = (SELECT '1972-01-01'::date, '4 ok', 4, false) RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1972 | 4 | 4 ok + 2 | f | 01-01-1972 | 4 | 4 ok + 3 | f | 01-01-1972 | 4 | 4 ok + 4 | f | 01-01-1972 | 4 | 4 ok +(4 rows) + UPDATE test_ref_indirection - SET (col_int, col_text) = (select 2, 'not ok') + SET (col_int, col_text) = (select 5, '5 ok') + , (col_bool) = (select true) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1972 | 5 | 5 ok + 2 | t | 01-01-1972 | 5 | 5 ok + 3 | t | 01-01-1972 | 5 | 5 ok + 4 | t | 01-01-1972 | 5 | 5 ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_int, col_text) = (select 5, '5 ok') + , (col_bool) = (select true) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1972 | 5 | 5 ok + 2 | t | 01-01-1972 | 5 | 5 ok + 3 | t | 01-01-1972 | 5 | 5 ok + 4 | t | 01-01-1972 | 5 | 5 ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_int, col_date) = (select 6, '1973-01-01'::date) + , (col_text, col_bool) = (select '6 ok', false) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1973 | 6 | 6 ok + 2 | f | 01-01-1973 | 6 | 6 ok + 3 | f | 01-01-1973 | 6 | 6 ok + 4 | f | 01-01-1973 | 6 | 6 ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_int, col_date) = (select 6, '1973-01-01'::date) + , (col_text, col_bool) = (select '6 ok', false) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1973 | 6 | 6 ok + 2 | f | 01-01-1973 | 6 | 6 ok + 3 | f | 01-01-1973 | 6 | 6 ok + 4 | f | 01-01-1973 | 6 | 6 ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_int, col_date, col_text) = (select 7, '1974-01-01'::date, '7 ok') + , (col_bool) = (select true) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1974 | 7 | 7 ok + 2 | t | 01-01-1974 | 7 | 7 ok + 3 | t | 01-01-1974 | 7 | 7 ok + 4 | t | 01-01-1974 | 7 | 7 ok +(4 rows) + +UPDATE test_dist_indirection + SET (col_int, col_date, col_text) = (select 7, '1974-01-01'::date, '7 ok') + , (col_bool) = (select true) +RETURNING *; + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | t | 01-01-1974 | 7 | 7 ok + 2 | t | 01-01-1974 | 7 | 7 ok + 3 | t | 01-01-1974 | 7 | 7 ok + 4 | t | 01-01-1974 | 7 | 7 ok +(4 rows) + +UPDATE test_ref_indirection + SET (col_date, col_text) = (select '1975-01-01'::date, '8 ok') + , (col_int) = (select 8) , (col_bool) = (select false) RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. -UPDATE test_ref_indirection - SET (col_int, col_date) = (select 2, '1970-06-06'::date) - , (col_text, col_bool) = (select 'not ok', false) -RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1975 | 8 | 8 ok + 2 | f | 01-01-1975 | 8 | 8 ok + 3 | f | 01-01-1975 | 8 | 8 ok + 4 | f | 01-01-1975 | 8 | 8 ok +(4 rows) + UPDATE test_dist_indirection - SET (col_int, col_date) = (select 2, '1970-06-06'::date) - , (col_text, col_bool) = (select 'not ok', false) + SET (col_date, col_text) = (select '1975-01-01'::date, '8 ok') + , (col_int) = (select 8) + , (col_bool) = (select false) RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 1 | f | 01-01-1975 | 8 | 8 ok + 2 | f | 01-01-1975 | 8 | 8 ok + 3 | f | 01-01-1975 | 8 | 8 ok + 4 | f | 01-01-1975 | 8 | 8 ok +(4 rows) + -- -- more restrictive ones, just in case we miss a wrong value -- -- those should work UPDATE test_ref_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT true, '9 ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1975 | 8 | 9 ok + 2 | t | 01-01-1975 | 8 | 9 ok + 3 | t | 01-01-1975 | 8 | 9 ok + 4 | t | 01-01-1975 | 8 | 9 ok (4 rows) UPDATE test_dist_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT true, '9 ok') RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1975 | 8 | 9 ok + 2 | t | 01-01-1975 | 8 | 9 ok + 3 | t | 01-01-1975 | 8 | 9 ok + 4 | t | 01-01-1975 | 8 | 9 ok (4 rows) UPDATE test_ref_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT false, '10 ok') WHERE id = 1 RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok + 1 | f | 01-01-1975 | 8 | 10 ok (1 row) UPDATE test_dist_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT false, '10 ok') WHERE id = 1 RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok + 1 | f | 01-01-1975 | 8 | 10 ok (1 row) -SELECT * FROM test_ref_indirection ORDER BY id; +UPDATE test_ref_indirection + SET (col_text, col_bool) = (SELECT '11 ok', true) +RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1975 | 8 | 11 ok + 2 | t | 01-01-1975 | 8 | 11 ok + 3 | t | 01-01-1975 | 8 | 11 ok + 4 | t | 01-01-1975 | 8 | 11 ok (4 rows) -SELECT * FROM test_dist_indirection ORDER BY id; +UPDATE test_dist_indirection + SET (col_text, col_bool) = (SELECT '11 ok', true) +RETURNING *; id | col_bool | col_date | col_int | col_text --------------------------------------------------------------------- - 1 | t | 01-01-1970 | 1 | ok - 2 | t | 01-01-1970 | 1 | ok - 3 | t | 01-01-1970 | 1 | ok - 4 | t | 01-01-1970 | 1 | ok + 1 | t | 01-01-1975 | 8 | 11 ok + 2 | t | 01-01-1975 | 8 | 11 ok + 3 | t | 01-01-1975 | 8 | 11 ok + 4 | t | 01-01-1975 | 8 | 11 ok (4 rows) --- those should not UPDATE test_ref_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) -RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. -UPDATE test_dist_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) -RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. -UPDATE test_ref_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '12 ok', false) WHERE id = 2 RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 2 | f | 01-01-1975 | 8 | 12 ok +(1 row) + UPDATE test_dist_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '12 ok', false) WHERE id = 2 RETURNING *; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 2 | f | 01-01-1975 | 8 | 12 ok +(1 row) + -- several updates in CTE shoult not work with qq3 as ( update test_ref_indirection SET (col_text, col_bool) - = (SELECT 'full', true) + = (SELECT '13', true) where id = 3 returning * ), qq4 as ( update test_ref_indirection SET (col_text, col_bool) - = (SELECT 'fully', true) + = (SELECT '14', false) where id = 4 returning * ) select * from qq3 union all select * from qq4; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 3 | t | 01-01-1975 | 8 | 13 + 4 | f | 01-01-1975 | 8 | 14 +(2 rows) + with qq3 as ( update test_dist_indirection SET (col_text, col_bool) - = (SELECT 'full', true) + = (SELECT '13', true) where id = 3 returning * ), qq4 as ( update test_dist_indirection SET (col_text, col_bool) - = (SELECT 'fully', true) + = (SELECT '14', false) where id = 4 returning * ) select * from qq3 union all select * from qq4; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. + id | col_bool | col_date | col_int | col_text +--------------------------------------------------------------------- + 3 | t | 01-01-1975 | 8 | 13 + 4 | f | 01-01-1975 | 8 | 14 +(2 rows) + DROP TABLE test_dist_indirection; DROP TABLE test_dist_indirection_new; DROP TABLE test_ref_indirection; @@ -349,10 +442,6 @@ SELECT create_reference_table('indirections.update_test'); UPDATE update_test SET (b,a) = (select a,b from update_test where b = 41 and c = 'car') WHERE a = 100 AND b = 20; -ERROR: cannot plan distributed UPDATE SET (..) = (SELECT ...) query when not sorted by physical order -HINT: Sort the columns on the left side by physical order. -- https://github.com/citusdata/citus/pull/5692 +set client_min_messages to ERROR; DROP SCHEMA indirections CASCADE; -NOTICE: drop cascades to 2 other objects -DETAIL: drop cascades to table update_test -drop cascades to table update_test_750006 diff --git a/src/test/regress/sql/indirections.sql b/src/test/regress/sql/indirections.sql index 73ff78b71..598124208 100644 --- a/src/test/regress/sql/indirections.sql +++ b/src/test/regress/sql/indirections.sql @@ -73,43 +73,69 @@ UPDATE test_dist_indirection RETURNING *; UPDATE test_ref_indirection - SET (col_bool, col_date) = (select true, '1970-01-01'::date) - , (col_int, col_text) = (select 1, 'still ok') + SET (col_bool, col_date) = (select false, '1971-01-01'::date) + , (col_int, col_text) = (select 2, '2 ok') RETURNING *; UPDATE test_dist_indirection - SET (col_bool, col_date) = (select true, '1970-01-01'::date) - , (col_int, col_text) = (select 1, 'still ok') + SET (col_bool, col_date) = (select false, '1971-01-01'::date) + , (col_int, col_text) = (select 2, '2 ok') RETURNING *; UPDATE test_ref_indirection - SET (col_bool, col_int) = (select true, 1) - , (col_text) = (select 'ok') + SET (col_bool, col_int) = (select true, 3) + , (col_text) = (select '3 ok') +RETURNING *; +UPDATE test_dist_indirection + SET (col_bool, col_int) = (select true, 3) + , (col_text) = (select '3 ok') RETURNING *; -SELECT * FROM test_ref_indirection ORDER BY id; -SELECT * FROM test_dist_indirection ORDER BY id; - --- but those should not: +-- but those should work since 13.X UPDATE test_ref_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-06-06'::date, 'not ok', 2, false) + = (SELECT '1972-01-01'::date, '4 ok', 4, false) RETURNING *; UPDATE test_dist_indirection SET (col_date, col_text, col_int, col_bool) - = (SELECT '1970-06-06'::date, 'not ok', 2, false) + = (SELECT '1972-01-01'::date, '4 ok', 4, false) RETURNING *; UPDATE test_ref_indirection - SET (col_int, col_text) = (select 2, 'not ok') + SET (col_int, col_text) = (select 5, '5 ok') + , (col_bool) = (select true) +RETURNING *; +UPDATE test_dist_indirection + SET (col_int, col_text) = (select 5, '5 ok') + , (col_bool) = (select true) +RETURNING *; + +UPDATE test_ref_indirection + SET (col_int, col_date) = (select 6, '1973-01-01'::date) + , (col_text, col_bool) = (select '6 ok', false) +RETURNING *; +UPDATE test_dist_indirection + SET (col_int, col_date) = (select 6, '1973-01-01'::date) + , (col_text, col_bool) = (select '6 ok', false) +RETURNING *; + +UPDATE test_ref_indirection + SET (col_int, col_date, col_text) = (select 7, '1974-01-01'::date, '7 ok') + , (col_bool) = (select true) +RETURNING *; +UPDATE test_dist_indirection + SET (col_int, col_date, col_text) = (select 7, '1974-01-01'::date, '7 ok') + , (col_bool) = (select true) +RETURNING *; + +UPDATE test_ref_indirection + SET (col_date, col_text) = (select '1975-01-01'::date, '8 ok') + , (col_int) = (select 8) , (col_bool) = (select false) RETURNING *; -UPDATE test_ref_indirection - SET (col_int, col_date) = (select 2, '1970-06-06'::date) - , (col_text, col_bool) = (select 'not ok', false) -RETURNING *; UPDATE test_dist_indirection - SET (col_int, col_date) = (select 2, '1970-06-06'::date) - , (col_text, col_bool) = (select 'not ok', false) + SET (col_date, col_text) = (select '1975-01-01'::date, '8 ok') + , (col_int) = (select 8) + , (col_bool) = (select false) RETURNING *; -- @@ -117,38 +143,34 @@ RETURNING *; -- -- those should work UPDATE test_ref_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT true, '9 ok') RETURNING *; UPDATE test_dist_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT true, '9 ok') RETURNING *; UPDATE test_ref_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT false, '10 ok') WHERE id = 1 RETURNING *; UPDATE test_dist_indirection - SET (col_bool, col_text) = (SELECT true, 'ok') + SET (col_bool, col_text) = (SELECT false, '10 ok') WHERE id = 1 RETURNING *; -SELECT * FROM test_ref_indirection ORDER BY id; -SELECT * FROM test_dist_indirection ORDER BY id; - --- those should not UPDATE test_ref_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '11 ok', true) RETURNING *; UPDATE test_dist_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '11 ok', true) RETURNING *; UPDATE test_ref_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '12 ok', false) WHERE id = 2 RETURNING *; UPDATE test_dist_indirection - SET (col_text, col_bool) = (SELECT 'not ok', false) + SET (col_text, col_bool) = (SELECT '12 ok', false) WHERE id = 2 RETURNING *; @@ -156,14 +178,14 @@ RETURNING *; with qq3 as ( update test_ref_indirection SET (col_text, col_bool) - = (SELECT 'full', true) + = (SELECT '13', true) where id = 3 returning * ), qq4 as ( update test_ref_indirection SET (col_text, col_bool) - = (SELECT 'fully', true) + = (SELECT '14', false) where id = 4 returning * ) @@ -171,14 +193,14 @@ select * from qq3 union all select * from qq4; with qq3 as ( update test_dist_indirection SET (col_text, col_bool) - = (SELECT 'full', true) + = (SELECT '13', true) where id = 3 returning * ), qq4 as ( update test_dist_indirection SET (col_text, col_bool) - = (SELECT 'fully', true) + = (SELECT '14', false) where id = 4 returning * ) @@ -201,5 +223,6 @@ SET (b,a) = (select a,b from update_test where b = 41 and c = 'car') WHERE a = 100 AND b = 20; -- https://github.com/citusdata/citus/pull/5692 - +set client_min_messages to ERROR; DROP SCHEMA indirections CASCADE; + From e18857998f5c8dfb2c8eafd2fed42b44eaeec35a Mon Sep 17 00:00:00 2001 From: Ibrahim Halatci Date: Mon, 23 Jun 2025 13:35:54 +0000 Subject: [PATCH 6/7] address couple of Colm's comments and also to enable compilation and tests --- src/backend/distributed/deparser/ruleutils_16.c | 3 +-- src/backend/distributed/planner/insert_select_planner.c | 5 ----- .../sql/{indirections.sql => multi_update_select.sql} | 0 3 files changed, 1 insertion(+), 7 deletions(-) rename src/test/regress/sql/{indirections.sql => multi_update_select.sql} (100%) diff --git a/src/backend/distributed/deparser/ruleutils_16.c b/src/backend/distributed/deparser/ruleutils_16.c index 91eeb4b76..b0eadc281 100644 --- a/src/backend/distributed/deparser/ruleutils_16.c +++ b/src/backend/distributed/deparser/ruleutils_16.c @@ -3558,8 +3558,7 @@ get_update_query_targetlist_def(Query *query, List *targetList, else if (!tle->resjunk) { int paramid = 0; - if (saw_junk) - elog(ERROR, "out of order target list"); + Assert(!saw_junk) paramid = GetParamId((Node *) tle->expr); if (paramid < previous_paramid) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index d100b8f74..3bf0bb327 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -1082,11 +1082,6 @@ ReorderInsertSelectTargetLists(Query *originalQuery, RangeTblEntry *insertRte, TargetEntry *newSubqueryTargetEntry = NULL; AttrNumber originalAttrNo = get_attnum(insertRelationId, oldInsertTargetEntry->resname); - Node *expr; - - /* we need to explore the underlying expression */ - expr = (Node *) oldInsertTargetEntry->expr; - expr = strip_implicit_coercions(expr); /* we need to explore the underlying expression */ Node *expr = strip_implicit_coercions((Node *) oldInsertTargetEntry->expr); diff --git a/src/test/regress/sql/indirections.sql b/src/test/regress/sql/multi_update_select.sql similarity index 100% rename from src/test/regress/sql/indirections.sql rename to src/test/regress/sql/multi_update_select.sql From fd099d1ca4e32a26d7cb7ca9d0122fa51cbb7464 Mon Sep 17 00:00:00 2001 From: Ibrahim Halatci Date: Mon, 23 Jun 2025 15:56:58 +0000 Subject: [PATCH 7/7] typo --- src/backend/distributed/deparser/ruleutils_16.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/distributed/deparser/ruleutils_16.c b/src/backend/distributed/deparser/ruleutils_16.c index b0eadc281..32aad726a 100644 --- a/src/backend/distributed/deparser/ruleutils_16.c +++ b/src/backend/distributed/deparser/ruleutils_16.c @@ -3558,7 +3558,7 @@ get_update_query_targetlist_def(Query *query, List *targetList, else if (!tle->resjunk) { int paramid = 0; - Assert(!saw_junk) + Assert(!saw_junk); paramid = GetParamId((Node *) tle->expr); if (paramid < previous_paramid)