From 3cef7d3f9b6d112f86ede57c9da154dc5b2eb3e1 Mon Sep 17 00:00:00 2001 From: Teja Mupparti Date: Wed, 4 Dec 2024 20:02:32 -0800 Subject: [PATCH] Fix PG community merge tests (pgmerge) pgmerge.sql: This is a clone of the PostgreSQL community's merge.sql test, adapted for Citus by converting tables into Citus local tables. The expectation is that any MERGE syntax that works on PostgreSQL should work on Citus as-is, utilizing our MERGE deparser. Diffs, which primarily seem to stem fromtwo major features in MERGE introduced by the community: RETURNING support for MERGE MERGE support for updatable views Currently, Citus code does not support these features. For now, I have implemented changes to catch these cases and raise clean exceptions. With these adjustments, the pgmerge tests now pass without diffs. --- src/backend/distributed/commands/multi_copy.c | 22 ++++++++++++++++ .../distributed/planner/merge_planner.c | 17 +++++++++++++ src/test/regress/expected/pgmerge.out | 25 ++++++++++++++----- src/test/regress/sql/pgmerge.sql | 14 +++++++++-- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index b9e9058501a..ff3b2dc91c8 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -346,6 +346,7 @@ static LocalCopyStatus GetLocalCopyStatus(void); static bool ShardIntervalListHasLocalPlacements(List *shardIntervalList); static void LogLocalCopyToRelationExecution(uint64 shardId); static void LogLocalCopyToFileExecution(uint64 shardId); +static void ErrorIfMergeInCopy(CopyStmt *copyStatement); /* exports for SQL callable functions */ @@ -2828,6 +2829,25 @@ CopyStatementHasFormat(CopyStmt *copyStatement, char *formatName) } +/* + * ErrorIfMergeInCopy Raises an exception if the MERGE is called in the COPY. + */ +static void +ErrorIfMergeInCopy(CopyStmt *copyStatement) +{ + #if PG_VERSION_NUM < PG_VERSION_15 + /* MERGE is supported from PG 15 onwards */ + return; + #endif + + if (!copyStatement->relation && (IsA(copyStatement->query, MergeStmt))) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("MERGE not supported in COPY"))); + } +} + + /* * ProcessCopyStmt handles Citus specific concerns for COPY like supporting * COPYing from distributed tables and preventing unsupported actions. The @@ -2838,6 +2858,8 @@ Node * ProcessCopyStmt(CopyStmt *copyStatement, QueryCompletion *completionTag, const char *queryString) { + ErrorIfMergeInCopy(copyStatement); + /* * Handle special COPY "resultid" FROM STDIN WITH (format result) commands * for sending intermediate results to workers. diff --git a/src/backend/distributed/planner/merge_planner.c b/src/backend/distributed/planner/merge_planner.c index 8048002e005..32927e92af7 100644 --- a/src/backend/distributed/planner/merge_planner.c +++ b/src/backend/distributed/planner/merge_planner.c @@ -97,6 +97,7 @@ static DistributedPlan * CreateNonPushableMergePlan(Oid targetRelationId, uint64 plannerRestrictionContext, ParamListInfo boundParams); static char * MergeCommandResultIdPrefix(uint64 planId); +static void ErrorIfMergeHasReturningList(Query *query); #endif @@ -949,9 +950,24 @@ ConvertSourceRTEIntoSubquery(Query *mergeQuery, RangeTblEntry *sourceRte, } +/* + * ErrorIfMergeHasReturningList raises an exception if the MERGE + * has a RETURNING clause. + */ +static void +ErrorIfMergeHasReturningList(Query *query) +{ + if (query->returningList) + { + ereport(ERROR, (errmsg("MERGE with RETURNING is not yet supported"))); + } +} + + /* * ErrorIfMergeNotSupported Checks for conditions that are not supported in either * the routable or repartition strategies. It checks for + * - MERGE with a RETURNING clause * - Supported table types and their combinations * - Check the target lists and quals of both the query and merge actions * - Supported CTEs @@ -959,6 +975,7 @@ ConvertSourceRTEIntoSubquery(Query *mergeQuery, RangeTblEntry *sourceRte, static void ErrorIfMergeNotSupported(Query *query, Oid targetRelationId, List *rangeTableList) { + ErrorIfMergeHasReturningList(query); ErrorIfMergeHasUnsupportedTables(targetRelationId, rangeTableList); ErrorIfMergeQueryQualAndTargetListNotSupported(targetRelationId, query); ErrorIfUnsupportedCTEs(query); diff --git a/src/test/regress/expected/pgmerge.out b/src/test/regress/expected/pgmerge.out index a0f5d0c862b..f5e4c504d76 100644 --- a/src/test/regress/expected/pgmerge.out +++ b/src/test/regress/expected/pgmerge.out @@ -167,23 +167,36 @@ WITH foo AS ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) SELECT * FROM foo; -ERROR: MERGE not supported in WITH query +ERROR: WITH query "foo" does not have a RETURNING clause -- used in COPY COPY ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) TO stdout; ERROR: MERGE not supported in COPY +-- used in a CTE with RETURNING +WITH foo AS ( + MERGE INTO target USING source ON (true) + WHEN MATCHED THEN DELETE RETURNING target.* +) SELECT * FROM foo; +ERROR: MERGE with RETURNING is not yet supported +-- used in COPY with RETURNING +COPY ( + MERGE INTO target USING source ON (true) + WHEN MATCHED THEN DELETE RETURNING target.* +) TO stdout; +ERROR: MERGE not supported in COPY -- unsupported relation types --- view -CREATE VIEW tv AS SELECT * FROM target; +-- non-updatable view +CREATE VIEW tv AS SELECT count(tid) AS tid FROM target; MERGE INTO tv t USING source s ON t.tid = s.sid WHEN NOT MATCHED THEN INSERT DEFAULT VALUES; -ERROR: cannot execute MERGE on relation "tv" -DETAIL: This operation is not supported for views. +ERROR: cannot insert into view "tv" +DETAIL: Views that return aggregate functions are not automatically updatable. +HINT: To enable inserting into the view using MERGE, provide an INSTEAD OF INSERT trigger. DROP VIEW tv; -- materialized view CREATE MATERIALIZED VIEW mv AS SELECT * FROM target; @@ -1389,7 +1402,7 @@ WHEN NOT MATCHED THEN WHEN MATCHED AND tid < 2 THEN DELETE RETURNING *; -ERROR: syntax error at or near "RETURNING" +ERROR: MERGE with RETURNING is not yet supported ROLLBACK; -- EXPLAIN CREATE TABLE ex_mtarget (a int, b int) diff --git a/src/test/regress/sql/pgmerge.sql b/src/test/regress/sql/pgmerge.sql index e1f3c7aabc0..3de53123ef7 100644 --- a/src/test/regress/sql/pgmerge.sql +++ b/src/test/regress/sql/pgmerge.sql @@ -126,10 +126,20 @@ COPY ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) TO stdout; +-- used in a CTE with RETURNING +WITH foo AS ( + MERGE INTO target USING source ON (true) + WHEN MATCHED THEN DELETE RETURNING target.* +) SELECT * FROM foo; +-- used in COPY with RETURNING +COPY ( + MERGE INTO target USING source ON (true) + WHEN MATCHED THEN DELETE RETURNING target.* +) TO stdout; -- unsupported relation types --- view -CREATE VIEW tv AS SELECT * FROM target; +-- non-updatable view +CREATE VIEW tv AS SELECT count(tid) AS tid FROM target; MERGE INTO tv t USING source s ON t.tid = s.sid