public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomasz Rybak <[email protected]>
To: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re: Virtual generated columns
Date: Mon, 17 Jun 2024 21:31:18 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
On Wed, 2024-05-22 at 19:22 +0200, Peter Eisentraut wrote:
> On 29.04.24 10:23, Peter Eisentraut wrote:
> > Here is a patch set to implement virtual generated columns.
>
> > The main feature patch (0005 here) generally works but has a number
> > of
> > open corner cases that need to be thought about and/or fixed, many
> > of
> > which are marked in the code or the tests. I'll continue working
> > on
> > that. But I wanted to see if I can get some feedback on the test
> > structure, so I don't have to keep changing it around later.
>
> Here is an updated patch set. It needed some rebasing, especially
> around the reverting of the catalogued not-null constraints. I have
> also fixed up the various incomplete or "fixme" pieces of code
> mentioned
> above. I have in most cases added "not supported yet" error messages
> for now, with the idea that some of these things can be added in
> later,
> as incremental features.
>
This is not (yet) full review.
Patches applied cleanly on 76618097a6c027ec603a3dd143f61098e3fb9794
from 2024-06-14.
I've run
./configure && make world-bin && make check && make check-world
on 0001, then 0001+0002, then 0001+0002+0003, up to applying
all 5 patches. All cases passed on Debian unstable on aarch64 (ARM64)
on gcc (Debian 13.2.0-25) 13.2.0.
v1-0001-Rename-regress-test-generated-to-generated_stored.patch:
no objections here, makes sense as preparation for future changes
v1-0002-Put-generated_stored-test-objects-in-a-schema.patch:
also no objections.
OTOH other tests (like publication.out, rowsecurity.out, stats_ext.out,
tablespace.out) are creating schemas and later dropping them - so here
it might also make sense to drop schema at the end of testing.
v1-0003-Remove-useless-initializations.patch:
All other cases (I checked directory src/backend/utils/cache)
calling MemoryContextAllocZero only initialize fields when they
are non-zero, so removing partial initialization with false brings
consistency to the code.
v1-0004-Remove-useless-code.patch:
Patch removes filling in of constraints from function
BuildDescForRelation. This function is only called from file
view.c and tablecmds.c (twice). In none of those cases
result->constr is used, so proposed change makes sense.
While I do not know code well, so might be wrong here,
I would apply this patch.
I haven't looked at the most important (and biggest) file yet,
v1-0005-Virtual-generated-columns.patch; hope to look at it
this week.
At the same I believe 0001-0004 can be applied - even backported
if it'll make maintenance of future changes easier. But that should
be commiter's decision.
Best regards
--
Tomasz Rybak, Debian Developer <[email protected]>
GPG: A565 CE64 F866 A258 4DDC F9C7 ECB7 3E37 E887 AA8C
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Virtual generated columns
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox