Skip to content

Commit

Permalink
Fix PG community merge tests (pgmerge)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tejeswarm committed Dec 5, 2024
1 parent 6be0649 commit 97115bf
Show file tree
Hide file tree
Showing 5 changed files with 2,216 additions and 2 deletions.
26 changes: 26 additions & 0 deletions src/backend/distributed/commands/multi_copy.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ static LocalCopyStatus GetLocalCopyStatus(void);
static bool ShardIntervalListHasLocalPlacements(List *shardIntervalList);
static void LogLocalCopyToRelationExecution(uint64 shardId);
static void LogLocalCopyToFileExecution(uint64 shardId);
#if PG_VERSION_NUM >= PG_VERSION_15
static void ErrorIfMergeInCopy(CopyStmt *copyStatement);
#endif


/* exports for SQL callable functions */
Expand Down Expand Up @@ -2828,6 +2831,25 @@ CopyStatementHasFormat(CopyStmt *copyStatement, char *formatName)
}


#if PG_VERSION_NUM >= PG_VERSION_15

/*
* ErrorIfMergeInCopy Raises an exception if the MERGE is called in the COPY.
*/
static void
ErrorIfMergeInCopy(CopyStmt *copyStatement)
{
if (!copyStatement->relation && (IsA(copyStatement->query, MergeStmt)))
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

Check warning on line 2844 in src/backend/distributed/commands/multi_copy.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/commands/multi_copy.c#L2844

Added line #L2844 was not covered by tests
errmsg("MERGE not supported in COPY")));
}
}


#endif


/*
* ProcessCopyStmt handles Citus specific concerns for COPY like supporting
* COPYing from distributed tables and preventing unsupported actions. The
Expand All @@ -2838,6 +2860,10 @@ Node *
ProcessCopyStmt(CopyStmt *copyStatement, QueryCompletion *completionTag, const
char *queryString)
{
#if PG_VERSION_NUM >= PG_VERSION_15
ErrorIfMergeInCopy(copyStatement);
#endif

/*
* Handle special COPY "resultid" FROM STDIN WITH (format result) commands
* for sending intermediate results to workers.
Expand Down
17 changes: 17 additions & 0 deletions src/backend/distributed/planner/merge_planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ static DistributedPlan * CreateNonPushableMergePlan(Oid targetRelationId, uint64
plannerRestrictionContext,
ParamListInfo boundParams);
static char * MergeCommandResultIdPrefix(uint64 planId);
static void ErrorIfMergeHasReturningList(Query *query);

#endif

Expand Down Expand Up @@ -949,16 +950,32 @@ ConvertSourceRTEIntoSubquery(Query *mergeQuery, RangeTblEntry *sourceRte,
}


/*
* ErrorIfMergeHasReturningList raises an exception if the MERGE
* has a RETURNING clause.
*/
static void
ErrorIfMergeHasReturningList(Query *query)

Check warning on line 958 in src/backend/distributed/planner/merge_planner.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/planner/merge_planner.c#L958

Added line #L958 was not covered by tests
{
if (query->returningList)
{
ereport(ERROR, (errmsg("MERGE with RETURNING is not yet supported")));

Check warning on line 962 in src/backend/distributed/planner/merge_planner.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/planner/merge_planner.c#L962

Added line #L962 was not covered by tests
}
}


/*
* 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
*/
static void
ErrorIfMergeNotSupported(Query *query, Oid targetRelationId, List *rangeTableList)
{
ErrorIfMergeHasReturningList(query);

Check warning on line 978 in src/backend/distributed/planner/merge_planner.c

View check run for this annotation

Codecov / codecov/patch

src/backend/distributed/planner/merge_planner.c#L978

Added line #L978 was not covered by tests
ErrorIfMergeHasUnsupportedTables(targetRelationId, rangeTableList);
ErrorIfMergeQueryQualAndTargetListNotSupported(targetRelationId, query);
ErrorIfUnsupportedCTEs(query);
Expand Down
12 changes: 12 additions & 0 deletions src/test/regress/expected/pgmerge.out
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,18 @@ COPY (
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 not supported in WITH query
-- 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;
Expand Down
Loading

0 comments on commit 97115bf

Please sign in to comment.