public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v48 1/7] sequential scan for dshash 4+ messages / 3 participants [nested] [flat]
* [PATCH v48 1/7] sequential scan for dshash @ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Kyotaro Horiguchi @ 2020-03-13 07:58 UTC (permalink / raw) Dshash did not allow scan the all entries sequentially. This adds the functionality. The interface is similar but a bit different both from that of dynahash and simple dshash search functions. One of the most significant differences is the sequential scan interface of dshash always needs a call to dshash_seq_term when scan ends. Another is locking. Dshash holds partition lock when returning an entry, dshash_seq_next() also holds lock when returning an entry but callers shouldn't release it, since the lock is essential to continue a scan. The seqscan interface allows entry deletion while a scan. The in-scan deletion should be performed by dshash_delete_current(). --- src/backend/lib/dshash.c | 160 ++++++++++++++++++++++++++++++- src/include/lib/dshash.h | 22 +++++ src/tools/pgindent/typedefs.list | 1 + 3 files changed, 182 insertions(+), 1 deletion(-) diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c index e0c763be32..520bfa0979 100644 --- a/src/backend/lib/dshash.c +++ b/src/backend/lib/dshash.c @@ -127,6 +127,10 @@ struct dshash_table #define NUM_SPLITS(size_log2) \ (size_log2 - DSHASH_NUM_PARTITIONS_LOG2) +/* How many buckets are there in a given size? */ +#define NUM_BUCKETS(size_log2) \ + (((size_t) 1) << (size_log2)) + /* How many buckets are there in each partition at a given size? */ #define BUCKETS_PER_PARTITION(size_log2) \ (((size_t) 1) << NUM_SPLITS(size_log2)) @@ -153,6 +157,10 @@ struct dshash_table #define BUCKET_INDEX_FOR_PARTITION(partition, size_log2) \ ((partition) << NUM_SPLITS(size_log2)) +/* Choose partition based on bucket index. */ +#define PARTITION_FOR_BUCKET_INDEX(bucket_idx, size_log2) \ + ((bucket_idx) >> NUM_SPLITS(size_log2)) + /* The head of the active bucket for a given hash value (lvalue). */ #define BUCKET_FOR_HASH(hash_table, hash) \ (hash_table->buckets[ \ @@ -324,7 +332,7 @@ dshash_destroy(dshash_table *hash_table) ensure_valid_bucket_pointers(hash_table); /* Free all the entries. */ - size = ((size_t) 1) << hash_table->size_log2; + size = NUM_BUCKETS(hash_table->size_log2); for (i = 0; i < size; ++i) { dsa_pointer item_pointer = hash_table->buckets[i]; @@ -592,6 +600,156 @@ dshash_memhash(const void *v, size_t size, void *arg) return tag_hash(v, size); } +/* + * dshash_seq_init/_next/_term + * Sequentially scan through dshash table and return all the + * elements one by one, return NULL when no more. + * + * dshash_seq_term should always be called when a scan finished. + * The caller may delete returned elements midst of a scan by using + * dshash_delete_current(). exclusive must be true to delete elements. + */ +void +dshash_seq_init(dshash_seq_status *status, dshash_table *hash_table, + bool exclusive) +{ + status->hash_table = hash_table; + status->curbucket = 0; + status->nbuckets = 0; + status->curitem = NULL; + status->pnextitem = InvalidDsaPointer; + status->curpartition = -1; + status->exclusive = exclusive; +} + +/* + * Returns the next element. + * + * Returned elements are locked and the caller must not explicitly release + * it. It is released at the next call to dshash_next(). + */ +void * +dshash_seq_next(dshash_seq_status *status) +{ + dsa_pointer next_item_pointer; + + if (status->curitem == NULL) + { + int partition; + + Assert(status->curbucket == 0); + Assert(!status->hash_table->find_locked); + + /* first shot. grab the first item. */ + partition = + PARTITION_FOR_BUCKET_INDEX(status->curbucket, + status->hash_table->size_log2); + LWLockAcquire(PARTITION_LOCK(status->hash_table, partition), + status->exclusive ? LW_EXCLUSIVE : LW_SHARED); + status->curpartition = partition; + + /* resize doesn't happen from now until seq scan ends */ + status->nbuckets = + NUM_BUCKETS(status->hash_table->control->size_log2); + ensure_valid_bucket_pointers(status->hash_table); + + next_item_pointer = status->hash_table->buckets[status->curbucket]; + } + else + next_item_pointer = status->pnextitem; + + Assert(LWLockHeldByMeInMode(PARTITION_LOCK(status->hash_table, + status->curpartition), + status->exclusive ? LW_EXCLUSIVE : LW_SHARED)); + + /* Move to the next bucket if we finished the current bucket */ + while (!DsaPointerIsValid(next_item_pointer)) + { + int next_partition; + + if (++status->curbucket >= status->nbuckets) + { + /* all buckets have been scanned. finish. */ + return NULL; + } + + /* Check if move to the next partition */ + next_partition = + PARTITION_FOR_BUCKET_INDEX(status->curbucket, + status->hash_table->size_log2); + + if (status->curpartition != next_partition) + { + /* + * Move to the next partition. Lock the next partition then release + * the current, not in the reverse order to avoid concurrent + * resizing. Avoid dead lock by taking lock in the same order + * with resize(). + */ + LWLockAcquire(PARTITION_LOCK(status->hash_table, + next_partition), + status->exclusive ? LW_EXCLUSIVE : LW_SHARED); + LWLockRelease(PARTITION_LOCK(status->hash_table, + status->curpartition)); + status->curpartition = next_partition; + } + + next_item_pointer = status->hash_table->buckets[status->curbucket]; + } + + status->curitem = + dsa_get_address(status->hash_table->area, next_item_pointer); + status->hash_table->find_locked = true; + status->hash_table->find_exclusively_locked = status->exclusive; + + /* + * The caller may delete the item. Store the next item in case of deletion. + */ + status->pnextitem = status->curitem->next; + + return ENTRY_FROM_ITEM(status->curitem); +} + +/* + * Terminates the seqscan and release all locks. + * + * Should be always called when finishing or exiting a seqscan. + */ +void +dshash_seq_term(dshash_seq_status *status) +{ + status->hash_table->find_locked = false; + status->hash_table->find_exclusively_locked = false; + + if (status->curpartition >= 0) + LWLockRelease(PARTITION_LOCK(status->hash_table, status->curpartition)); +} + +/* Remove the current entry while a seq scan. */ +void +dshash_delete_current(dshash_seq_status *status) +{ + dshash_table *hash_table = status->hash_table; + dshash_table_item *item = status->curitem; + size_t partition = PARTITION_FOR_HASH(item->hash); + + Assert(status->exclusive); + Assert(hash_table->control->magic == DSHASH_MAGIC); + Assert(hash_table->find_locked); + Assert(hash_table->find_exclusively_locked); + Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table, partition), + LW_EXCLUSIVE)); + + delete_item(hash_table, item); +} + +/* Get the current entry while a seq scan. */ +void * +dshash_get_current(dshash_seq_status *status) +{ + return ENTRY_FROM_ITEM(status->curitem); +} + /* * Print debugging information about the internal state of the hash table to * stderr. The caller must hold no partition locks. diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h index c069ec9de7..a6ea377173 100644 --- a/src/include/lib/dshash.h +++ b/src/include/lib/dshash.h @@ -59,6 +59,21 @@ typedef struct dshash_parameters struct dshash_table_item; typedef struct dshash_table_item dshash_table_item; +/* + * Sequential scan state. The detail is exposed to let users know the storage + * size but it should be considered as an opaque type by callers. + */ +typedef struct dshash_seq_status +{ + dshash_table *hash_table; /* dshash table working on */ + int curbucket; /* bucket number we are at */ + int nbuckets; /* total number of buckets in the dshash */ + dshash_table_item *curitem; /* item we are currently at */ + dsa_pointer pnextitem; /* dsa-pointer to the next item */ + int curpartition; /* partition number we are at */ + bool exclusive; /* locking mode */ +} dshash_seq_status; + /* Creating, sharing and destroying from hash tables. */ extern dshash_table *dshash_create(dsa_area *area, const dshash_parameters *params, @@ -80,6 +95,13 @@ extern bool dshash_delete_key(dshash_table *hash_table, const void *key); extern void dshash_delete_entry(dshash_table *hash_table, void *entry); extern void dshash_release_lock(dshash_table *hash_table, void *entry); +/* seq scan support */ +extern void dshash_seq_init(dshash_seq_status *status, dshash_table *hash_table, + bool exclusive); +extern void *dshash_seq_next(dshash_seq_status *status); +extern void dshash_seq_term(dshash_seq_status *status); +extern void dshash_delete_current(dshash_seq_status *status); +extern void *dshash_get_current(dshash_seq_status *status); /* Convenience hash and compare functions wrapping memcmp and tag_hash. */ extern int dshash_memcmp(const void *a, const void *b, size_t size, void *arg); extern dshash_hash dshash_memhash(const void *v, size_t size, void *arg); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 8bd95aefa1..2b591e94e6 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2954,6 +2954,7 @@ dshash_hash dshash_hash_function dshash_parameters dshash_partition +dshash_seq_status dshash_table dshash_table_control dshash_table_handle -- 2.27.0 ----Next_Part(Fri_Mar__5_17_18_56_2021_497)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v48-0002-Add-conditional-lock-feature-to-dshash.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: MERGE ... RETURNING @ 2023-01-22 10:09 Dean Rasheed <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Dean Rasheed @ 2023-01-22 10:09 UTC (permalink / raw) To: Vik Fearing <[email protected]>; +Cc: Isaac Morland <[email protected]>; PostgreSQL Hackers <[email protected]> On Mon, 9 Jan 2023 at 17:44, Dean Rasheed <[email protected]> wrote: > > On Mon, 9 Jan 2023 at 16:23, Vik Fearing <[email protected]> wrote: > > > > Bikeshedding here. Instead of Yet Another WITH Clause, could we perhaps > > make a MERGING() function analogous to the GROUPING() function that goes > > with grouping sets? > > > > MERGE ... > > RETURNING *, MERGING('clause'), MERGING('action'); > > > > Hmm, possibly, but I think that would complicate the implementation quite a bit. > > GROUPING() is not really a function (in the sense that there is no > pg_proc entry for it, you can't do "\df grouping", and it isn't > executed with its arguments like a normal function). Rather, it > requires special-case handling in the parser, through to the executor, > and I think MERGING() would be similar. > > Also, it masks any user function with the same name, and would > probably require MERGING to be some level of reserved keyword. > I thought about this some more, and I think functions do make more sense here, rather than inventing a special WITH syntax. However, rather than using a special MERGING() function like GROUPING(), which isn't really a function at all, I think it's better (and much simpler to implement) to have a pair of normal functions (one returning int, and one text). The example from the tests shows the sort of thing this allows: MERGE INTO sq_target t USING sq_source s ON tid = sid WHEN MATCHED AND tid >= 2 THEN UPDATE SET balance = t.balance + delta WHEN NOT MATCHED THEN INSERT (balance, tid) VALUES (balance + delta, sid) WHEN MATCHED AND tid < 2 THEN DELETE RETURNING pg_merge_when_clause() AS when_clause, pg_merge_action() AS merge_action, t.*, CASE pg_merge_action() WHEN 'INSERT' THEN 'Inserted '||t WHEN 'UPDATE' THEN 'Added '||delta||' to balance' WHEN 'DELETE' THEN 'Removed '||t END AS description; when_clause | merge_action | tid | balance | description -------------+--------------+-----+---------+--------------------- 3 | DELETE | 1 | 100 | Removed (1,100) 1 | UPDATE | 2 | 220 | Added 20 to balance 2 | INSERT | 4 | 40 | Inserted (4,40) (3 rows) I think this is easier to use than the WITH syntax, and more flexible, since the new functions can be used anywhere in the RETURNING list, including in expressions. There is one limitation though. Due to the way these functions need access to the originating query, they need to appear directly in MERGE's RETURNING list, not in subqueries, plpgsql function bodies, or anything else that amounts to a different query. Maybe there's a way round that, but it looks tricky. In practice though, it's easy to work around, if necessary (e.g., by wrapping the MERGE in a CTE). Regards, Dean Attachments: [text/x-patch] v2-POC-support-MERGE-RETURNING.patch (38.2K, ../../CAEZATCViD_PvYUP0scY_LAbm5Ueg0Z4A4xbdDkKYo2eWO4hAuw@mail.gmail.com/2-v2-POC-support-MERGE-RETURNING.patch) download | inline diff: diff --git a/doc/src/sgml/ref/merge.sgml b/doc/src/sgml/ref/merge.sgml new file mode 100644 index 0995fe0..bedb2c8 --- a/doc/src/sgml/ref/merge.sgml +++ b/doc/src/sgml/ref/merge.sgml @@ -25,6 +25,7 @@ PostgreSQL documentation MERGE INTO [ ONLY ] <replaceable class="parameter">target_table_name</replaceable> [ * ] [ [ AS ] <replaceable class="parameter">target_alias</replaceable> ] USING <replaceable class="parameter">data_source</replaceable> ON <replaceable class="parameter">join_condition</replaceable> <replaceable class="parameter">when_clause</replaceable> [...] +[ RETURNING * | <replaceable class="parameter">output_expression</replaceable> [ [ AS ] <replaceable class="parameter">output_name</replaceable> ] [, ...] ] <phrase>where <replaceable class="parameter">data_source</replaceable> is:</phrase> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c new file mode 100644 index e34f583..aa3cca0 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -274,12 +274,6 @@ DoCopy(ParseState *pstate, const CopyStm { Assert(stmt->query); - /* MERGE is allowed by parser, but unimplemented. Reject for now */ - if (IsA(stmt->query, MergeStmt)) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("MERGE not supported in COPY")); - query = makeNode(RawStmt); query->stmt = stmt->query; query->stmt_location = stmt_location; diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c new file mode 100644 index 8043b4e..e02d7d0 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -510,7 +510,8 @@ BeginCopyTo(ParseState *pstate, { Assert(query->commandType == CMD_INSERT || query->commandType == CMD_UPDATE || - query->commandType == CMD_DELETE); + query->commandType == CMD_DELETE || + query->commandType == CMD_MERGE); ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c new file mode 100644 index 812ead9..8572b01 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -48,6 +48,7 @@ #include "utils/array.h" #include "utils/builtins.h" #include "utils/datum.h" +#include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/typcache.h" @@ -2485,6 +2486,22 @@ ExecInitFunc(ExprEvalStep *scratch, Expr InitFunctionCallInfoData(*fcinfo, flinfo, nargs, inputcollid, NULL, NULL); + /* + * Merge support functions should only be called directly from a MERGE + * command, and need access to the parent ModifyTableState. The parser + * should have checked that such functions only appear in the RETURNING + * list of a MERGE, so this should never fail. + */ + if (IsMergeSupportFunction(funcid)) + { + if (!state->parent || + !IsA(state->parent, ModifyTableState) || + ((ModifyTableState *) state->parent)->operation != CMD_MERGE) + elog(ERROR, "merge support function called in non-merge context"); + + fcinfo->context = (Node *) state->parent; + } + /* Keep extra copies of this info to save an indirection at runtime */ scratch->d.func.fn_addr = flinfo->fn_addr; scratch->d.func.nargs = nargs; diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c new file mode 100644 index 651ad24..3391269 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -612,6 +612,9 @@ ExecInitPartitionInfo(ModifyTableState * * case or in the case of UPDATE tuple routing where we didn't find a * result rel to reuse. */ + + /* XXX: What about the MERGE case ??? */ + if (node && node->returningLists != NIL) { TupleTableSlot *slot; @@ -877,6 +880,7 @@ ExecInitPartitionInfo(ModifyTableState * List *firstMergeActionList = linitial(node->mergeActionLists); ListCell *lc; ExprContext *econtext = mtstate->ps.ps_ExprContext; + int action_idx = 1; if (part_attmap == NULL) part_attmap = @@ -897,6 +901,7 @@ ExecInitPartitionInfo(ModifyTableState * /* Generate the action's state for this relation */ action_state = makeNode(MergeActionState); action_state->mas_action = action; + action_state->mas_action_idx = action_idx++; /* And put the action in the appropriate list */ if (action->matched) diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c new file mode 100644 index 50e06ec..7e4717a --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -1665,8 +1665,8 @@ check_sql_fn_retval(List *queryTreeLists /* * If it's a plain SELECT, it returns whatever the targetlist says. - * Otherwise, if it's INSERT/UPDATE/DELETE with RETURNING, it returns - * that. Otherwise, the function return type must be VOID. + * Otherwise, if it's INSERT/UPDATE/DELETE/MERGE with RETURNING, it + * returns that. Otherwise, the function return type must be VOID. * * Note: eventually replace this test with QueryReturnsTuples? We'd need * a more general method of determining the output type, though. Also, it @@ -1684,7 +1684,8 @@ check_sql_fn_retval(List *queryTreeLists else if (parse && (parse->commandType == CMD_INSERT || parse->commandType == CMD_UPDATE || - parse->commandType == CMD_DELETE) && + parse->commandType == CMD_DELETE || + parse->commandType == CMD_MERGE) && parse->returningList) { tlist = parse->returningList; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c new file mode 100644 index f419c47..87ae37f --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -36,8 +36,7 @@ * RETURNING tuple after completing each row insert, update, or delete. * It must be called again to continue the operation. Without RETURNING, * we just loop within the node until all the work is done, then - * return NULL. This avoids useless call/return overhead. (MERGE does - * not support RETURNING.) + * return NULL. This avoids useless call/return overhead. */ #include "postgres.h" @@ -97,9 +96,6 @@ typedef struct ModifyTableContext TupleTableSlot *oldSlot, MergeActionState *relaction); - /* MERGE specific */ - MergeActionState *relaction; /* MERGE action in progress */ - /* * Information about the changes that were made concurrently to a tuple * being updated or deleted @@ -172,13 +168,14 @@ static TupleTableSlot *ExecMerge(ModifyT ItemPointer tupleid, bool canSetTag); static void ExecInitMerge(ModifyTableState *mtstate, EState *estate); -static bool ExecMergeMatched(ModifyTableContext *context, - ResultRelInfo *resultRelInfo, - ItemPointer tupleid, - bool canSetTag); -static void ExecMergeNotMatched(ModifyTableContext *context, - ResultRelInfo *resultRelInfo, - bool canSetTag); +static TupleTableSlot *ExecMergeMatched(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + ItemPointer tupleid, + bool canSetTag, + bool *matched); +static TupleTableSlot *ExecMergeNotMatched(ModifyTableContext *context, + ResultRelInfo *resultRelInfo, + bool canSetTag); static TupleTableSlot *mergeGetUpdateNewTuple(ResultRelInfo *relinfo, TupleTableSlot *planSlot, TupleTableSlot *oldSlot, @@ -987,7 +984,7 @@ ExecInsert(ModifyTableContext *context, if (mtstate->operation == CMD_UPDATE) wco_kind = WCO_RLS_UPDATE_CHECK; else if (mtstate->operation == CMD_MERGE) - wco_kind = (context->relaction->mas_action->commandType == CMD_UPDATE) ? + wco_kind = (mtstate->mt_merge_action->mas_action->commandType == CMD_UPDATE) ? WCO_RLS_UPDATE_CHECK : WCO_RLS_INSERT_CHECK; else wco_kind = WCO_RLS_INSERT_CHECK; @@ -1838,7 +1835,7 @@ ExecCrossPartitionUpdate(ModifyTableCont /* and project the new tuple to retry the UPDATE with */ context->cpUpdateRetrySlot = context->GetUpdateNewTuple(resultRelInfo, epqslot, oldSlot, - context->relaction); + mtstate->mt_merge_action); return false; } } @@ -2054,7 +2051,7 @@ lreplace: * No luck, a retry is needed. If running MERGE, we do not do so * here; instead let it handle that on its own rules. */ - if (context->relaction != NULL) + if (context->mtstate->mt_merge_action != NULL) return TM_Updated; /* @@ -2692,6 +2689,7 @@ static TupleTableSlot * ExecMerge(ModifyTableContext *context, ResultRelInfo *resultRelInfo, ItemPointer tupleid, bool canSetTag) { + TupleTableSlot *rslot = NULL; bool matched; /*----- @@ -2739,7 +2737,8 @@ ExecMerge(ModifyTableContext *context, R */ matched = tupleid != NULL; if (matched) - matched = ExecMergeMatched(context, resultRelInfo, tupleid, canSetTag); + rslot = ExecMergeMatched(context, resultRelInfo, tupleid, canSetTag, + &matched); /* * Either we were dealing with a NOT MATCHED tuple or ExecMergeMatched() @@ -2747,10 +2746,9 @@ ExecMerge(ModifyTableContext *context, R * matches. */ if (!matched) - ExecMergeNotMatched(context, resultRelInfo, canSetTag); + rslot = ExecMergeNotMatched(context, resultRelInfo, canSetTag); - /* No RETURNING support yet */ - return NULL; + return rslot; } /* @@ -2760,8 +2758,8 @@ ExecMerge(ModifyTableContext *context, R * We start from the first WHEN MATCHED action and check if the WHEN quals * pass, if any. If the WHEN quals for the first action do not pass, we * check the second, then the third and so on. If we reach to the end, no - * action is taken and we return true, indicating that no further action is - * required for this tuple. + * action is taken and "matched" is set to true, indicating that no further + * action is required for this tuple. * * If we do find a qualifying action, then we attempt to execute the action. * @@ -2770,15 +2768,16 @@ ExecMerge(ModifyTableContext *context, R * with individual actions are evaluated by this routine via ExecQual, while * EvalPlanQual checks for the join quals. If EvalPlanQual tells us that the * updated tuple still passes the join quals, then we restart from the first - * action to look for a qualifying action. Otherwise, we return false -- - * meaning that a NOT MATCHED action must now be executed for the current - * source tuple. + * action to look for a qualifying action. Otherwise, "matched" is set to + * false -- meaning that a NOT MATCHED action must now be executed for the + * current source tuple. */ -static bool +static TupleTableSlot * ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo, - ItemPointer tupleid, bool canSetTag) + ItemPointer tupleid, bool canSetTag, bool *matched) { ModifyTableState *mtstate = context->mtstate; + TupleTableSlot *rslot = NULL; TupleTableSlot *newslot; EState *estate = context->estate; ExprContext *econtext = mtstate->ps.ps_ExprContext; @@ -2790,7 +2789,10 @@ ExecMergeMatched(ModifyTableContext *con * If there are no WHEN MATCHED actions, we are done. */ if (resultRelInfo->ri_matchedMergeAction == NIL) - return true; + { + *matched = true; + return NULL; + } /* * Make tuple and any needed join variables available to ExecQual and @@ -2869,7 +2871,7 @@ lmerge_matched: */ newslot = ExecProject(relaction->mas_proj); - context->relaction = relaction; + mtstate->mt_merge_action = relaction; context->GetUpdateNewTuple = mergeGetUpdateNewTuple; context->cpUpdateRetrySlot = NULL; @@ -2892,7 +2894,7 @@ lmerge_matched: break; case CMD_DELETE: - context->relaction = relaction; + mtstate->mt_merge_action = relaction; if (!ExecDeletePrologue(context, resultRelInfo, tupleid, NULL, NULL)) { @@ -2952,7 +2954,8 @@ lmerge_matched: * If the tuple was already deleted, return to let caller * handle it under NOT MATCHED clauses. */ - return false; + *matched = false; + return NULL; case TM_Updated: { @@ -3020,13 +3023,19 @@ lmerge_matched: * NOT MATCHED actions. */ if (TupIsNull(epqslot)) - return false; + { + *matched = false; + return NULL; + } (void) ExecGetJunkAttribute(epqslot, resultRelInfo->ri_RowIdAttNo, &isNull); if (isNull) - return false; + { + *matched = false; + return NULL; + } /* * When a tuple was updated and migrated to @@ -3061,7 +3070,8 @@ lmerge_matched: * tuple already deleted; tell caller to run NOT * MATCHED actions */ - return false; + *matched = false; + return NULL; case TM_SelfModified: @@ -3081,13 +3091,14 @@ lmerge_matched: (errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION), errmsg("tuple to be updated or deleted was already modified by an operation triggered by the current command"), errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows."))); - return false; + *matched = false; + return NULL; default: /* see table_tuple_lock call in ExecDelete() */ elog(ERROR, "unexpected table_tuple_lock status: %u", result); - return false; + return NULL; } } @@ -3099,6 +3110,30 @@ lmerge_matched: break; } + /* Process RETURNING if present */ + if (resultRelInfo->ri_projectReturning) + { + switch (commandType) + { + case CMD_UPDATE: + rslot = ExecProcessReturning(resultRelInfo, newslot, + context->planSlot); + break; + + case CMD_DELETE: + rslot = ExecProcessReturning(resultRelInfo, + resultRelInfo->ri_oldTupleSlot, + context->planSlot); + break; + + case CMD_NOTHING: + break; + + default: + elog(ERROR, "unknown action in MERGE WHEN MATCHED clause"); + } + } + /* * We've activated one of the WHEN clauses, so we don't search * further. This is required behaviour, not an optimization. @@ -3109,19 +3144,22 @@ lmerge_matched: /* * Successfully executed an action or no qualifying action was found. */ - return true; + *matched = true; + + return rslot; } /* * Execute the first qualifying NOT MATCHED action. */ -static void +static TupleTableSlot * ExecMergeNotMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo, bool canSetTag) { ModifyTableState *mtstate = context->mtstate; ExprContext *econtext = mtstate->ps.ps_ExprContext; List *actionStates = NIL; + TupleTableSlot *rslot = NULL; ListCell *l; /* @@ -3171,10 +3209,10 @@ ExecMergeNotMatched(ModifyTableContext * * so we don't need to map the tuple here. */ newslot = ExecProject(action->mas_proj); - context->relaction = action; + mtstate->mt_merge_action = action; - (void) ExecInsert(context, mtstate->rootResultRelInfo, newslot, - canSetTag, NULL, NULL); + rslot = ExecInsert(context, mtstate->rootResultRelInfo, + newslot, canSetTag, NULL, NULL); mtstate->mt_merge_inserted += 1; break; case CMD_NOTHING: @@ -3190,6 +3228,8 @@ ExecMergeNotMatched(ModifyTableContext * */ break; } + + return rslot; } /* @@ -3227,6 +3267,7 @@ ExecInitMerge(ModifyTableState *mtstate, List *mergeActionList = lfirst(lc); TupleDesc relationDesc; ListCell *l; + int action_idx; resultRelInfo = mtstate->resultRelInfo + i; i++; @@ -3236,6 +3277,7 @@ ExecInitMerge(ModifyTableState *mtstate, if (unlikely(!resultRelInfo->ri_projectNewInfoValid)) ExecInitMergeTupleSlots(mtstate, resultRelInfo); + action_idx = 1; foreach(l, mergeActionList) { MergeAction *action = (MergeAction *) lfirst(l); @@ -3250,6 +3292,7 @@ ExecInitMerge(ModifyTableState *mtstate, */ action_state = makeNode(MergeActionState); action_state->mas_action = action; + action_state->mas_action_idx = action_idx++; action_state->mas_whenqual = ExecInitQual((List *) action->qual, &mtstate->ps); @@ -3386,6 +3429,64 @@ mergeGetUpdateNewTuple(ResultRelInfo *re } /* + * pg_merge_action() - + * SQL merge support function to retrieve the currently executing merge + * action command string ("INSERT", "UPDATE", or "DELETE"). + */ +Datum +pg_merge_action(PG_FUNCTION_ARGS) +{ + ModifyTableState *mtstate = (ModifyTableState *) fcinfo->context; + MergeActionState *relaction; + + if (!mtstate || mtstate->operation != CMD_MERGE) + elog(ERROR, "merge support function called in non-merge context"); + + relaction = mtstate->mt_merge_action; + if (relaction) + { + CmdType commandType = relaction->mas_action->commandType; + + switch (commandType) + { + case CMD_INSERT: + PG_RETURN_TEXT_P(cstring_to_text("INSERT")); + case CMD_UPDATE: + PG_RETURN_TEXT_P(cstring_to_text("UPDATE")); + case CMD_DELETE: + PG_RETURN_TEXT_P(cstring_to_text("DELETE")); + case CMD_NOTHING: + PG_RETURN_NULL(); + default: + elog(ERROR, "unrecognized commandType: %d", (int) commandType); + } + } + + PG_RETURN_NULL(); +} + +/* + * pg_merge_when_clause() - + * SQL merge support function to retrieve the 1-based index of the + * currently executing merge WHEN clause. + */ +Datum +pg_merge_when_clause(PG_FUNCTION_ARGS) +{ + ModifyTableState *mtstate = (ModifyTableState *) fcinfo->context; + MergeActionState *relaction; + + if (!mtstate || mtstate->operation != CMD_MERGE) + elog(ERROR, "merge support function called in non-merge context"); + + relaction = mtstate->mt_merge_action; + if (relaction) + PG_RETURN_INT32((int32) relaction->mas_action_idx); + + PG_RETURN_NULL(); +} + +/* * Process BEFORE EACH STATEMENT triggers */ static void @@ -3671,8 +3772,17 @@ ExecModifyTable(PlanState *pstate) { EvalPlanQualSetSlot(&node->mt_epqstate, context.planSlot); - ExecMerge(&context, node->resultRelInfo, NULL, node->canSetTag); - continue; /* no RETURNING support yet */ + slot = ExecMerge(&context, node->resultRelInfo, NULL, + node->canSetTag); + + /* + * If we got a RETURNING result, return it to the caller. + * We'll continue the work on next call. + */ + if (slot) + return slot; + + continue; /* continue with the next tuple */ } elog(ERROR, "tableoid is NULL"); @@ -3749,8 +3859,17 @@ ExecModifyTable(PlanState *pstate) { EvalPlanQualSetSlot(&node->mt_epqstate, context.planSlot); - ExecMerge(&context, node->resultRelInfo, NULL, node->canSetTag); - continue; /* no RETURNING support yet */ + slot = ExecMerge(&context, node->resultRelInfo, NULL, + node->canSetTag); + + /* + * If we got a RETURNING result, return it to the + * caller. We'll continue the work on next call. + */ + if (slot) + return slot; + + continue; /* continue with the next tuple */ } elog(ERROR, "ctid is NULL"); @@ -3843,7 +3962,7 @@ ExecModifyTable(PlanState *pstate) slot = internalGetUpdateNewTuple(resultRelInfo, context.planSlot, oldSlot, NULL); context.GetUpdateNewTuple = internalGetUpdateNewTuple; - context.relaction = NULL; + node->mt_merge_action = NULL; /* Now apply the update. */ slot = ExecUpdate(&context, resultRelInfo, tupleid, oldtuple, diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c new file mode 100644 index 4a817b7..c13c866 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -73,7 +73,6 @@ static void determineRecursiveColTypes(P Node *larg, List *nrtargetlist); static Query *transformReturnStmt(ParseState *pstate, ReturnStmt *stmt); static Query *transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt); -static List *transformReturningList(ParseState *pstate, List *returningList); static Query *transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt); static Query *transformDeclareCursorStmt(ParseState *pstate, @@ -514,7 +513,8 @@ transformDeleteStmt(ParseState *pstate, qual = transformWhereClause(pstate, stmt->whereClause, EXPR_KIND_WHERE, "WHERE"); - qry->returningList = transformReturningList(pstate, stmt->returningList); + qry->returningList = transformReturningList(pstate, stmt->returningList, + EXPR_KIND_RETURNING); /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; @@ -940,7 +940,8 @@ transformInsertStmt(ParseState *pstate, /* Process RETURNING, if any. */ if (stmt->returningList) qry->returningList = transformReturningList(pstate, - stmt->returningList); + stmt->returningList, + EXPR_KIND_RETURNING); /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; @@ -2405,7 +2406,8 @@ transformUpdateStmt(ParseState *pstate, qual = transformWhereClause(pstate, stmt->whereClause, EXPR_KIND_WHERE, "WHERE"); - qry->returningList = transformReturningList(pstate, stmt->returningList); + qry->returningList = transformReturningList(pstate, stmt->returningList, + EXPR_KIND_RETURNING); /* * Now we are done with SELECT-like processing, and can get on with @@ -2499,10 +2501,11 @@ transformUpdateTargetList(ParseState *ps /* * transformReturningList - - * handle a RETURNING clause in INSERT/UPDATE/DELETE + * handle a RETURNING clause in INSERT/UPDATE/DELETE/MERGE */ -static List * -transformReturningList(ParseState *pstate, List *returningList) +List * +transformReturningList(ParseState *pstate, List *returningList, + ParseExprKind exprKind) { List *rlist; int save_next_resno; @@ -2519,7 +2522,7 @@ transformReturningList(ParseState *pstat pstate->p_next_resno = 1; /* transform RETURNING identically to a SELECT targetlist */ - rlist = transformTargetList(pstate, returningList, EXPR_KIND_RETURNING); + rlist = transformTargetList(pstate, returningList, exprKind); /* * Complain if the nonempty tlist expanded to nothing (which is possible diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y new file mode 100644 index a013838..4406430 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -12241,6 +12241,7 @@ MergeStmt: USING table_ref ON a_expr merge_when_list + returning_clause { MergeStmt *m = makeNode(MergeStmt); @@ -12249,6 +12250,7 @@ MergeStmt: m->sourceRelation = $6; m->joinCondition = $8; m->mergeWhenClauses = $9; + m->returningList = $10; $$ = (Node *) m; } diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c new file mode 100644 index f7a1046..c66b7cf --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -455,6 +455,7 @@ check_agglevels_and_constraints(ParseSta errkind = true; break; case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: errkind = true; break; case EXPR_KIND_VALUES: @@ -903,6 +904,7 @@ transformWindowFuncCall(ParseState *psta errkind = true; break; case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: errkind = true; break; case EXPR_KIND_VALUES: diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c new file mode 100644 index c5b1a49..ad3c525 --- a/src/backend/parser/parse_cte.c +++ b/src/backend/parser/parse_cte.c @@ -126,13 +126,6 @@ transformWithClause(ParseState *pstate, CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc); ListCell *rest; - /* MERGE is allowed by parser, but unimplemented. Reject for now */ - if (IsA(cte->ctequery, MergeStmt)) - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("MERGE not supported in WITH query"), - parser_errposition(pstate, cte->location)); - for_each_cell(rest, withClause->ctes, lnext(withClause->ctes, lc)) { CommonTableExpr *cte2 = (CommonTableExpr *) lfirst(rest); @@ -153,7 +146,8 @@ transformWithClause(ParseState *pstate, /* must be a data-modifying statement */ Assert(IsA(cte->ctequery, InsertStmt) || IsA(cte->ctequery, UpdateStmt) || - IsA(cte->ctequery, DeleteStmt)); + IsA(cte->ctequery, DeleteStmt) || + IsA(cte->ctequery, MergeStmt)); pstate->p_hasModifyingCTE = true; } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c new file mode 100644 index 53e904c..2416cd6 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -482,6 +482,7 @@ transformColumnRef(ParseState *pstate, C case EXPR_KIND_LIMIT: case EXPR_KIND_OFFSET: case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: case EXPR_KIND_CHECK_CONSTRAINT: @@ -1708,6 +1709,7 @@ transformSubLink(ParseState *pstate, Sub case EXPR_KIND_LIMIT: case EXPR_KIND_OFFSET: case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: case EXPR_KIND_CYCLE_MARK: @@ -2997,6 +2999,7 @@ ParseExprKindName(ParseExprKind exprKind case EXPR_KIND_OFFSET: return "OFFSET"; case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: return "RETURNING"; case EXPR_KIND_VALUES: case EXPR_KIND_VALUES_SINGLE: diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c new file mode 100644 index ca14f06..914624e --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -31,6 +31,7 @@ #include "parser/parse_target.h" #include "parser/parse_type.h" #include "utils/builtins.h" +#include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -348,6 +349,15 @@ ParseFuncOrColumn(ParseState *pstate, Li parser_errposition(pstate, location))); } + /* Merge support functions are only allowed in MERGE's RETURNING list */ + if (IsMergeSupportFunction(funcid) && + pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("merge support function %s can only be called from the RETURNING list of a MERGE command", + NameListToString(funcname)), + parser_errposition(pstate, location))); + /* * So far so good, so do some fdresult-type-specific processing. */ @@ -2602,6 +2612,7 @@ check_srf_call_placement(ParseState *pst errkind = true; break; case EXPR_KIND_RETURNING: + case EXPR_KIND_MERGE_RETURNING: errkind = true; break; case EXPR_KIND_VALUES: diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c new file mode 100644 index d886637..40afa7c --- a/src/backend/parser/parse_merge.c +++ b/src/backend/parser/parse_merge.c @@ -233,6 +233,10 @@ transformMergeStmt(ParseState *pstate, M */ qry->jointree = makeFromExpr(pstate->p_joinlist, joinExpr); + /* Transform the RETURNING list, if any */ + qry->returningList = transformReturningList(pstate, stmt->returningList, + EXPR_KIND_MERGE_RETURNING); + /* * We now have a good query shape, so now look at the WHEN conditions and * action targetlists. @@ -390,9 +394,6 @@ transformMergeStmt(ParseState *pstate, M qry->mergeActionList = mergeActionList; - /* RETURNING could potentially be added in the future, but not in SQL std */ - qry->returningList = NULL; - qry->hasTargetSRFs = false; qry->hasSubLinks = pstate->p_hasSubLinks; diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c new file mode 100644 index b490541..9fe7bcb --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -2313,9 +2313,10 @@ addRangeTableEntryForCTE(ParseState *pst cte->cterefcount++; /* - * We throw error if the CTE is INSERT/UPDATE/DELETE without RETURNING. - * This won't get checked in case of a self-reference, but that's OK - * because data-modifying CTEs aren't allowed to be recursive anyhow. + * We throw error if the CTE is INSERT/UPDATE/DELETE/MERGE without + * RETURNING. This won't get checked in case of a self-reference, but + * that's OK because data-modifying CTEs aren't allowed to be recursive + * anyhow. */ if (IsA(cte->ctequery, Query)) { diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c new file mode 100644 index c74bac2..939d61e --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -3626,7 +3626,8 @@ RewriteQuery(Query *parsetree, List *rew if (!(ctequery->commandType == CMD_SELECT || ctequery->commandType == CMD_UPDATE || ctequery->commandType == CMD_INSERT || - ctequery->commandType == CMD_DELETE)) + ctequery->commandType == CMD_DELETE || + ctequery->commandType == CMD_MERGE)) { /* * Currently it could only be NOTIFY; this error message will diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c new file mode 100644 index c7d9d96..d393f51 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -2131,11 +2131,10 @@ QueryReturnsTuples(Query *parsetree) case CMD_SELECT: /* returns tuples */ return true; - case CMD_MERGE: - return false; case CMD_INSERT: case CMD_UPDATE: case CMD_DELETE: + case CMD_MERGE: /* the forms with RETURNING return tuples */ if (parsetree->returningList) return true; diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c new file mode 100644 index f907f5d..85eb868 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -962,13 +962,17 @@ PrintQueryResult(PGresult *result, bool else success = true; - /* if it's INSERT/UPDATE/DELETE RETURNING, also print status */ + /* + * If it's INSERT/UPDATE/DELETE/MERGE RETURNING, also print + * status. + */ if (last || pset.show_all_results) { cmdstatus = PQcmdStatus(result); if (strncmp(cmdstatus, "INSERT", 6) == 0 || strncmp(cmdstatus, "UPDATE", 6) == 0 || - strncmp(cmdstatus, "DELETE", 6) == 0) + strncmp(cmdstatus, "DELETE", 6) == 0 || + strncmp(cmdstatus, "MERGE", 5) == 0) PrintQueryStatus(result, printStatusFout); } diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat new file mode 100644 index 86eb8e8..2c1c3ee --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -11891,4 +11891,14 @@ prorettype => 'bytea', proargtypes => 'pg_brin_minmax_multi_summary', prosrc => 'brin_minmax_multi_summary_send' }, +# MERGE support functions +{ oid => '9499', descr => 'command type of current MERGE action', + proname => 'pg_merge_action', provolatile => 'v', + prorettype => 'text', proargtypes => '', + prosrc => 'pg_merge_action' }, +{ oid => '9500', descr => 'index of current MERGE WHEN clause', + proname => 'pg_merge_when_clause', provolatile => 'v', + prorettype => 'int4', proargtypes => '', + prosrc => 'pg_merge_when_clause' }, + ] diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h new file mode 100644 index e7abe0b..e597826 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -182,6 +182,11 @@ DECLARE_UNIQUE_INDEX(pg_proc_proname_arg #define PROARGMODE_VARIADIC 'v' #define PROARGMODE_TABLE 't' +/* Is this a merge support function? (Requires fmgroids.h) */ +#define IsMergeSupportFunction(oid) \ + ((oid) == F_PG_MERGE_ACTION || \ + (oid) == F_PG_MERGE_WHEN_CLAUSE) + #endif /* EXPOSE_TO_CLIENT_CODE */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h new file mode 100644 index 20f4c8b..5a0126d --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -415,6 +415,7 @@ typedef struct MergeActionState NodeTag type; MergeAction *mas_action; /* associated MergeAction node */ + int mas_action_idx; /* 1-based index of MergeAction node */ ProjectionInfo *mas_proj; /* projection of the action's targetlist for * this rel */ ExprState *mas_whenqual; /* WHEN [NOT] MATCHED AND conditions */ @@ -1302,6 +1303,9 @@ typedef struct ModifyTableState /* Flags showing which subcommands are present INS/UPD/DEL/DO NOTHING */ int mt_merge_subcommands; + /* For MERGE, the action currently being executed */ + MergeActionState *mt_merge_action; + /* tuple counters for MERGE */ double mt_merge_inserted; double mt_merge_updated; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h new file mode 100644 index 89335d9..b001d45 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1775,6 +1775,7 @@ typedef struct MergeStmt Node *sourceRelation; /* source relation */ Node *joinCondition; /* join condition between source and target */ List *mergeWhenClauses; /* list of MergeWhenClause(es) */ + List *returningList; /* list of expressions to return */ WithClause *withClause; /* WITH clause */ } MergeStmt; diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h new file mode 100644 index 1cef183..78eb55e --- a/src/include/parser/analyze.h +++ b/src/include/parser/analyze.h @@ -44,6 +44,8 @@ extern List *transformInsertRow(ParseSta bool strip_indirection); extern List *transformUpdateTargetList(ParseState *pstate, List *origTlist); +extern List *transformReturningList(ParseState *pstate, List *returningList, + ParseExprKind exprKind); extern Query *transformTopLevelStmt(ParseState *pstate, RawStmt *parseTree); extern Query *transformStmt(ParseState *pstate, Node *parseTree); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h new file mode 100644 index 1a37922..810a707 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -61,7 +61,8 @@ typedef enum ParseExprKind EXPR_KIND_DISTINCT_ON, /* DISTINCT ON */ EXPR_KIND_LIMIT, /* LIMIT */ EXPR_KIND_OFFSET, /* OFFSET */ - EXPR_KIND_RETURNING, /* RETURNING */ + EXPR_KIND_RETURNING, /* RETURNING in INSERT/UPDATE/DELETE */ + EXPR_KIND_MERGE_RETURNING, /* RETURNING in MERGE */ EXPR_KIND_VALUES, /* VALUES */ EXPR_KIND_VALUES_SINGLE, /* single-row VALUES (in INSERT only) */ EXPR_KIND_CHECK_CONSTRAINT, /* CHECK constraint for a table */ diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out new file mode 100644 index bc53b21..be7128e --- a/src/test/regress/expected/merge.out +++ b/src/test/regress/expected/merge.out @@ -123,20 +123,20 @@ ON tid = tid WHEN MATCHED THEN DO NOTHING; ERROR: name "target" specified more than once DETAIL: The name is used both as MERGE target table and data source. --- used in a CTE +-- used in a CTE without RETURNING WITH foo AS ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) SELECT * FROM foo; -ERROR: MERGE not supported in WITH query -LINE 1: WITH foo AS ( - ^ --- used in COPY +ERROR: WITH query "foo" does not have a RETURNING clause +LINE 4: ) SELECT * FROM foo; + ^ +-- used in COPY without RETURNING COPY ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) TO stdout; -ERROR: MERGE not supported in COPY +ERROR: COPY query must have a RETURNING clause -- unsupported relation types -- view CREATE VIEW tv AS SELECT * FROM target; @@ -1289,21 +1289,40 @@ WHEN MATCHED AND tid < 2 THEN ROLLBACK; -- RETURNING BEGIN; -INSERT INTO sq_source (sid, balance, delta) VALUES (-1, -1, -10); MERGE INTO sq_target t -USING v +USING sq_source s ON tid = sid -WHEN MATCHED AND tid > 2 THEN +WHEN MATCHED AND tid >= 2 THEN UPDATE SET balance = t.balance + delta WHEN NOT MATCHED THEN INSERT (balance, tid) VALUES (balance + delta, sid) WHEN MATCHED AND tid < 2 THEN DELETE -RETURNING *; -ERROR: syntax error at or near "RETURNING" -LINE 10: RETURNING *; - ^ +RETURNING pg_merge_when_clause() AS when_clause, + pg_merge_action() AS merge_action, + t.*, + CASE pg_merge_action() + WHEN 'INSERT' THEN 'Inserted '||t + WHEN 'UPDATE' THEN 'Added '||delta||' to balance' + WHEN 'DELETE' THEN 'Removed '||t + END AS description; + when_clause | merge_action | tid | balance | description +-------------+--------------+-----+---------+--------------------- + 3 | DELETE | 1 | 100 | Removed (1,100) + 1 | UPDATE | 2 | 220 | Added 20 to balance + 2 | INSERT | 4 | 40 | Inserted (4,40) +(3 rows) + ROLLBACK; +-- error when using MERGE support functions outside MERGE +SELECT pg_merge_action() FROM sq_target; +ERROR: merge support function pg_merge_action can only be called from the RETURNING list of a MERGE command +LINE 1: SELECT pg_merge_action() FROM sq_target; + ^ +UPDATE sq_target SET balance = balance + 1 RETURNING pg_merge_when_clause(); +ERROR: merge support function pg_merge_when_clause can only be called from the RETURNING list of a MERGE command +LINE 1: ...ATE sq_target SET balance = balance + 1 RETURNING pg_merge_w... + ^ -- EXPLAIN CREATE TABLE ex_mtarget (a int, b int) WITH (autovacuum_enabled=off); diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql new file mode 100644 index fdbcd70..e434a2b --- a/src/test/regress/sql/merge.sql +++ b/src/test/regress/sql/merge.sql @@ -86,12 +86,12 @@ MERGE INTO target USING target ON tid = tid WHEN MATCHED THEN DO NOTHING; --- used in a CTE +-- used in a CTE without RETURNING WITH foo AS ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE ) SELECT * FROM foo; --- used in COPY +-- used in COPY without RETURNING COPY ( MERGE INTO target USING source ON (true) WHEN MATCHED THEN DELETE @@ -844,19 +844,29 @@ ROLLBACK; -- RETURNING BEGIN; -INSERT INTO sq_source (sid, balance, delta) VALUES (-1, -1, -10); MERGE INTO sq_target t -USING v +USING sq_source s ON tid = sid -WHEN MATCHED AND tid > 2 THEN +WHEN MATCHED AND tid >= 2 THEN UPDATE SET balance = t.balance + delta WHEN NOT MATCHED THEN INSERT (balance, tid) VALUES (balance + delta, sid) WHEN MATCHED AND tid < 2 THEN DELETE -RETURNING *; +RETURNING pg_merge_when_clause() AS when_clause, + pg_merge_action() AS merge_action, + t.*, + CASE pg_merge_action() + WHEN 'INSERT' THEN 'Inserted '||t + WHEN 'UPDATE' THEN 'Added '||delta||' to balance' + WHEN 'DELETE' THEN 'Removed '||t + END AS description; ROLLBACK; +-- error when using MERGE support functions outside MERGE +SELECT pg_merge_action() FROM sq_target; +UPDATE sq_target SET balance = balance + 1 RETURNING pg_merge_when_clause(); + -- EXPLAIN CREATE TABLE ex_mtarget (a int, b int) WITH (autovacuum_enabled=off); ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: MERGE ... RETURNING @ 2023-01-22 18:58 Alvaro Herrera <[email protected]> parent: Dean Rasheed <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Alvaro Herrera @ 2023-01-22 18:58 UTC (permalink / raw) To: Dean Rasheed <[email protected]>; +Cc: Vik Fearing <[email protected]>; Isaac Morland <[email protected]>; PostgreSQL Hackers <[email protected]> > diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c > new file mode 100644 > index e34f583..aa3cca0 > --- a/src/backend/commands/copy.c > +++ b/src/backend/commands/copy.c > @@ -274,12 +274,6 @@ DoCopy(ParseState *pstate, const CopyStm > { > Assert(stmt->query); > > - /* MERGE is allowed by parser, but unimplemented. Reject for now */ > - if (IsA(stmt->query, MergeStmt)) > - ereport(ERROR, > - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > - errmsg("MERGE not supported in COPY")); Does this COPY stuff come from another branch where you're adding support for MERGE in COPY? I see that you add a test that MERGE without RETURNING fails, but you didn't add any tests that it works with RETURNING. Anyway, I suspect these small changes shouldn't be here. Overall, the idea of using Postgres-specific functions for extracting context in the RETURNING clause looks acceptable to me. We can change that to add support to whatever clauses the SQL committee offers, when they get around to offering something. (We do have to keep our fingers crossed that they will decide to use the same RETURNING syntax as we do in this patch, of course.) Regarding mas_action_idx, I would have thought that it belongs in MergeAction rather than MergeActionState. After all, you determine it once at parse time, and it is a constant from there onwards, right? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: MERGE ... RETURNING @ 2023-01-23 16:54 Dean Rasheed <[email protected]> parent: Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Dean Rasheed @ 2023-01-23 16:54 UTC (permalink / raw) To: Alvaro Herrera <[email protected]>; +Cc: Vik Fearing <[email protected]>; Isaac Morland <[email protected]>; PostgreSQL Hackers <[email protected]> On Sun, 22 Jan 2023 at 19:08, Alvaro Herrera <[email protected]> wrote: > > > diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c > > new file mode 100644 > > index e34f583..aa3cca0 > > --- a/src/backend/commands/copy.c > > +++ b/src/backend/commands/copy.c > > @@ -274,12 +274,6 @@ DoCopy(ParseState *pstate, const CopyStm > > { > > Assert(stmt->query); > > > > - /* MERGE is allowed by parser, but unimplemented. Reject for now */ > > - if (IsA(stmt->query, MergeStmt)) > > - ereport(ERROR, > > - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > > - errmsg("MERGE not supported in COPY")); > > Does this COPY stuff come from another branch where you're adding > support for MERGE in COPY? I see that you add a test that MERGE without > RETURNING fails, but you didn't add any tests that it works with > RETURNING. Anyway, I suspect these small changes shouldn't be here. > A few of the code changes I made were entirely untested (this was after all just a proof-of-concept, intended to gather opinions and get consensus about the overall shape of the feature). They serve as useful reminders of things to test. In fact, since then, I've been doing more testing, and so far everything I have tried has just worked, including COPY (MERGE ... RETURNING ...) TO ... Thinking about it, I can't see any reason why it wouldn't. Still, there's a lot more testing to do. Just going through the docs looking for references to RETURNING gave me a lot more ideas of things to test. > Overall, the idea of using Postgres-specific functions for extracting > context in the RETURNING clause looks acceptable to me. Cool. > We can change > that to add support to whatever clauses the SQL committee offers, when > they get around to offering something. (We do have to keep our fingers > crossed that they will decide to use the same RETURNING syntax as we do > in this patch, of course.) > Yes indeed. At least, done this way, the only non-SQL-standard syntax is the RETURNING keyword itself, which we've already settled on for INSERT/UPDATE/DELETE. Let's just hope they don't decide to use RETURNING in an incompatible way in the future. > Regarding mas_action_idx, I would have thought that it belongs in > MergeAction rather than MergeActionState. After all, you determine it > once at parse time, and it is a constant from there onwards, right? > Oh, yes that makes sense (and removes the need for a couple of the executor changes). Thanks for looking. Regards, Dean ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-01-23 16:54 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 07:58 [PATCH v48 1/7] sequential scan for dshash Kyotaro Horiguchi <[email protected]> 2023-01-22 10:09 Re: MERGE ... RETURNING Dean Rasheed <[email protected]> 2023-01-22 18:58 ` Re: MERGE ... RETURNING Alvaro Herrera <[email protected]> 2023-01-23 16:54 ` Re: MERGE ... RETURNING Dean Rasheed <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox