Re: Virtual generated columns

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.