Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch - Mailing list pgsql-patches

From Zoltan Boszormenyi
Subject Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch
Date
Msg-id 4613E3F0.7000908@cybertec.at
Whole thread Raw
In response to Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Tom Lane írta:
> I wrote:
>
>> I see another problem with this patch: the code added to
>> ATExecDropColumn is a crude hack.  It doesn't work anyway since this is
>> not the only possible way for columns to be dropped (another one that
>> comes to mind immediately is DROP TYPE ... CASCADE).  The only correct
>> way to handle things is to let the dependency mechanism do it.
>>

I will try that.

> Actually, the whole question of dependencies for generated columns
> probably needs some thought.  What should happen if a function or
> operator used in a GENERATED expression gets dropped?  The result
> for a normal column's default expression is that the default expression
> goes away, but the column is still there.  I suspect we don't want
> that for a GENERATED column --- it would then be effectively a plain
> column.
>

No, I would want the DROP FUNCTION to be cancelled if used in
a GENERATED, but a DROP ... CASCADE would drop it, too.
So, DEPENDENCY_NORMAL will keep the referencing object
but DEPENDENCY_AUTO would drop it too if the referenced
object is dropped?

> Along the same lines, is ALTER COLUMN DROP DEFAULT a legal operation
> on a generated column?  What about just replacing the expression with
> ALTER COLUMN SET DEFAULT?
>

Neither of these options are legal for GENERATED columns,
AFAIK I prohibited them already.

> Before you get too excited about making generated columns disappear
> automatically in all these cases, consider that dropping a column
> is not something to be done lightly --- it might contain irreplaceable
> data.
>

The standard says that the GENERATED column should be
dropped silently if either of the referenced columns is dropped.
I haven't seen anything about the expression, though.

> On second thought maybe the right approach is just to allow the default
> expression to be dropped the same as it would be for an ordinary column,
> and to make sure that if a GENERATED column doesn't (currently) have a
> default, it is treated the same as an ordinary column.
>
> This leads to the further thought that maybe GENERATED is not a property
> of a column at all, but of its default (ie, it should be stored in
> pg_attrdef not pg_attribute, which would certainly make the patch less
> messy).  AFAICS plain GENERATED merely indicates that the default
> expression can depend on other columns, which is certainly a property
> of the default --- you could imagine ALTER COLUMN SET DEFAULT GENERATED
> AS ... to make a formerly plain column into a GENERATED one.  I'm not
> entirely sure about ALWAYS though.
>

The standard says somewhere that GENERATED columns
can only be added to and dropped from a table.

My observation is: I deleted my hack from ATExecDropColumn()
and now I cannot drop the referenced column without CASCADE.
The comment in StoreAttrDefault() says the objects in the expression
will have dependencies registered. I guess "objects" also means functions?
This way, if I do explicit recordDependencyOn(, DEPENDENCY_AUTO)
on referenced columns then the standard requirement will
be satisfied, i.e. dropping columns will drop GENERATED columns
silently that reference the said column but . Am I right? Or how about using
recordDependencyOnSingleRelExpr(... , DEP_NORMAL, DEP_AUTO ) ?

>             regards, tom lane
>
>


--
----------------------------------
Zoltán Böszörményi
Cybertec Geschwinde & Schönig GmbH
http://www.postgresql.at/


pgsql-patches by date:

Previous
From: "Pavan Deolasee"
Date:
Subject: Re: HOT WIP Patch - version 6.3
Next
From: Tom Lane
Date:
Subject: Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch