Horton

Re: Virtual generated columns

jian he <jian.universality@gmail.com>

View thread
From: jian he <jian.universality@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Dean Rasheed <dean.a.rasheed@gmail.com>
Date: 2024-08-29T13:35:49Z
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__.

Attachments

On Thu, Aug 29, 2024 at 8:15 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
>
> > drop table if exists comment_test cascade;
> > CREATE TABLE comment_test (
> >    id int,
> >    positive_col int  GENERATED ALWAYS AS (22) CHECK (positive_col > 0),
> >    positive_col1 int  GENERATED ALWAYS AS (22) stored CHECK (positive_col > 0) ,
> >    indexed_col int,
> >    CONSTRAINT comment_test_pk PRIMARY KEY (id));
> > CREATE INDEX comment_test_index ON comment_test(indexed_col);
> > ALTER TABLE comment_test ALTER COLUMN positive_col1 SET DATA TYPE text;
> > ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE text;
> > the last query should work just fine?
>
> I played with this and I don't see anything wrong with the current
> behavior.  I noticed that in your test case
>
>  >    positive_col1 int  GENERATED ALWAYS AS (22) stored CHECK
> (positive_col > 0) ,
>
> you have the wrong column name in the check constraint.  I'm not sure if
> that was intentional.
>

That's my mistake. sorry for the noise.


On Wed, Aug 21, 2024 at 6:52 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
>
> Another argument for doing it that way round is to not add too many
> extra cycles to the processing of existing queries that don't
> reference generated expressions. ISTM that this patch is potentially
> adding quite a lot of additional overhead -- it looks like, for every
> Var in the tree, it's calling get_attgenerated(), which involves a
> syscache lookup to see if that column is a generated expression (which
> most won't be). Ideally, we should be trying to do the minimum amount
> of extra work in the common case where there are no generated
> expressions.
>
> Regards,
> Dean



>
> The new patch does some rebasing and contains various fixes to the
> issues you presented.  As I mentioned, I'll look into improving the
> rewriting.


based on your latest patch (v4-0001-Virtual-generated-columns.patch),
I did some minor cosmetic code change
and tried to address get_attgenerated overhead.

basically in expand_generated_columns_in_query
and expand_generated_columns_in_expr preliminary collect (reloid,attnum)
that have generated_virtual flag into expand_generated_context.
later in expand_generated_columns_mutator use the collected information.

deal with wholerow within the expand_generated_columns_mutator seems
tricky, will try later.