PG17 - Add tests for MERGE ... WHEN NOT MATCHED BY SOURCE #7807
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-13.0 #7807 +/- ##
===============================================
Coverage ? 89.50%
===============================================
Files ? 274
Lines ? 59989
Branches ? 7506
===============================================
Hits ? 53695
Misses ? 4156
Partials ? 2138 |
| @@ -1547,7 +1547,8 @@ FetchAndValidateInsertVarIfExists(Oid targetRelationId, Query *query) | |||
| } | |||
|
|
|||
| /* NOT MATCHED can have either INSERT or DO NOTHING */ | |||
There was a problem hiding this comment.
Let's combine both the comments into one
naisila
force-pushed
the
naisila/merge
branch
from
333c22e to
012ec64
Compare
| ON t.tid = s.sid | ||
| WHEN MATCHED THEN | ||
| UPDATE SET balance = balance + delta, val = val || ' updated by merge' | ||
| WHEN NOT MATCHED THEN |
There was a problem hiding this comment.
The PG17 commit (0294df2f1) also adds WHEN NOT MATCHED BY TARGET as an optional way to specify the already-supported WHEN NOT MATCHED, so let's check that Citus treats this correctly ? I.e., WHEN NOT MATCHED BY TARGET should have the same behavior as WHEN NOT MATCHED. So how about changing some of the WHEN NOT MATCHED THEN clauses to WHEN NOT MATCHED BY TARGET THEN ?
| INSERT VALUES (sid, delta, 'inserted by merge') | ||
| WHEN NOT MATCHED BY SOURCE THEN | ||
| UPDATE SET val = val || ' not matched by source'; | ||
|
|
There was a problem hiding this comment.
It looks like the MERGE statements here are all have router plans ? (so they take the CreateRouterMergePlan() codepath in merge_planner.c). If so, how about adding some MERGE statements that require repartitioning (take the CreateNonPushableMergePlan() codepath in merge_planner.c) ? Regress test merge.sql has some examples (there's a couple of queries that have a comment "with repartition") and there's also an example in the docs on Merge (see "Exploring MERGE in Complex Scenarios") that might help in constructing a test with the schema here.
This (router vs repartition plan) may be an orthogonal concern, but I thought it was worth mentioning.
There was a problem hiding this comment.
Thanks Colm, definitely worth mentioning. I should test this in my local first, I am not sure if there are any issues with it.
DESCRIPTION: Propagates MERGE ... WHEN NOT MATCHED BY SOURCE It seems like there is not much needed to be done here. `get_merge_query_def` from `ruleutils_17` is updated with "WHEN NOT MATCHED BY SOURCE" therefore `deparse_shard_query` parses the merge query for execution on the shard correctly. Relevant PG commit: postgres/postgres@0294df2f1
DESCRIPTION: Propagates MERGE ... WHEN NOT MATCHED BY SOURCE It seems like there is not much needed to be done here. `get_merge_query_def` from `ruleutils_17` is updated with "WHEN NOT MATCHED BY SOURCE" therefore `deparse_shard_query` parses the merge query for execution on the shard correctly. Relevant PG commit: postgres/postgres@0294df2f1
DESCRIPTION: Propagates MERGE ... WHEN NOT MATCHED BY SOURCE It seems like there is not much needed to be done here. `get_merge_query_def` from `ruleutils_17` is updated with "WHEN NOT MATCHED BY SOURCE" therefore `deparse_shard_query` parses the merge query for execution on the shard correctly. Relevant PG commit: postgres/postgres@0294df2f1
DESCRIPTION: Propagates MERGE ... WHEN NOT MATCHED BY SOURCE It seems like there is not much needed to be done here. `get_merge_query_def` from `ruleutils_17` is updated with "WHEN NOT MATCHED BY SOURCE" therefore `deparse_shard_query` parses the merge query for execution on the shard correctly. Relevant PG commit: postgres/postgres@0294df2f1
DESCRIPTION: Propagates MERGE ... WHEN NOT MATCHED BY SOURCE
It seems like there is not much needed to be done here.
get_merge_query_deffromruleutils_17is updated with "WHEN NOT MATCHED BY SOURCE" thereforedeparse_shard_queryparses the merge query for execution on the shard correctly.Relevant PG commit: postgres/postgres@0294df2f1