On Sun, Feb 18, 2024 at 1:59 PM Andy Fan <zhihuifan1213@163.com> wrote:
>
>
> Hi Ashutosh,
>
> > data_type is described on that page as "Data type of the new column,
> > or new data type for an existing column." but CREATE TABLE
> > documentation [2] redirects data_type to [3], which mentions serial.
> > The impression created by the documentation is the second statement
> > above is a valid statement as should not throw an error; instead
> > change the data type of the column (and create required sequence).
>
> I didn't find out a reason to not support it, if have any reason, I
> think it is better have some explaination in the document.
>
> > In code ATPrepAlterColumnType() calls typenameTypeIdAndMod(), whereas
> > transformColumnDefinition() (called for ALTER TABLE ... ADD COLUMN and
> > CREATE TABLE) handles "serial" data type separately. Looks like we are
> > missing a call to transformColumnDefinition() in
> > transformAlterTableStmt() under case AT_AlterColumnType.
>
> I tried your idea with the attatchment, it is still in a drafted state
> but it can be used as a prove-of-concept and for better following
> communicating. Just one point needs to metion is serial implies
> "default value" + "not null" constaint. So when we modify a column into
> serial, we need to modify the 'NULL value' and only to the default value
> at the RewriteTable stage.
>
I am surprised that this requires changes in ReWrite. I thought adding
NOT NULL constraint and default value commands would be done by
transformColumnDefinition(). But I haven't looked at the patch close
enough.
--
Best Wishes,
Ashutosh Bapat