Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch

From: Zoltan Boszormenyi <zb(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-patches(at)postgresql(dot)org
Subject: Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch
Date: 2007-04-04 17:44:16
Message-ID: 4613E3F0.7000908@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers 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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2007-04-04 18:04:50 elog(FATAL) vs shared memory
Previous Message Bruce Momjian 2007-04-04 17:28:12 Re: PL/Python warnings in CVS HEAD

Browse pgsql-patches by date

  From Date Subject
Next Message Gregory Stark 2007-04-04 18:06:50 Re: Auto Partitioning
Previous Message Pavan Deolasee 2007-04-04 17:37:19 Re: HOT WIP Patch - version 6.3