Re: Virtual generated columns

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Virtual generated columns
Date: 2024-09-04 08:40:06
Message-ID: cd7c0fea-6c4b-4bdd-af3b-4d4dabb32ca1@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 21.08.24 12:51, Dean Rasheed wrote:
> On Wed, 21 Aug 2024 at 08:00, Peter Eisentraut<peter(at)eisentraut(dot)org> wrote:
>> On 08.08.24 20:22, Dean Rasheed wrote:
>>> Looking at the rewriter changes, it occurred to me that it could
>>> perhaps be done more simply using ReplaceVarsFromTargetList() for each
>>> RTE with virtual generated columns. That function already has the
>>> required wholerow handling code, so there'd be less code duplication.
>> Hmm, I don't quite see how ReplaceVarsFromTargetList() could be used
>> here. It does have the wholerow logic that we need somehow, but other
>> than that it seems to target something different?
>>
> Well what I was thinking was that (in fireRIRrules()'s final loop over
> relations in the rtable), if the relation had any virtual generated
> columns, you'd build a targetlist containing a TLE for each one,
> containing the generated expression. Then you could just call
> ReplaceVarsFromTargetList() to replace any Vars in the query with the
> corresponding generated expressions. That takes care of descending
> into subqueries, adjusting varlevelsup, and expanding wholerow Vars
> that might refer to the generated expression.
>
> I also have half an eye on how this patch will interact with my patch
> to support RETURNING OLD/NEW values. If you use
> ReplaceVarsFromTargetList(), it should just do the right thing for
> RETURNING OLD/NEW generated expressions.

Here is an implementation of this. It's much nicer! It also appears to
fix all the additional test cases that have been presented. (I haven't
integrated them into the patch set yet.)

I left the 0001 patch alone for now and put the new rewriting
implementation into 0002. (Unfortunately, the diff is kind of useless
for visual inspection.) Let me know if this matches what you had in
mind, please. Also, is this the right place in fireRIRrules()?

Attachment Content-Type Size
v6-0001-Virtual-generated-columns.patch text/plain 188.6 KB
v6-0002-Use-ReplaceVarsFromTargetList-for-rewriting-virtu.patch text/plain 13.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2024-09-04 08:47:11 Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes
Previous Message Aleksander Alekseev 2024-09-04 08:34:51 Re: Commit Timestamp and LSN Inversion issue