Re: Virtual generated columns

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Subject: Re: Virtual generated columns
Date: 2024-08-29 13:35:49
Message-ID: CACJufxGyuYDTHSwGfKavzAq_JypbAgH4k2ygqhgdycSD9RJk7A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 29, 2024 at 8:15 PM Peter Eisentraut <peter(at)eisentraut(dot)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(dot)a(dot)rasheed(at)gmail(dot)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.

Attachment Content-Type Size
v4-0001-Virtual-generated-columns_minorchange.no-cfbot application/octet-stream 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amul Sul 2024-08-29 13:46:49 Re: pg_verifybackup: TAR format backup verification
Previous Message Amit Langote 2024-08-29 13:34:17 Re: generic plans and "initial" pruning