Re: Adding OLD/NEW support to RETURNING

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Jeff Davis <pgsql(at)j-davis(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(at)vondra(dot)me>
Subject: Re: Adding OLD/NEW support to RETURNING
Date: 2025-01-01 08:19:41
Message-ID: CAEZATCWsN0JgjcAtrGLitj=JywODb40qPx7eHMAD4xKBxX_Odw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 28 Nov 2024 at 11:45, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> In the light of 7f798aca1d5df290aafad41180baea0ae311b4ee, I guess I
> should remove various (void *) casts that this patch adds, copied from
> old code.

Attached is an updated patch with some additional tidying up, plus the
following changes:

I decided to get rid of the fast-path ExecJust* functions, which I had
added by more-or-less copy-pasting existing code, without thinking too
hard. In practice, compared to the cost of a DML query, it seems
unlikely that these optimisations would be worth it, and AFAICS only
ExecJustAssign{Old|New}Var could ever be hit, and then only for a
RETURNING list containing a single Var, which seems unlikely to be
common. Also, these optimisations weren't free, because they were
adding a small number of additional cycles to
ExecReadyInterpretedExpr().

In the Query struct, the 2 new fields specifying the names of the old
and new aliases in the RETURNING clause were called "returningOld" and
"returningNew", but those names didn't seem quite right, so I've
changed them to "returningOldAlias" and "returningNewAlias", which
seems a little more consistent and descriptive of what they are. In
addition, I had overlooked the fact that they should have been marked
as query_jumble_ignore because, like other aliases in a Query, the
choice of alias names doesn't materially affect the query.

Looking again at ruleutils.c, a ReturningExpr node can only occur when
running EXPLAIN, not when deparsing a query, so I've added a comment
to make that clear. Thinking more about what the result should look
like in EXPLAIN, I think the best thing to do is to just output the
referenced expression, ignoring the nulling effects of the
ReturningExpr node, making the result simpler and more intuitive.

Regards,
Dean

Attachment Content-Type Size
v22-0001-Add-OLD-NEW-support-to-RETURNING-in-DML-queries.patch text/x-patch 243.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2025-01-01 08:52:29 Re: SQL Property Graph Queries (SQL/PGQ)
Previous Message Shlok Kyal 2025-01-01 06:47:24 Re: Logical replication timeout