Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PG community merge tests (pgmerge) #7778

Open
wants to merge 2 commits into
base: release-13.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ jobs:
style_checker_image_name: "ghcr.io/citusdata/stylechecker"
style_checker_tools_version: "0.8.18"
sql_snapshot_pg_version: "16.3"
image_suffix: "-v13fd57c"
image_suffix: "-dev-e5b0348"
pg14_version: '{ "major": "14", "full": "14.12" }'
pg15_version: '{ "major": "15", "full": "15.7" }'
pg16_version: '{ "major": "16", "full": "16.3" }'
upgrade_pg_versions: "14.12-15.7-16.3"
pg17_version: '{ "major": "17", "full": "17.0" }'
upgrade_pg_versions: "14.12-15.7-16.3-17.0"
steps:
# Since GHA jobs needs at least one step we use a noop step here.
- name: Set up parameters
Expand Down Expand Up @@ -108,6 +109,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
runs-on: ubuntu-20.04
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ matrix.image_suffix }}"
Expand Down Expand Up @@ -139,6 +141,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
make:
- check-split
- check-multi
Expand Down Expand Up @@ -168,6 +171,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-failure
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -180,6 +187,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -192,6 +203,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
Expand All @@ -200,6 +215,10 @@ jobs:
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg16_version }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg17_version }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -212,6 +231,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
runs-on: ubuntu-20.04
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ needs.params.outputs.image_suffix }}"
Expand Down Expand Up @@ -255,6 +278,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
parallel: [0,1,2,3,4,5] # workaround for running 6 parallel jobs
steps:
- uses: actions/[email protected]
Expand Down Expand Up @@ -303,6 +327,10 @@ jobs:
new_pg_major: 16
- old_pg_major: 14
new_pg_major: 16
- old_pg_major: 16
new_pg_major: 17
- old_pg_major: 15
new_pg_major: 17
env:
old_pg_major: ${{ matrix.old_pg_major }}
new_pg_major: ${{ matrix.new_pg_major }}
Expand Down Expand Up @@ -386,7 +414,7 @@ jobs:
CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }}
runs-on: ubuntu-20.04
container:
image: ${{ needs.params.outputs.test_image_name }}:${{ fromJson(needs.params.outputs.pg16_version).full }}${{ needs.params.outputs.image_suffix }}
image: ${{ needs.params.outputs.test_image_name }}:${{ fromJson(needs.params.outputs.pg17_version).full }}${{ needs.params.outputs.image_suffix }}
needs:
- params
- test-citus
Expand Down Expand Up @@ -481,7 +509,7 @@ jobs:
name: Test flakyness
runs-on: ubuntu-20.04
container:
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ needs.params.outputs.pg16_version }}${{ needs.params.outputs.image_suffix }}
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ needs.params.outputs.pg17_version }}${{ needs.params.outputs.image_suffix }}
options: --user root
env:
runs: 8
Expand Down
2 changes: 1 addition & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -2588,7 +2588,7 @@ fi
if test "$with_pg_version_check" = no; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num (skipped compatibility check)" >&5
$as_echo "$as_me: building against PostgreSQL $version_num (skipped compatibility check)" >&6;}
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16'; then
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
as_fn_error $? "Citus is not compatible with the detected PostgreSQL version ${version_num}." "$LINENO" 5
else
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num" >&5
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ AC_SUBST(with_pg_version_check)

if test "$with_pg_version_check" = no; then
AC_MSG_NOTICE([building against PostgreSQL $version_num (skipped compatibility check)])
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16'; then
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
AC_MSG_ERROR([Citus is not compatible with the detected PostgreSQL version ${version_num}.])
else
AC_MSG_NOTICE([building against PostgreSQL $version_num])
Expand Down
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 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 @@
}


#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 @@
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 @@
plannerRestrictionContext,
ParamListInfo boundParams);
static char * MergeCommandResultIdPrefix(uint64 planId);
static void ErrorIfMergeHasReturningList(Query *query);

#endif

Expand Down Expand Up @@ -949,16 +950,32 @@
}


/*
* 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")));

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);
ErrorIfMergeHasUnsupportedTables(targetRelationId, rangeTableList);
ErrorIfMergeQueryQualAndTargetListNotSupported(targetRelationId, query);
ErrorIfUnsupportedCTEs(query);
Expand Down
1 change: 1 addition & 0 deletions src/test/regress/citus_tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def get_pg_major_version():
14: "10.2.0",
15: "11.1.5",
16: "12.1devel",
17: "12.1devel",
}

OLDEST_SUPPORTED_CITUS_VERSION = OLDEST_SUPPORTED_CITUS_VERSION_MATRIX[PG_MAJOR_VERSION]
Expand Down
14 changes: 13 additions & 1 deletion src/test/regress/expected/pgmerge.out
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,21 @@ 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: syntax error at or near "RETURNING"
-- used in COPY with RETURNING
COPY (
MERGE INTO target USING source ON (true)
WHEN MATCHED THEN DELETE RETURNING target.*
) TO stdout;
ERROR: syntax error at or near "RETURNING"
-- unsupported relation types
-- view
CREATE VIEW tv AS SELECT * FROM target;
CREATE VIEW tv AS SELECT count(tid) AS tid FROM target;
MERGE INTO tv t
USING source s
ON t.tid = s.sid
Expand Down
Loading
Loading