Horton

Re: Virtual generated columns

Tomasz Rybak <tomasz.rybak@post.pl>

View thread
From: Tomasz Rybak <tomasz.rybak@post.pl>
To: Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-06-17T19:31:18Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Expand virtual generated columns for ALTER COLUMN TYPE

  2. Eliminate code duplication in replace_rte_variables callbacks

  3. Expand virtual generated columns in the planner

  4. Virtual generated columns

  5. Additional tests for stored generated columns

  6. Improve generated_stored test

  7. Fix handling of CREATE DOMAIN with GENERATED constraint syntax

  8. Add pg_constraint rows for not-null constraints

  9. Put generated_stored test objects in a schema

  10. Rename regress test generated to generated_stored

  11. Small code simplification

  12. Remove useless code

  13. Remove useless initializations

  14. doc: Clarify that pg_attrdef also stores generation expressions

  15. Clean out column-level pg_init_privs entries when dropping tables.

  16. Re-implement the ereport() macro using __VA_ARGS__.

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 <serpent@debian.org>
GPG: A565 CE64 F866 A258 4DDC F9C7 ECB7 3E37 E887 AA8C