Обсуждение: serial not accepted as datatype in ALTER TABLE ... ALTER COLUMN

Hi All,
alter table t1 add column c serial;
ALTER TABLE

this works, but not
#alter table t1 add column c int;
ALTER TABLE
#alter table t1 alter column c type serial;
ERROR:  type "serial" does not exist

Looking at the documentation [1], the grammar for both mentions data_type

ADD [ COLUMN ] [ IF NOT EXISTS ] column_name data_type and

ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type

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).

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.

[1] https://www.postgresql.org/docs/current/sql-altertable.html
[2] https://www.postgresql.org/docs/16/sql-createtable.html
[3] https://www.postgresql.org/docs/16/datatype.html

-- 
Best Wishes,
Ashutosh Bapat

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.

-- 
Best Regards
Andy Fan

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

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

> On Sun, Feb 18, 2024 at 1:59 PM Andy Fan <zhihuifan1213@163.com> wrote:
>>
>>
>> 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.

Hmm, I think this depends on how to handle the NULL values before the
RewriteTable.

Consider the example like this:

\pset null (null)
create table t(a int);
insert into t select 1;
insert into t select;

postgres=# select * from t;
   a
--------
      1
 (null)
(2 rows)

since serial type implies "not null" + "default value", shall we raise error
or fill the value with the "default" value?  The patch choose the later
way which needs changes in RewirteTable stage, but now I think the raise
error directly is an option as well.

--
Best Regards
Andy Fan