From 988d017813b8014214377c94fabf0b6eb568328a Mon Sep 17 00:00:00 2001 From: Zhenglong Li Date: Mon, 19 Sep 2022 17:15:09 +0800 Subject: [PATCH 01/13] Don't simplify uncorrelated EXISTS sublink in some cases (#13838) In Greenplum, different from upstream, will try to pull up the correlated EXISTS sublink which has aggregate, just like: ``` postgres=# create table t(a int, b int); postgres=# create table s (a int, b int); postgres=# explain (costs off) select * from t where exists (select sum(s.a) from s where s.a = t.a group by s.a); QUERY PLAN ------------------------------------------ Gather Motion 3:1 (slice1; segments: 3) -> Hash Join Hash Cond: (t.a = s.a) -> Seq Scan on t -> Hash -> HashAggregate Group Key: s.a -> Seq Scan on s Optimizer: Postgres query optimizer (9 rows) ``` So Greenplum changed the behavior of function `convert_EXISTS_sublink_to_join()` and `simplify_EXISTS_query()`, so `simplify_EXISTS_query()` will simplify the EXISTS sublink which has aggregate node, it will reset `targetList` and other clauses to null, just like: ``` query->targetList = NIL; query->distinctClause = NIL; query->sortClause = NIL; query->hasDistinctOn = false; ``` But when we exec uncorrelated EXISTS sublink which also has an aggregate node, we can't reset `targetList`. Otherwise, we can't make a plan for an aggregate node that doesn't have an aggregate column, just like #11849. This patch tries to fix the above issue, and the thought is **NOT** simplify uncorrelated EXISTS sublink in some situation. From 98e402651c907308c77a555fffe6c5e7afcca4f4 Mon Sep 17 00:00:00 2001 From: Chris Hajas Date: Tue, 13 Dec 2022 16:25:53 -0800 Subject: [PATCH 02/13] Fix gp_dqa test to explicitly analyze tables (#14643) Commit 0f4af19b71f2f31acd28c39d0d0a865cbf1d3bab added this test, but the `analyze` statement exposed an unrelated issue and caused CI to fail. We should explicitly analyze tables to reduce runtime in tests anyway. --- src/test/regress/expected/gp_dqa.out | 64 +++++++++++++++- .../regress/expected/gp_dqa_optimizer.out | 74 +++++++++++++++++++ src/test/regress/sql/gp_dqa.sql | 18 +++++ 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/src/test/regress/expected/gp_dqa.out b/src/test/regress/expected/gp_dqa.out index 9090a1547a8..30505978ab3 100644 --- a/src/test/regress/expected/gp_dqa.out +++ b/src/test/regress/expected/gp_dqa.out @@ -2367,7 +2367,7 @@ select count(distinct a) from t_issue_659; -> Gather Motion 3:1 (slice1; segments: 3) -> Partial Aggregate -> Seq Scan on t_issue_659 - Optimizer: Postgres query optimizer + Optimizer: Pivotal Optimizer (GPORCA) (5 rows) select count(distinct a) from t_issue_659; @@ -2402,6 +2402,68 @@ select count(distinct a) from t_issue_659; reset gp_eager_distinct_dedup; reset optimizer_force_three_stage_scalar_dqa; +-- fix dqa bug when optimizer_force_multistage_agg is on +set optimizer_force_multistage_agg = on; +create table multiagg1(a int, b bigint, c int); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Cloudberry Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +create table multiagg2(a int, b bigint, c numeric(8, 4)); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Cloudberry Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +insert into multiagg1 values(generate_series(1, 10), generate_series(1, 10), generate_series(1, 10)); +insert into multiagg2 values(generate_series(1, 10), generate_series(1, 10), 555.55); +analyze multiagg1; +analyze multiagg2; +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg1; + QUERY PLAN +----------------------------------------------------------------------------------------------------------- + Finalize Aggregate + Output: count(DISTINCT b), sum(c) + -> Gather Motion 3:1 (slice1; segments: 3) + Output: (PARTIAL count(DISTINCT b)), (PARTIAL sum(c)) + -> Partial Aggregate + Output: PARTIAL count(DISTINCT b), PARTIAL sum(c) + -> Redistribute Motion 3:3 (slice2; segments: 3) + Output: b, c + Hash Key: b + -> Seq Scan on public.multiagg1 + Output: b, c + Settings: enable_groupagg = 'off', enable_hashagg = 'on', gp_motion_cost_per_row = '2', optimizer = 'off' + Optimizer: Postgres query optimizer +(13 rows) + +select count(distinct b), sum(c) from multiagg1; + count | sum +-------+----- + 10 | 55 +(1 row) + +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg2; + QUERY PLAN +----------------------------------------------------------------------------------------------------------- + Finalize Aggregate + Output: count(DISTINCT b), sum(c) + -> Gather Motion 3:1 (slice1; segments: 3) + Output: (PARTIAL count(DISTINCT b)), (PARTIAL sum(c)) + -> Partial Aggregate + Output: PARTIAL count(DISTINCT b), PARTIAL sum(c) + -> Redistribute Motion 3:3 (slice2; segments: 3) + Output: b, c + Hash Key: b + -> Seq Scan on public.multiagg2 + Output: b, c + Settings: enable_groupagg = 'off', enable_hashagg = 'on', gp_motion_cost_per_row = '2', optimizer = 'off' + Optimizer: Postgres query optimizer +(13 rows) + +select count(distinct b), sum(c) from multiagg2; + count | sum +-------+----------- + 10 | 5555.5000 +(1 row) + +drop table multiagg1; +drop table multiagg2; reset optimizer_force_multistage_agg; reset optimizer_enable_use_distribution_in_dqa; drop table t_issue_659; diff --git a/src/test/regress/expected/gp_dqa_optimizer.out b/src/test/regress/expected/gp_dqa_optimizer.out index 3f1c7f540e5..a99bf5a3ee3 100644 --- a/src/test/regress/expected/gp_dqa_optimizer.out +++ b/src/test/regress/expected/gp_dqa_optimizer.out @@ -2550,6 +2550,80 @@ select count(distinct a) from t_issue_659; reset gp_eager_distinct_dedup; reset optimizer_force_three_stage_scalar_dqa; +-- fix dqa bug when optimizer_force_multistage_agg is on +set optimizer_force_multistage_agg = on; +create table multiagg1(a int, b bigint, c int); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Cloudberry Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +create table multiagg2(a int, b bigint, c numeric(8, 4)); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Cloudberry Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +insert into multiagg1 values(generate_series(1, 10), generate_series(1, 10), generate_series(1, 10)); +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: Feature not supported: Unexpected target list entries in ProjectSet node +insert into multiagg2 values(generate_series(1, 10), generate_series(1, 10), 555.55); +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: Feature not supported: Unexpected target list entries in ProjectSet node +analyze multiagg1; +analyze multiagg2; +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg1; +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: GPDB Expression type: GPDB_96_MERGE_FIXME: Intermediate aggregate stage not implemented not supported in DXL + QUERY PLAN +---------------------------------------------------------------------------------------- + Finalize Aggregate + Output: count(DISTINCT b), sum(c) + -> Gather Motion 3:1 (slice1; segments: 3) + Output: (PARTIAL count(DISTINCT b)), (PARTIAL sum(c)) + -> Partial Aggregate + Output: PARTIAL count(DISTINCT b), PARTIAL sum(c) + -> Redistribute Motion 3:3 (slice2; segments: 3) + Output: b, c + Hash Key: b + -> Seq Scan on public.multiagg1 + Output: b, c + Settings: enable_groupagg = 'off', enable_hashagg = 'on', gp_motion_cost_per_row = '2' + Optimizer: Postgres query optimizer +(13 rows) + +select count(distinct b), sum(c) from multiagg1; +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: GPDB Expression type: GPDB_96_MERGE_FIXME: Intermediate aggregate stage not implemented not supported in DXL + count | sum +-------+----- + 10 | 55 +(1 row) + +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg2; +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: GPDB Expression type: GPDB_96_MERGE_FIXME: Intermediate aggregate stage not implemented not supported in DXL + QUERY PLAN +---------------------------------------------------------------------------------------- + Finalize Aggregate + Output: count(DISTINCT b), sum(c) + -> Gather Motion 3:1 (slice1; segments: 3) + Output: (PARTIAL count(DISTINCT b)), (PARTIAL sum(c)) + -> Partial Aggregate + Output: PARTIAL count(DISTINCT b), PARTIAL sum(c) + -> Redistribute Motion 3:3 (slice2; segments: 3) + Output: b, c + Hash Key: b + -> Seq Scan on public.multiagg2 + Output: b, c + Settings: enable_groupagg = 'off', enable_hashagg = 'on', gp_motion_cost_per_row = '2' + Optimizer: Postgres query optimizer +(13 rows) + +select count(distinct b), sum(c) from multiagg2; +INFO: GPORCA failed to produce a plan, falling back to planner +DETAIL: GPDB Expression type: GPDB_96_MERGE_FIXME: Intermediate aggregate stage not implemented not supported in DXL + count | sum +-------+----------- + 10 | 5555.5000 +(1 row) + +drop table multiagg1; +drop table multiagg2; reset optimizer_force_multistage_agg; reset optimizer_enable_use_distribution_in_dqa; drop table t_issue_659; diff --git a/src/test/regress/sql/gp_dqa.sql b/src/test/regress/sql/gp_dqa.sql index 65f52cd7436..9bbdf48c19a 100644 --- a/src/test/regress/sql/gp_dqa.sql +++ b/src/test/regress/sql/gp_dqa.sql @@ -428,6 +428,24 @@ select count(distinct a) from t_issue_659; select count(distinct a) from t_issue_659; reset gp_eager_distinct_dedup; reset optimizer_force_three_stage_scalar_dqa; + + +-- fix dqa bug when optimizer_force_multistage_agg is on +set optimizer_force_multistage_agg = on; +create table multiagg1(a int, b bigint, c int); +create table multiagg2(a int, b bigint, c numeric(8, 4)); +insert into multiagg1 values(generate_series(1, 10), generate_series(1, 10), generate_series(1, 10)); +insert into multiagg2 values(generate_series(1, 10), generate_series(1, 10), 555.55); +analyze multiagg1; +analyze multiagg2; + +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg1; +select count(distinct b), sum(c) from multiagg1; + +explain (verbose, costs off) select count(distinct b), sum(c) from multiagg2; +select count(distinct b), sum(c) from multiagg2; +drop table multiagg1; +drop table multiagg2; reset optimizer_force_multistage_agg; reset optimizer_enable_use_distribution_in_dqa; drop table t_issue_659; From 0121373dba4295c0209d9754f15de31857b12be2 Mon Sep 17 00:00:00 2001 From: Alexandra Wang Date: Tue, 6 Dec 2022 16:11:55 -0800 Subject: [PATCH 03/13] Support "Mq" in isolation test framework Previously, in commit 32b8646, we added "M" flag to support utility mode connection to a segment who is configured as mirror in FTS configuration but is actually running like a primary. We now also add "Mq" flag to support quitting from this connection. We will make use of it in the next commit. --- src/test/isolation2/sql_isolation_testcase.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/isolation2/sql_isolation_testcase.py b/src/test/isolation2/sql_isolation_testcase.py index f7b507c93bf..07b5292ab22 100644 --- a/src/test/isolation2/sql_isolation_testcase.py +++ b/src/test/isolation2/sql_isolation_testcase.py @@ -786,6 +786,10 @@ def process_command(self, command, output_file, global_sh_executor): pass elif flag == "M": self.get_process(output_file, process_name, con_mode, dbname=dbname).query(sql.strip(), post_run_cmd, global_sh_executor) + elif flag == "Mq": + if len(sql) > 0: + raise Exception("No query should be given on quit Mq") + self.quit_process(output_file, process_name, con_mode, dbname=dbname) else: raise Exception("Invalid isolation flag") @@ -812,7 +816,7 @@ def process_isolation_file(self, sql_file, output_file, initfile_prefix): if command_part == "" or command_part == "\n": print(file=output_file) newline = True - elif re.match(r".*;\s*$", command_part) or re.match(r"^\d+[q\\<]:\s*$", line) or re.match(r"^\*Rq:$", line) or re.match(r"^-?\d+[SUR][q\\<]:\s*$", line): + elif re.match(r".*;\s*$", command_part) or re.match(r"^\d+[q\\<]:\s*$", line) or re.match(r"^\*Rq:$", line) or re.match(r"^-?\d+[SUMR][q\\<]:\s*$", line): command += command_part try: self.process_command(command, output_file, shell_executor) From 4874eac2ad2710758096fed7fdef0d6d233743bc Mon Sep 17 00:00:00 2001 From: Zhenghua Lyu Date: Fri, 16 Dec 2022 07:20:20 +0800 Subject: [PATCH 04/13] Remove handle RangeSubselect in checkWellFormedRecursionWalker. Keep align with upstream. --- src/backend/parser/parse_cte.c | 12 --------- .../regress/expected/gp_recursive_cte.out | 25 +++++++++++++++++++ src/test/regress/expected/with.out | 6 ++--- src/test/regress/sql/gp_recursive_cte.sql | 12 +++++++++ 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c index ecc27df7668..beb0345d39e 100644 --- a/src/backend/parser/parse_cte.c +++ b/src/backend/parser/parse_cte.c @@ -1137,19 +1137,7 @@ checkWellFormedRecursionWalker(Node *node, CteState *cstate) checkWellFormedRecursionWalker(sl->testexpr, cstate); return false; } - if (IsA(node, RangeSubselect)) - { - RangeSubselect *rs = (RangeSubselect *) node; - /* - * we intentionally override outer context, since subquery is - * independent - */ - cstate->context = RECURSION_SUBLINK; - checkWellFormedRecursionWalker(rs->subquery, cstate); - cstate->context = save_context; - return false; - } return raw_expression_tree_walker(node, checkWellFormedRecursionWalker, (void *) cstate); diff --git a/src/test/regress/expected/gp_recursive_cte.out b/src/test/regress/expected/gp_recursive_cte.out index 72f493b88a1..00b08e4b84c 100644 --- a/src/test/regress/expected/gp_recursive_cte.out +++ b/src/test/regress/expected/gp_recursive_cte.out @@ -770,4 +770,29 @@ select id,name from cte; 4 | AAA>B1>C1_2 5 | AAA>B1>C1_3 (9 rows) +-- WTIH RECURSIVE and subquery +with recursive cte (n) as +( + select 1 + union all + select * from + ( + with x(n) as (select n from cte) + select n + 1 from x where n < 10 + ) q +) +select * from cte; + n +---- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 +(10 rows) diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 14944834c8d..9077579ea0a 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -1955,9 +1955,9 @@ WITH RECURSIVE foo(i) AS UNION ALL SELECT i+1 FROM foo WHERE i < 5) AS t ) SELECT * FROM foo; -ERROR: recursive reference to query "foo" must not appear within a subquery -LINE 5: (SELECT i+1 FROM foo WHERE i < 10 - ^ +ERROR: recursive reference to query "foo" must not appear more than once +LINE 7: SELECT i+1 FROM foo WHERE i < 5) AS t + ^ WITH RECURSIVE foo(i) AS (values (1) UNION ALL diff --git a/src/test/regress/sql/gp_recursive_cte.sql b/src/test/regress/sql/gp_recursive_cte.sql index 5d982210209..90aeda88147 100644 --- a/src/test/regress/sql/gp_recursive_cte.sql +++ b/src/test/regress/sql/gp_recursive_cte.sql @@ -506,3 +506,15 @@ with RECURSIVE cte as ( ) select id,name from cte; +-- WTIH RECURSIVE and subquery +with recursive cte (n) as +( + select 1 + union all + select * from + ( + with x(n) as (select n from cte) + select n + 1 from x where n < 10 + ) q +) +select * from cte; From 01681f8d6d01319b422ad94e009bbf39d60a9567 Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty Date: Thu, 15 Dec 2022 20:32:03 -0800 Subject: [PATCH 05/13] ao/co: Clarify compute_xid_horizon_for_tuples API This removes FIXMEs for the compute_xid_horizon_for_tuples() API, which was introduced in 558a9165e08. This API is used to compute what snapshots to conflict with when replaying WAL records for page-level index vacuums. Page-level index vacuums don't even arise for AO/CO tables today due to absence of HOT. Thus it is perfectly fine to leave this unimplemented. Details: (1) The amgettuple() routine for index AMs can optionally kill off tuples if they are found dead in the underlying table scan. This is done with IndexScanDesc->kill_prior_tuple. For examples: see btgettuple(), hashgettuple() and gistgettuple(). (2) The index AMs typically have a killitems function (like _bt_killitems()), which among other things sets a garbage flag on the index page (like BTP_HAS_GARBAGE) (3) Afterwards, if an index page has a garbage flag, it can undergo page level vacuum, even inside a scan (see _bt_vacuum_one_page()). This is where table_compute_xid_horizon_for_tuples() is typically called and the latestRemovedXid is recorded in the WAL (eg XLOG_BTREE_DELETE). (4) During replay on a hot standby, the latestRemovedXid is extracted from the record and then used to ResolveRecoveryConflictWithSnapshot(). Since, AO/CO tables don't have HOT chains, in index_fetch_heap(), all_dead is always set to false and thus the IndexScanDesc->kill_prior_tuple is always false. Thus, we never even perform index page-level vacuum for indexes on AO/CO tables, let alone compute the xid horizon. --- src/backend/access/aocs/aocsam_handler.c | 13 +++++++++++-- .../access/appendonly/appendonlyam_handler.c | 13 +++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/backend/access/aocs/aocsam_handler.c b/src/backend/access/aocs/aocsam_handler.c index 86b55ac1dc9..d91c913f87f 100644 --- a/src/backend/access/aocs/aocsam_handler.c +++ b/src/backend/access/aocs/aocsam_handler.c @@ -1269,8 +1269,17 @@ static TransactionId aoco_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate) { - // GPDB_14_MERGE_FIXME: vacuum related call back. - elog(ERROR, "not implemented yet"); + /* + * This API is only useful for hot standby snapshot conflict resolution + * (for eg. see btree_xlog_delete()), in the context of index page-level + * vacuums (aka page-level cleanups). This operation is only done when + * IndexScanDesc->kill_prior_tuple is true, which is never for AO/CO tables + * (we always return all_dead = false in the index_fetch_tuple() callback + * as we don't support HOT) + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("feature not supported on appendoptimized relations"))); } diff --git a/src/backend/access/appendonly/appendonlyam_handler.c b/src/backend/access/appendonly/appendonlyam_handler.c index 41c7d001572..4f4fd969ddb 100644 --- a/src/backend/access/appendonly/appendonlyam_handler.c +++ b/src/backend/access/appendonly/appendonlyam_handler.c @@ -888,8 +888,17 @@ static TransactionId appendonly_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate) { - // GPDB_14_MERGE_FIXME: vacuum related call back. - elog(ERROR, "not implemented yet"); + /* + * This API is only useful for hot standby snapshot conflict resolution + * (for eg. see btree_xlog_delete()), in the context of index page-level + * vacuums (aka page-level cleanups). This operation is only done when + * IndexScanDesc->kill_prior_tuple is true, which is never for AO/CO tables + * (we always return all_dead = false in the index_fetch_tuple() callback + * as we don't support HOT) + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("feature not supported on appendoptimized relations"))); } From 896cba6c5ae9d0f2c79f2401984969e667e4919c Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty Date: Fri, 16 Dec 2022 18:16:45 -0800 Subject: [PATCH 06/13] ao/co: Clarify tuple_lock API This commit confirms that the tuple_lock API can be safely left unimplemented for AO/CO tables. See below for details: (1) UPSERT: ExecOnConflictUpdate() calls this, but clearly upsert is not supported for AO/CO tables. (2) DELETE and UPDATE triggers: GetTupleForTrigger() calls this, but clearly these trigger types are not supported for AO/CO tables. (3) Logical replication: RelationFindReplTupleByIndex() and RelationFindReplTupleSeq() calls this, but clearly we don't support logical replication yet for GPDB. (4) For DELETEs/UPDATEs, when a state of TM_Updated is returned from table_tuple_delete() and table_tuple_update() respectively, this API is invoked. However, that is impossible for AO/CO tables as an AO/CO tuple cannot be deleted/updated while another transaction is updating it (see CdbTryOpenTable()). (5) Row-level locking (SELECT FOR ..): ExecLockRows() calls this but a plan containing the LockRows plan node is never generated for AO/CO tables. In fact, we lock at the table level instead. See 6ebce733317 for details. pstate->p_canOptSelectLockingClause = false; is done in addRangeTableEntry() for AO/CO tables, which prevents such plans. Also, ORCA falls back to planner when presented with a query carrying a locking clause. --- src/backend/access/aocs/aocsam_handler.c | 28 +++++++++++++++++-- .../access/appendonly/appendonlyam_handler.c | 28 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/backend/access/aocs/aocsam_handler.c b/src/backend/access/aocs/aocsam_handler.c index d91c913f87f..3ebf5d6a82d 100644 --- a/src/backend/access/aocs/aocsam_handler.c +++ b/src/backend/access/aocs/aocsam_handler.c @@ -1175,14 +1175,38 @@ aoco_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, return result; } +/* + * This API is called for a variety of purposes, which are either not supported + * for AO/CO tables or not supported for GPDB in general: + * + * (1) UPSERT: ExecOnConflictUpdate() calls this, but clearly upsert is not + * supported for AO/CO tables. + * + * (2) DELETE and UPDATE triggers: GetTupleForTrigger() calls this, but clearly + * these trigger types are not supported for AO/CO tables. + * + * (3) Logical replication: RelationFindReplTupleByIndex() and + * RelationFindReplTupleSeq() calls this, but clearly we don't support logical + * replication yet for GPDB. + * + * (4) For DELETEs/UPDATEs, when a state of TM_Updated is returned from + * table_tuple_delete() and table_tuple_update() respectively, this API is invoked. + * However, that is impossible for AO/CO tables as an AO/CO tuple cannot be + * deleted/updated while another transaction is updating it (see CdbTryOpenTable()). + * + * (5) Row-level locking (SELECT FOR ..): ExecLockRows() calls this but a plan + * containing the LockRows plan node is never generated for AO/CO tables. In fact, + * we lock at the table level instead. + */ static TM_Result aoco_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot, TupleTableSlot *slot, CommandId cid, LockTupleMode mode, LockWaitPolicy wait_policy, uint8 flags, TM_FailureData *tmfd) { - /* GPDB_12_MERGE_FIXME: not supported. Can this function be left out completely? Or ereport()? */ - elog(ERROR, "speculative insertion not supported on AO tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("tuple locking is not supported on appendoptimized tables"))); } static void diff --git a/src/backend/access/appendonly/appendonlyam_handler.c b/src/backend/access/appendonly/appendonlyam_handler.c index 4f4fd969ddb..c86c143963b 100644 --- a/src/backend/access/appendonly/appendonlyam_handler.c +++ b/src/backend/access/appendonly/appendonlyam_handler.c @@ -1131,14 +1131,38 @@ appendonly_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slo return result; } +/* + * This API is called for a variety of purposes, which are either not supported + * for AO/CO tables or not supported for GPDB in general: + * + * (1) UPSERT: ExecOnConflictUpdate() calls this, but clearly upsert is not + * supported for AO/CO tables. + * + * (2) DELETE and UPDATE triggers: GetTupleForTrigger() calls this, but clearly + * these trigger types are not supported for AO/CO tables. + * + * (3) Logical replication: RelationFindReplTupleByIndex() and + * RelationFindReplTupleSeq() calls this, but clearly we don't support logical + * replication yet for GPDB. + * + * (4) For DELETEs/UPDATEs, when a state of TM_Updated is returned from + * table_tuple_delete() and table_tuple_update() respectively, this API is invoked. + * However, that is impossible for AO/CO tables as an AO/CO tuple cannot be + * deleted/updated while another transaction is updating it (see CdbTryOpenTable()). + * + * (5) Row-level locking (SELECT FOR ..): ExecLockRows() calls this but a plan + * containing the LockRows plan node is never generated for AO/CO tables. In fact, + * we lock at the table level instead. + */ static TM_Result appendonly_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot, TupleTableSlot *slot, CommandId cid, LockTupleMode mode, LockWaitPolicy wait_policy, uint8 flags, TM_FailureData *tmfd) { - /* GPDB_12_MERGE_FIXME: not supported. Can this function be left out completely? Or ereport()? */ - elog(ERROR, "speculative insertion not supported on AO tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("tuple locking is not supported on appendoptimized tables"))); } static void From 27b87009bae79b5196f53248983d15e16e7f423b Mon Sep 17 00:00:00 2001 From: Chen Mulong Date: Mon, 19 Dec 2022 09:53:10 +0800 Subject: [PATCH 07/13] Clean up compilation warnings coming from PL/Perl with clang-12~ (#14657) clang-12 has introduced -Wcompound-token-split-by-macro, that is causing a large amount of warnings when building PL/Perl because of its interactions with upstream Perl. This commit adds one -Wno to CFLAGS at ./configure time if the flag is supported by the compiler to silence all those warnings. Upstream perl has fixed this issue, but it is going to take some time before this is spread across the buildfarm, and we have noticed that some animals would be useful with an extra -Werror to help with the detection of incorrect placeholders (see b0cf544), dangomushi being one. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/YYr3qYa/R3Gw+Sbg@paquier.xyz Backpatch-through: 10 (cherry picked from commit 9ff47ea414c4e6ace347fc4e59866e38b9bbceaf) Co-authored-by: Michael Paquier --- configure | 3 ++- configure.ac | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure b/configure index e7f74042f67..6a7e4103eef 100755 --- a/configure +++ b/configure @@ -6693,7 +6693,8 @@ fi CFLAGS="$CFLAGS -Wno-unused-command-line-argument" fi # Remove clang 12+'s compound-token-split-by-macro, as this causes a lot - # of warnings when building plperl because of usages in the Perl headers. + # of warnings when building plperl because of Perl. Like previously, test + # for the positive form and add the negative form NOT_THE_CFLAGS="" { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wcompound-token-split-by-macro, for NOT_THE_CFLAGS" >&5 $as_echo_n "checking whether ${CC} supports -Wcompound-token-split-by-macro, for NOT_THE_CFLAGS... " >&6; } diff --git a/configure.ac b/configure.ac index 80fc20d4513..aee14b35da4 100644 --- a/configure.ac +++ b/configure.ac @@ -606,7 +606,8 @@ if test "$GCC" = yes -a "$ICC" = no; then CFLAGS="$CFLAGS -Wno-unused-command-line-argument" fi # Remove clang 12+'s compound-token-split-by-macro, as this causes a lot - # of warnings when building plperl because of usages in the Perl headers. + # of warnings when building plperl because of Perl. Like previously, test + # for the positive form and add the negative form NOT_THE_CFLAGS="" PGAC_PROG_CC_VAR_OPT(NOT_THE_CFLAGS, [-Wcompound-token-split-by-macro]) if test -n "$NOT_THE_CFLAGS"; then From 58b8f04ef6e65c9f380d33b317eed6e0403b5611 Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty Date: Tue, 20 Dec 2022 11:44:08 -0800 Subject: [PATCH 08/13] ao/co: Ban speculative insert in parse analysis We don't support speculative insert (INSERT ON CONFLICT) for AO/CO tables yet. This commit does two things: (1) Confirms that its okay to keep the speculative insert APIs unimplemented, by removing the FIXMEs and adding ereport(). (2) Instead of relying on the ERROR at the table AM layer, it introduces a check at the parse analysis stage to perform the ban. This is important as we can't rely on the table AM layer always. For instance, with DO NOTHING, if there is no conflict, the table AM routine is invoked. However, if there is a conflict, then the routine doesn't get invoked and the customer would see: INSERT INTO spec_insert VALUES(1, 3) ON CONFLICT(i) DO NOTHING; INSERT 0 0 This would lead to a false impression that it is supported. So, we ban it at the parse analysis stage. --- src/backend/access/aocs/aocsam_handler.c | 15 ++++++++++++--- .../access/appendonly/appendonlyam_handler.c | 15 ++++++++++++--- src/backend/parser/analyze.c | 6 ++++++ src/test/regress/input/uao_dml/uao_dml.source | 10 ++++++++++ src/test/regress/output/uao_dml/uao_dml.source | 13 +++++++++++++ 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/backend/access/aocs/aocsam_handler.c b/src/backend/access/aocs/aocsam_handler.c index 3ebf5d6a82d..f8466269ef7 100644 --- a/src/backend/access/aocs/aocsam_handler.c +++ b/src/backend/access/aocs/aocsam_handler.c @@ -1058,20 +1058,29 @@ aoco_tuple_insert(Relation relation, TupleTableSlot *slot, CommandId cid, pgstat_count_heap_insert(relation, 1); } +/* + * We don't support speculative inserts on appendoptimized tables, i.e. we don't + * support INSERT ON CONFLICT DO NOTHING or INSERT ON CONFLICT DO UPDATE. Thus, + * the following functions are left unimplemented. + */ + static void aoco_tuple_insert_speculative(Relation relation, TupleTableSlot *slot, CommandId cid, int options, BulkInsertState bistate, uint32 specToken) { - /* GPDB_12_MERGE_FIXME: not supported. Can this function be left out completely? Or ereport()? */ - elog(ERROR, "speculative insertion not supported on AO_COLUMN tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("speculative insert is not supported on appendoptimized relations"))); } static void aoco_tuple_complete_speculative(Relation relation, TupleTableSlot *slot, uint32 specToken, bool succeeded) { - elog(ERROR, "speculative insertion not supported on AO_COLUMN tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("speculative insert is not supported on appendoptimized relations"))); } /* diff --git a/src/backend/access/appendonly/appendonlyam_handler.c b/src/backend/access/appendonly/appendonlyam_handler.c index c86c143963b..f1b07d2adaf 100644 --- a/src/backend/access/appendonly/appendonlyam_handler.c +++ b/src/backend/access/appendonly/appendonlyam_handler.c @@ -928,20 +928,29 @@ appendonly_tuple_insert(Relation relation, TupleTableSlot *slot, CommandId cid, appendonly_free_memtuple(mtuple); } +/* + * We don't support speculative inserts on appendoptimized tables, i.e. we don't + * support INSERT ON CONFLICT DO NOTHING or INSERT ON CONFLICT DO UPDATE. Thus, + * the following functions are left unimplemented. + */ + static void appendonly_tuple_insert_speculative(Relation relation, TupleTableSlot *slot, CommandId cid, int options, BulkInsertState bistate, uint32 specToken) { - /* GPDB_12_MERGE_FIXME: not supported. Can this function be left out completely? Or ereport()? */ - elog(ERROR, "speculative insertion not supported on AO tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("speculative insert is not supported on appendoptimized relations"))); } static void appendonly_tuple_complete_speculative(Relation relation, TupleTableSlot *slot, uint32 specToken, bool succeeded) { - elog(ERROR, "speculative insertion not supported on AO tables"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("speculative insert is not supported on appendoptimized relations"))); } /* diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 60dd13744a2..b794228959b 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -702,6 +702,12 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) icolumns = checkInsertTargets(pstate, stmt->cols, &attrnos); Assert(list_length(icolumns) == list_length(attrnos)); + /* GPDB: We don't support speculative insert for AO/CO tables yet */ + if (RelationIsAppendOptimized(pstate->p_target_relation) && stmt->onConflictClause) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("INSERT ON CONFLICT is not supported for appendoptimized relations")); + /* * Determine which variant of INSERT we have. */ diff --git a/src/test/regress/input/uao_dml/uao_dml.source b/src/test/regress/input/uao_dml/uao_dml.source index adb9f033b70..b51faff222d 100644 --- a/src/test/regress/input/uao_dml/uao_dml.source +++ b/src/test/regress/input/uao_dml/uao_dml.source @@ -1015,3 +1015,13 @@ explain (costs off) select * from taocs where c4>5 and c3>3 and c2!=12 and c4<15 and c2!=10 and c3<15 and c2>1 and c2<15 and c2+c3!=28 and gp_segment_id=1; select * from taocs where c4>5 and c3>3 and c2!=12 and c4<15 and c2!=10 and c3<15 and c2>1 and c2<15 and c2+c3!=28 and gp_segment_id=1; +-- @Description Tests that speculative inserts are not supported (for +-- conflicting inserts and otherwise). +-- +CREATE TABLE spec_insert(i int unique, j int) distributed by (i); +INSERT INTO spec_insert VALUES(1, 2); +INSERT INTO spec_insert VALUES(1, 3) ON CONFLICT(i) DO NOTHING; +INSERT INTO spec_insert VALUES(1, 3) ON CONFLICT(i) DO UPDATE SET j = 3; + +INSERT INTO spec_insert VALUES(2, 3) ON CONFLICT(i) DO NOTHING; +INSERT INTO spec_insert VALUES(2, 3) ON CONFLICT(i) DO UPDATE SET j = 4; diff --git a/src/test/regress/output/uao_dml/uao_dml.source b/src/test/regress/output/uao_dml/uao_dml.source index f07b1b8de1e..67653949c5e 100644 --- a/src/test/regress/output/uao_dml/uao_dml.source +++ b/src/test/regress/output/uao_dml/uao_dml.source @@ -1929,3 +1929,16 @@ select * from taocs where c4>5 and c3>3 and c2!=12 and c4<15 and c2!=10 and c3<1 1 | 11 | 11 | 11 | 11 (4 rows) +-- @Description Tests that speculative inserts are not supported (for +-- conflicting inserts and otherwise). +-- +CREATE TABLE spec_insert(i int unique, j int) distributed by (i); +INSERT INTO spec_insert VALUES(1, 2); +INSERT INTO spec_insert VALUES(1, 3) ON CONFLICT(i) DO NOTHING; +ERROR: INSERT ON CONFLICT is not supported for appendoptimized relations +INSERT INTO spec_insert VALUES(1, 3) ON CONFLICT(i) DO UPDATE SET j = 3; +ERROR: INSERT ON CONFLICT is not supported for appendoptimized relations +INSERT INTO spec_insert VALUES(2, 3) ON CONFLICT(i) DO NOTHING; +ERROR: INSERT ON CONFLICT is not supported for appendoptimized relations +INSERT INTO spec_insert VALUES(2, 3) ON CONFLICT(i) DO UPDATE SET j = 4; +ERROR: INSERT ON CONFLICT is not supported for appendoptimized relations From ee171a4ae611131adb2d767e4b1ac9b7ba239fcf Mon Sep 17 00:00:00 2001 From: Adam Lee Date: Thu, 15 Dec 2022 10:15:03 +0800 Subject: [PATCH 09/13] Remove the generated 'stdin' and 'stdout' files `\!rm` to test if they are located in the right directory. --- src/test/regress/.gitignore | 9 ++------- src/test/regress/input/gpcopy.source | 2 ++ src/test/regress/output/gpcopy.source | 2 ++ src/test/regress/stdin | 2 ++ src/test/regress/stdout | 4 ++++ 5 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 src/test/regress/stdin create mode 100644 src/test/regress/stdout diff --git a/src/test/regress/.gitignore b/src/test/regress/.gitignore index 52c43864130..427c0a901fe 100644 --- a/src/test/regress/.gitignore +++ b/src/test/regress/.gitignore @@ -9,13 +9,13 @@ # Configure-generated files /GPTest.pm - # ignore test tablespaces /testtablespace_default_tablespace/ /testtablespace_temp_tablespace/ twophase_pqexecparams +data/part_ext.tbl data/wet_csv?.tbl data/wet_text?.tbl data/wet_syntax?.tbl @@ -25,12 +25,7 @@ data/gpsd-without-hll.sql data/gpsd-with-hll.sql data/minirepro.sql -regression.diffs -regression.out - -stdin -stdout -# Note: regression.* are only left behind on a failure; that's why they're not ignored +# Note: regreesion.* are only left behind on a failure; that's why they're not ignored #/regression.diffs #/regression.out diff --git a/src/test/regress/input/gpcopy.source b/src/test/regress/input/gpcopy.source index 12f0baa34c3..c06f9ef1130 100644 --- a/src/test/regress/input/gpcopy.source +++ b/src/test/regress/input/gpcopy.source @@ -703,6 +703,8 @@ COPY segment_reject_limit_from to STDOUT log errors segment reject limit 3 rows; \copy segment_reject_limit_from from 'stdin'; \copy segment_reject_limit_from to 'stdout'; \copy segment_reject_limit_from from 'stdout'; +\!rm stdin +\!rm stdout -- gp_initial_bad_row_limit guc test. This guc allows user to set the initial -- number of rows which can contain errors before the database stops loading diff --git a/src/test/regress/output/gpcopy.source b/src/test/regress/output/gpcopy.source index b1ef7e5af69..0870ce7c414 100755 --- a/src/test/regress/output/gpcopy.source +++ b/src/test/regress/output/gpcopy.source @@ -748,6 +748,8 @@ ERROR: COPY single row error handling only available using COPY FROM \copy segment_reject_limit_from from 'stdin'; \copy segment_reject_limit_from to 'stdout'; \copy segment_reject_limit_from from 'stdout'; +\!rm stdin +\!rm stdout -- gp_initial_bad_row_limit guc test. This guc allows user to set the initial -- number of rows which can contain errors before the database stops loading -- the data. If there is a valid row within the first 'n' rows specified by diff --git a/src/test/regress/stdin b/src/test/regress/stdin new file mode 100644 index 00000000000..14df008570e --- /dev/null +++ b/src/test/regress/stdin @@ -0,0 +1,2 @@ +1 1 +1 2 diff --git a/src/test/regress/stdout b/src/test/regress/stdout new file mode 100644 index 00000000000..d03c99dafdb --- /dev/null +++ b/src/test/regress/stdout @@ -0,0 +1,4 @@ +1 1 +1 2 +1 1 +1 2 From 9851b3d0bdb4d0ab74ae57bd3ef899cfee552b71 Mon Sep 17 00:00:00 2001 From: QingMa Date: Wed, 21 Dec 2022 17:20:36 +0800 Subject: [PATCH 10/13] Remove GPDB_12_MERGE_FIXME in cdb_create_multistage_grouping_paths() (#14556) We used to have the same logic between create_grouping_paths() and cdb_create_multistage_grouping_paths() to determine if the input is hashable / sortable. ``` can_sort = grouping_is_sortable(parse->groupClause); can_hash = (parse->groupClause != NIL && agg_costs->numPureOrderedAggs == 0 && grouping_is_hashable(parse->groupClause)); ``` The logic in create_grouping_paths() has been changed by commit b5635948ab165b6070e7d05d111f966e07570d81 because if we have grouping sets we might be able to sort / hash some but not all of them. ``` can_sort = (gd && gd->rollups != NIL) || grouping_is_sortable(parse->groupClause); can_hash = (parse->groupClause != NIL && parse->groupingSets == NIL && agg_costs->numOrderedAggs == 0 && (gd ? gd->any_hashable : grouping_is_hashable(parse->groupClause))); ``` It's not necessary to support multistage grouping sets(unhashable_col, unsortable_col). Ordinary grouping path can cover such corner cases. So just remove this merge fixme. --- src/backend/cdb/cdbgroupingpaths.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/backend/cdb/cdbgroupingpaths.c b/src/backend/cdb/cdbgroupingpaths.c index c9870444337..4f050433dc5 100644 --- a/src/backend/cdb/cdbgroupingpaths.c +++ b/src/backend/cdb/cdbgroupingpaths.c @@ -289,11 +289,7 @@ cdb_create_multistage_grouping_paths(PlannerInfo *root, /* * Is the input hashable / sortable? This is largely the same logic as in * upstream create_grouping_paths(), but we can do hashing in limited ways - * even if there are DISTINCT aggs or grouping setst. - * - * GPDB_12_MERGE:FIXME: the similar rules in planner.c got more complicated. - * Does this need to be more fine-grained too? See GROUPING_CAN_USE_SORT and - * GROUPING_CAN_USE_HASH. + * even if there are DISTINCT aggs or grouping sets. */ can_sort = grouping_is_sortable(parse->groupClause); can_hash = (parse->groupClause != NIL && From 0e958d8249f0e2746e5a7e7c7b98bdc0c4afb57e Mon Sep 17 00:00:00 2001 From: Huansong Fu Date: Fri, 16 Dec 2022 08:52:15 -0800 Subject: [PATCH 11/13] Fix test_consume_xids where it consumes +1 to what's intended test_consume_xids(n int) is supposed to consume n xids but it actually consumes n+1. The reason is that we initialize 'xid' to be the next xid but during advancing it we'll rewrite it to be the xid that we just allocated (the "current xid"). Fixing it by making its meaning consistent. --- src/test/regress/regress_gp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/regress/regress_gp.c b/src/test/regress/regress_gp.c index 9dbcdbe4b3e..21800e95844 100644 --- a/src/test/regress/regress_gp.c +++ b/src/test/regress/regress_gp.c @@ -1167,7 +1167,8 @@ test_consume_xids(PG_FUNCTION_ARGS) xid = ReadNextTransactionId(); - targetxid = xid + nxids; + /* xid is the "next xid" now, so minus one here */ + targetxid = xid + nxids - 1; while (targetxid < FirstNormalTransactionId) targetxid++; From 03f5f34bea44edfa17d1ba20c1e9bc019b3859fb Mon Sep 17 00:00:00 2001 From: Soumyadeep Chakraborty Date: Tue, 27 Dec 2022 10:00:39 -0800 Subject: [PATCH 12/13] dsnap: Remove dead function GetCurrentDistributedSnapshotWithLocalMapping() has been dead since b3f300b9457. --- src/backend/utils/time/snapmgr.c | 13 ------------- src/include/utils/snapmgr.h | 1 - 2 files changed, 14 deletions(-) diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 514fe5d1f7e..ddf2194eead 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -2842,16 +2842,3 @@ XidInMVCCSnapshot_Local(TransactionId xid, Snapshot snapshot) return false; } - -DistributedSnapshotWithLocalMapping * -GetCurrentDistributedSnapshotWithLocalMapping() -{ - if (!FirstSnapshotSet) - return NULL; - - Assert(CurrentSnapshot); - if (CurrentSnapshot->haveDistribSnapshot) - return &CurrentSnapshot->distribSnapshotWithLocalMapping; - - return NULL; -} diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 5a93cada7c7..83b2beb6e7a 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -132,7 +132,6 @@ extern void AtSubAbort_Snapshot(int level); extern void AtEOXact_Snapshot(bool isCommit, bool resetXmin); extern void LogDistributedSnapshotInfo(Snapshot snapshot, const char *prefix); -extern DistributedSnapshotWithLocalMapping *GetCurrentDistributedSnapshotWithLocalMapping(void); extern void ImportSnapshot(const char *idstr); extern bool XactHasExportedSnapshots(void); From 4ea17cb7e05991df241e632cb499097a79901857 Mon Sep 17 00:00:00 2001 From: Zhang Mingli Date: Mon, 30 Dec 2024 19:57:31 +0800 Subject: [PATCH 13/13] Fix test cases of cherry-pick. --- src/test/regress/expected/gp_recursive_cte.out | 1 + src/test/regress/expected/with_optimizer.out | 6 +++--- src/test/singlenode_regress/expected/with.out | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/regress/expected/gp_recursive_cte.out b/src/test/regress/expected/gp_recursive_cte.out index 00b08e4b84c..e0f47dac814 100644 --- a/src/test/regress/expected/gp_recursive_cte.out +++ b/src/test/regress/expected/gp_recursive_cte.out @@ -770,6 +770,7 @@ select id,name from cte; 4 | AAA>B1>C1_2 5 | AAA>B1>C1_3 (9 rows) + -- WTIH RECURSIVE and subquery with recursive cte (n) as ( diff --git a/src/test/regress/expected/with_optimizer.out b/src/test/regress/expected/with_optimizer.out index fa150c6aa54..ec7b6c6ce8b 100644 --- a/src/test/regress/expected/with_optimizer.out +++ b/src/test/regress/expected/with_optimizer.out @@ -1964,9 +1964,9 @@ WITH RECURSIVE foo(i) AS UNION ALL SELECT i+1 FROM foo WHERE i < 5) AS t ) SELECT * FROM foo; -ERROR: recursive reference to query "foo" must not appear within a subquery -LINE 5: (SELECT i+1 FROM foo WHERE i < 10 - ^ +ERROR: recursive reference to query "foo" must not appear more than once +LINE 7: SELECT i+1 FROM foo WHERE i < 5) AS t + ^ WITH RECURSIVE foo(i) AS (values (1) UNION ALL diff --git a/src/test/singlenode_regress/expected/with.out b/src/test/singlenode_regress/expected/with.out index 886674bcf5f..a583889351d 100644 --- a/src/test/singlenode_regress/expected/with.out +++ b/src/test/singlenode_regress/expected/with.out @@ -1948,9 +1948,9 @@ WITH RECURSIVE foo(i) AS UNION ALL SELECT i+1 FROM foo WHERE i < 5) AS t ) SELECT * FROM foo; -ERROR: recursive reference to query "foo" must not appear within a subquery -LINE 5: (SELECT i+1 FROM foo WHERE i < 10 - ^ +ERROR: recursive reference to query "foo" must not appear more than once +LINE 7: SELECT i+1 FROM foo WHERE i < 5) AS t + ^ WITH RECURSIVE foo(i) AS (values (1) UNION ALL