On Fri, 15 Mar 2024 at 17:14, Jeff Davis wrote:
>
> On Fri, 2024-03-15 at 11:20 +, Dean Rasheed wrote:
>
> > So barring any further objections, I'd like to go ahead and get this
> > patch committed.
>
> I like this feature from a user perspective. So +1 from me.
>
I have committed this.
On Fri, 2024-03-15 at 11:20 +, Dean Rasheed wrote:
> To recap, this adds support for a single RETURNING list at the end of
> a MERGE command, and a special MERGE_ACTION() function that may be
> used in the RETURNING list to return the action command string
> ('INSERT', 'UPDATE', or 'DELETE')
On Fri, 15 Mar 2024 at 11:06, Dean Rasheed wrote:
>
> Updated patch attached.
>
I have gone over this patch again in detail, and I believe that the
code is in good shape. All review comments have been addressed, and
the only thing remaining is the syntax question.
To recap, this adds support
On 2024-Mar-13, Dean Rasheed wrote:
> On Wed, 13 Mar 2024 at 06:44, jian he wrote:
> >
> >
> > [ WITH with_query [, ...] ]
> > MERGE INTO [ ONLY ] >
> > here the "WITH" part should have "[ RECURSIVE ]"
>
> Actually, no. MERGE doesn't support WITH RECURSIVE.
>
> It's not entirely clear to me
RETURNING
UPDATE
RETURNING
DELETE
RETURNING
MERGE
RETURNING
in doc/src/sgml/dml.sgml, what is the point of these?
It is not rendered in the html file, deleting it still generates all the
html file
n
diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml
new file mode 100644
index cbbc5e2..3d95bdb
--- a/doc/src/sgml/dml.sgml
+++ b/doc/src/sgml/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obta
On Wed, 13 Mar 2024 at 06:44, jian he wrote:
>
>
> [ WITH with_query [, ...] ]
> MERGE INTO [ ONLY ]
> here the "WITH" part should have "[ RECURSIVE ]"
Actually, no. MERGE doesn't support WITH RECURSIVE.
It's not entirely clear to me why though. I did a quick test, removing
that restriction
Hi, some minor issues:
[ WITH with_query [, ...] ]
MERGE INTO [ ONLY ] target_table_name [ * ] [ [ AS ]
target_alias ]
USING data_source ON
join_condition
when_clause [...]
[ RETURNING * | output_expression [ [ AS ]
output_name ] [, ...] ]
here the "WITH" part should have "[ RECURSIVE ]" like:
Jeff Davis:
To summarize, most of the problem has been in retrieving the action
(INSERT/UPDATE/DELETE) taken or the WHEN-clause number applied to a
particular matched row. The reason this is important is because the row
returned is the old row for a DELETE action, and the new row for an
INSERT
On Thu, Feb 29, 2024 at 1:49 PM Jeff Davis wrote:
>
> Can we get some input on whether the current MERGE ... RETURNING patch
> is the right approach from a language standpoint?
>
MERGE_CLAUSE_NUMBER() seems really out of place to me, it feels out of
place to identify output set by n
On Wed, 6 Mar 2024 at 08:51, Peter Eisentraut wrote:
>
> For comparison with standard SQL (see ):
>
> For an INSERT you could write
>
> SELECT whatever FROM NEW TABLE (INSERT statement here)
>
> or for an DELETE
>
> SELECT whatever FROM OLD TABLE (DELETE statement here)
>
> And for an UPDATE
pic=statement-result-sets-from-sql-data-changes>)
For a MERGE statement, whether you can specify OLD or NEW (or FINAL)
depends on what actions appear in the MERGE statement.
So if we were to translate that to our syntax, it might be something like
MERGE ... RETURNING OLD *
or
On Thu, 2024-02-29 at 19:24 +, Dean Rasheed wrote:
> Attached is a rebased version on top of 5f2e179bd3 (support for MERGE
> into views), with a few additional tests to confirm that MERGE ...
> RETURNING works for views as well as tables.
Thank you for the rebase. I just missed you
Can we get some input on whether the current MERGE ... RETURNING patch
is the right approach from a language standpoint?
We've gone through a lot of iterations -- thank you Dean, for
implementing so many variations.
To summarize, most of the problem has been in retrieving the action
(INSERT
I didn't find any issue with v15.
no commit message in the patch, If a commit message is there, I can
help proofread.
On Fri, Jan 19, 2024 at 1:44 AM Dean Rasheed wrote:
>
>
> Thanks for reviewing. Updated patch attached.
>
> The wider question is whether people are happy with the overall
> approach this patch now takes, and the new MERGING() function and
> MergingFunc node.
>
one minor white space issue:
git
On Sat, Nov 18, 2023 at 8:55 PM Dean Rasheed wrote:
>
> The v13 patch still applies on top of this, so I won't re-post it.
>
Hi.
minor issues based on v13.
+
+MERGING (
property )
+
+ The following are valid property values specifying what to return:
+
+
+
+ ACTION
+
+
+
On Fri, 17 Nov 2023 at 04:30, jian he wrote:
>
> I think it should be:
> + You will require the SELECT privilege on any column(s)
> + of the data_source and
> + target_table_name referred to
> + in any condition or expression.
>
Ah, of course. As always, I'm blind to grammatical errors
>
> Attached is a separate patch with those doc updates, intended to be
> applied and back-patched independently of the main RETURNING patch.
>
> Regards,
> Dean
+ You will require the SELECT privilege and any column(s)
+ of the data_source and
+ target_table_name referred to
+ in any
On Mon, 13 Nov 2023 at 05:29, jian he wrote:
>
> v13 works fine. all tests passed. The code is very intuitive. played
> with multi WHEN clauses, even with before/after row triggers, work as
> expected.
>
Thanks for the review and testing!
> I don't know when replace_outer_merging will be
Hi.
v13 works fine. all tests passed. The code is very intuitive. played
with multi WHEN clauses, even with before/after row triggers, work as
expected.
I don't know when replace_outer_merging will be invoked. even set a
breakpoint on it. coverage shows replace_outer_merging only called
once.
cts;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and MERGE commands all have an
optional RETURNING clause that s
l
+++ b/doc/src/sgml/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and
On Wed, 2023-11-01 at 10:12 +, Dean Rasheed wrote:
> Something I'm wondering about is to what extent this discussion is
> driven by concerns about aspects of the implementation (specifically,
> references to function OIDs in code), versus a desire for a different
> user-visible syntax. To a
;
> If pg_merge_action() and pg_merge_when_clause_number() were
> implemented using a MergeFunc node, it would reduce the number of
> places that refer to specific function OIDs. Basically, a MergeFunc
> node would be very much like a FuncExpr node, except that it would
> have a "
On 11/1/23 11:12, Dean Rasheed wrote:
On Tue, 31 Oct 2023 at 23:19, Vik Fearing wrote:
On 10/31/23 19:28, Jeff Davis wrote:
Assuming we have one RETURNING clause at the end, then it creates the
problem of how to communicate which WHEN clause a tuple came from,
whether it's the old or the
regular FuncExpr would suffice.
If pg_merge_action() and pg_merge_when_clause_number() were
implemented using a MergeFunc node, it would reduce the number of
places that refer to specific function OIDs. Basically, a MergeFunc
node would be very much like a FuncExpr node, except that it would
hav
On 10/31/23 19:28, Jeff Davis wrote:
On Tue, 2023-10-31 at 12:45 +0100, Vik Fearing wrote:
On 10/24/23 21:10, Jeff Davis wrote:
Can we revisit the idea of a per-WHEN RETURNING clause?
For the record, I dislike this idea.
I agree that it makes things awkward, and if it creates grammatical
On Tue, 2023-10-31 at 12:45 +0100, Vik Fearing wrote:
> On 10/24/23 21:10, Jeff Davis wrote:
> > Can we revisit the idea of a per-WHEN RETURNING clause?
>
> For the record, I dislike this idea.
I agree that it makes things awkward, and if it creates grammatical
problems as well, then it's not
On 10/24/23 21:10, Jeff Davis wrote:
Can we revisit the idea of a per-WHEN RETURNING clause?
For the record, I dislike this idea.
--
Vik Fearing
On Fri, 2023-10-27 at 15:46 +0100, Dean Rasheed wrote:
>
> One fiddly part is resolving the shift/reduce conflicts in the
> grammar. Specifically, on seeing "RETURNING expr when ...", there is
> ambiguity over whether the "when" is a column alias or the start of
> the next merge action. I've
commandType == CMD_MERGE) &&
parse->returningList)
{
tlist = parse->returningList;
@@ -1693,7 +1694,7 @@ check_sql_fn_retval(List *queryTreeLists
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("return type mismatch in function declared to return %
On Tue, Oct 24, 2023 at 2:11 PM Jeff Davis wrote:
> On Wed, 2023-08-23 at 11:58 +0100, Dean Rasheed wrote:
> > Updated version attached, fixing an uninitialized-variable warning
> > from the cfbot.
>
> I took another look and I'm still not comfortable with the special
> IsMergeSupportFunction()
On Wed, 2023-08-23 at 11:58 +0100, Dean Rasheed wrote:
> Updated version attached, fixing an uninitialized-variable warning
> from the cfbot.
I took another look and I'm still not comfortable with the special
IsMergeSupportFunction() functions. I don't object necessarily -- if
someone else wants
products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and MERGE commands all have an
optional RETURNING clause
command (see commit c2e08b04c9).
Regards,
Dean
diff --git a/doc/src/sgml/dml.sgml b/doc/src/sgml/dml.sgml
new file mode 100644
index cbbc5e2..7f65f6e
--- a/doc/src/sgml/dml.sgml
+++ b/doc/src/sgml/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETU
On Fri, Jul 21, 2023 at 7:17 PM Dean Rasheed wrote:
>
> On Mon, 17 Jul 2023 at 20:43, Jeff Davis wrote:
> >
> > > > Maybe instead of a function it could be a special table reference
> > > > like:
> > > >
> > > > ME
On Mon, 17 Jul 2023 at 20:43, Jeff Davis wrote:
>
> > > Maybe instead of a function it could be a special table reference
> > > like:
> > >
> > > MERGE ... RETURNING MERGE.action, MERGE.action_number, id, val?
> > >
> The benefits are:
>
On Thu, 2023-07-20 at 23:19 -0700, Gurjeet Singh wrote:
> Plus, if we were able to make it work as SQL syntax, it's very likely
> we can use the same technique to implement BEFORE and AFTER behaviour
> in UPDATE ... RETURNING that the old thread could not accomplish a
> decade ago.
To clarify, I
ncerns were mainly around use of OLD.* and NEW.*, too much
special-casing of RTE_ALIAS, and then the code quality of the patch
itself.
> > > Maybe instead of a function it could be a special table reference
> > > like:
> > >
> > > MERGE ... RETURNING MERGE.ac
t; you wouldn't need pg_merge_action() (if the column was NOT NULL), so
> > > I
> > > think the features should work well together.
> >
> > For use cases where a user could do it either way, which would you
> > expect to be the "typical" way (assuming w
how exactly the table alias should work
such that it doesn't look too much like a join. Not sure how much of a
problem that is.
> > Maybe instead of a function it could be a special table reference
> > like:
> >
> > MERGE ... RETURNING MERGE.action, MERGE.action_number,
think the features should work well together.
>
> For use cases where a user could do it either way, which would you
> expect to be the "typical" way (assuming we supported the new/old)?
>
> MERGE ... RETURNING pg_merge_action(), id, val;
>
> or
>
> MERGE .
do it either way, which would you
expect to be the "typical" way (assuming we supported the new/old)?
MERGE ... RETURNING pg_merge_action(), id, val;
or
MERGE ... RETURNING id, OLD.val, NEW.val;
?
I am still bothered that pg_merge_action() is so context-sensitive.
"SELECT p
On Thu, 13 Jul 2023 at 17:43, Vik Fearing wrote:
>
> There is also the WITH ORDINALITY and FOR ORDINALITY examples.
>
True. I just think "number" is a friendlier, more familiar word than "ordinal".
> So perhaps pg_merge_when_clause_number() would
> > be a better name. It's still quite long, but
On Thu, 13 Jul 2023 at 17:01, Jeff Davis wrote:
>
> MERGE can end up combining old and new values in a way that doesn't
> happen with INSERT/UPDATE/DELETE. For instance, a "MERGE ... RETURNING
> id" would return a mix of NEW.id (for INSERT/UPDATE actions) and OLD.id
On 7/13/23 17:38, Dean Rasheed wrote:
On Tue, 11 Jul 2023 at 21:43, Gurjeet Singh wrote:
I think the name of function pg_merge_when_clause() can be improved.
How about pg_merge_when_clause_ordinal().
That's a bit of a mouthful, but I don't have a better idea right now.
I've left the names
On Thu, Jul 13, 2023 at 8:38 AM Dean Rasheed wrote:
>
> On Tue, 11 Jul 2023 at 21:43, Gurjeet Singh wrote:
> > { oid => '9499', descr => 'command type of current MERGE action',
> > - proname => 'pg_merge_action', provolatile => 'v',
> > + proname => 'pg_merge_action', provolatile => 'v',
On Mon, 2023-01-09 at 12:29 +, Dean Rasheed wrote:
> > Would it be useful to have just the action? Perhaps "WITH ACTION"?
> > My idea is that this would return an enum of INSERT, UPDATE, DELETE
> > (so is "action" the right word?). It seems to me in many situations
> > I would be more likely
work, so there might be
> technical difficulties. But if it could be made to work for UPDATE,
> it
> shouldn't be much more effort to make it work for MERGE.
MERGE can end up combining old and new values in a way that doesn't
happen with INSERT/UPDATE/DELETE. For instance, a "MERGE ... RETURNI
On Tue, 11 Jul 2023 at 21:43, Gurjeet Singh wrote:
>
> > > I think the name of function pg_merge_when_clause() can be improved.
> > > How about pg_merge_when_clause_ordinal().
> >
> > That's a bit of a mouthful, but I don't have a better idea right now.
> > I've left the names alone for now, in
On Sun, 2023-01-08 at 12:28 +, Dean Rasheed wrote:
> I considered allowing a separate RETURNING list at the end of each
> action, but rapidly dismissed that idea.
One potential benefit of that approach is that it would be more natural
to specify output specific to the action, e.g.
WHEN
On 7/13/23 01:48, Jeff Davis wrote:
On Wed, 2023-07-12 at 03:47 +0200, Vik Fearing wrote:
There is no RETURNING clause in Standard SQL, and the way they would
do
this is:
SELECT ...
FROM OLD TABLE (
MERGE ...
) AS m
The rules for that for MERGE are well defined.
On Wed, 2023-07-12 at 03:47 +0200, Vik Fearing wrote:
> There is no RETURNING clause in Standard SQL, and the way they would
> do
> this is:
>
> SELECT ...
> FROM OLD TABLE (
> MERGE ...
> ) AS m
>
> The rules for that for MERGE are well defined.
I only see OLD TABLE
On 7/12/23 02:43, Jeff Davis wrote:
On Sun, 2023-01-22 at 19:58 +0100, Alvaro Herrera wrote:
(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.)
Do we have a reason to think that they will accept something
On Sun, 2023-01-22 at 19:58 +0100, Alvaro Herrera wrote:
>
> (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.)
Do we have a reason to think that they will accept something similar?
Regards,
Jeff Davis
On Tue, Jul 11, 2023 at 1:43 PM Gurjeet Singh wrote:
>
> In the above hunk, if there's an exception/ERROR, I believe we should
> PG_RE_THROW(). If there's a reason to continue, we should at least set
> rslot = NULL, otherwise we may be returning an uninitialized value to
> the caller.
Excuse the
On Fri, Jul 7, 2023 at 3:48 PM Dean Rasheed wrote:
>
> On Thu, 6 Jul 2023 at 06:13, Gurjeet Singh wrote:
> >
> > > One minor annoyance is that, due to the way that pg_merge_action() and
> > > pg_merge_when_clause() require access to the MergeActionState, they
> > > only work if they appear
ace that uses this macro, whereas there are a
lot of places that include pg_proc.h and not fmgroids.h, so I don't
think forcing them all to include fmgroids.h is a good idea. (BTW,
this approach and comment is borrowed from IsTrueArrayType() in
pg_type.h)
> --- a/src/test/regress/expected/merge
On Thu, Jul 6, 2023 at 4:07 AM jian he wrote:
>
> On Thu, Jul 6, 2023 at 1:13 PM Gurjeet Singh wrote:
> > I think the name of function pg_merge_when_clause() can be improved.
> > How about pg_merge_when_clause_ordinal().
>
> another idea: pg_merge_action_ordinal()
Since there can be many
On Thu, Jul 6, 2023 at 3:39 AM Alvaro Herrera wrote:
>
> On 2023-Jul-05, Gurjeet Singh wrote:
> > I expected the .out file to have captured the stdout. I'm gradually,
> > and gladly, re-learning bits of the test infrastructure.
> >
> > The DELETE command tag in the output does not feel
On Thu, Jul 6, 2023 at 1:13 PM Gurjeet Singh wrote:
>
> On Sat, Jul 1, 2023 at 4:08 AM Dean Rasheed wrote:
> >
> > On Mon, 13 Mar 2023 at 13:36, Dean Rasheed wrote:
> > >
> > > And another rebase.
> > >
> >
> > I ran out of cycles to pursue the MERGE patches in v16, but hopefully
> > I can make
On 2023-Jul-05, Gurjeet Singh wrote:
> +BEGIN;
> +COPY (
> +MERGE INTO sq_target t
> +USING v
> +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,
ll eliminate the need to keep the "Requires..."
note above, and avoid confusion, too.
--- a/src/test/regress/expected/merge.out
+++ b/src/test/regress/expected/merge.out
-WHEN MATCHED AND tid > 2 THEN
+WHEN MATCHED AND tid >= 2 THEN
This change can be treated as a bug fix :-)
+-- C
--- a/doc/src/sgml/dml.sgml
+++ b/doc/src/sgml/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE com
@@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and MERGE commands all have an
optional RETURNI
0 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and MERGE commands all have an
optional
/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT, UPDATE,
- and DELETE commands all have an
+ DELETE, and MERGE commands
00644
index cbbc5e2..ff2a827
--- a/doc/src/sgml/dml.sgml
+++ b/doc/src/sgml/dml.sgml
@@ -283,10 +283,15 @@ DELETE FROM products;
RETURNING
+
+ MERGE
+ RETURNING
+
+
Sometimes it is useful to obtain data from modified rows while they are
being manipulated. The INSERT
e 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
wo
> 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
> {
>
->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
On Mon, 9 Jan 2023 at 16:23, Vik Fearing 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'), MERG
a MERGING() function analogous to the GROUPING() function that goes
with grouping sets?
MERGE ...
RETURNING *, MERGING('clause'), MERGING('action');
Or something.
--
Vik Fearing
On Sun, 8 Jan 2023 at 20:09, Isaac Morland wrote:
>
> Would it be useful to have just the action? Perhaps "WITH ACTION"? My idea is
> that this would return an enum of INSERT, UPDATE, DELETE (so is "action" the
> right word?). It seems to me in many situations I would be more likely to
> care
On Sun, 8 Jan 2023 at 07:28, Dean Rasheed wrote:
So playing around with it (and inspired by the WITH ORDINALITY syntax
> for SRFs), I had the idea of allowing "WITH WHEN CLAUSE" at the end of
> the returning list, which adds an integer column to the list, whose
> value is set to the index of the
lity.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 RET
77 matches
Mail list logo
