ALTER COLUMN TYPE doesn’t work if the column being altered has a default value

[dikshant]

Alter column type doesn’t work if the column has a default value.

For example:

CREATE TABLE test2 (
   testytest INT4 NOT NULL DEFAULT 0:::INT8
);
ALTER TABLE test2 ALTER COLUMN testytest TYPE int2 USING testytest::int2;

Will fail with:

ERROR: relation "test2" (104): computed column "testytest_1" cannot also have a DEFAULT expression
SQLSTATE: 42P16

But this will work without the DEFAULT

CREATE TABLE test2 (
   testytest INT4 NOT NULL
);
ALTER TABLE test2 ALTER COLUMN testytest TYPE int2 USING testytest::int2;

Epic CRDB-25314

Jira issue: CRDB-36224

[uds5501]

Hi @rimadeodhar, can I pick this issue to contribute? There doesn't seem to be any takers for this and seems perfect for a newcomer like me.

[cjhetzle]

@uds5501 have you been working on this? What is the new behavior going to do to the existing default?

If we want, we can remove the default or we can try to convert it as well.

[uds5501]

@cjhetzle I am currently exploring the codebase and checking relevant files to change. My primary thought was to remove the default altogether, however if conversion is feasible internally, I could explore that as well (however this might not be be the case everytime.)

[sahilwahi7]

func (node AlterTableSetDefault) Format(ctx FmtCtx) { ctx.WriteString(" ALTER COLUMN ") ctx.FormatNode(&node.Column) if node.Default == nil { ctx.WriteString(" DROP DEFAULT") } else { ctx.WriteString(" SET DEFAULT ") ctx.FormatNode(node.Default) } }

This function should be responsible for ALTER table having default feature added in node need to fix logic here @uds5501 are you looking into similar part of code?

[uds5501]

I think it's got to be AlterColumnType , writing tests to understand that.

[sahilwahi7]

@cjhetzle @uds5501 can you please let me do we need to remove the default one or need to update handling for that Found the codebase causing the issue In AlterColumnType we have a function called alterColumnTypeGeneral in which we have a check added for

  hasDefault := col.HasDefault()
    if hasDefault {
        if validCast := cast.ValidCast(col.GetType(), toType, cast.ContextAssignment); !validCast {
            return pgerror.Newf(
                pgcode.DatatypeMismatch,
                "default for column %q cannot be cast automatically to type %s",
                col.GetName(),
                toType.SQLString(),
            )
        }
    }

Here we need to remove the hasDefault Check

[uds5501]

hey @sahilwahi7 , since you've debugged the issue in depth, you can carry on with the dev of the same. Feel free to raise an MR

[sahilwahi7]

@uds5501 please go ahead with raising PR , it was your initial hint from which i was able to debug

[uds5501]

Hi @sahilwahi7 , I am not able to setup cockroach locally over the weekend and don't want myself to be a blocker if someone else can contribute to this. Please feel free to raise the MR.

[krishnakeshan]

@dikshant In your example:

CREATE TABLE test2 (
   testytest INT4 NOT NULL DEFAULT 0:::INT8
);

why do we cast the default value to INT8 when the column is defined to be an INT4? Just curious to know if that's relevant to the issue?

[mohitsethia]

@sahilwahi7 @uds5501 the issue is not in alterColumnType, but in the validator pkg>sql>catalog>tabledesc>validate.go (line 1144).

    if column.IsComputed() {
            if column.HasDefault() {
                return pgerror.Newf(pgcode.InvalidTableDefinition,
                    "computed column %q cannot also have a DEFAULT expression",
                    column.GetName(),
                )
            }
            if column.HasOnUpdate() {
                return pgerror.Newf(pgcode.InvalidTableDefinition,
                    "computed column %q cannot also have an ON UPDATE expression",
                    column.GetName(),
                )
            }
    }

@dikshant I am new here, and not sure what needs to be done, as it was a bug with clusters & cluster backup which contain tables with such columns as described here. What should be the new behaviour/additional check that would resolve this issue?

[adityeah8969]

@dikshant

I was not able to reproduce this bug. The "alter" command which error'd out in your case works fine for me

root@localhost:26257/bank> ALTER TABLE test2 ALTER COLUMN testytest TYPE int2 USING testytest::int2;               
NOTICE: ALTER COLUMN TYPE changes are finalized asynchronously; further schema changes on this table may be restricted until the job completes; some writes to the altered column may be rejected until the schema change is finalized
ALTER TABLE

Time: 590ms total (execution 589ms / network 0ms)

root@localhost:26257/bank> \d test2;                                                                               
Table "public.test2"
   Column   |   Type   | Collation | Nullable |    Default
------------+----------+-----------+----------+-----------------
  testytest | smallint |           | not null | 0
  rowid     | bigint   |           | not null | unique_rowid()

I have set the following to true though SET enable_experimental_alter_column_type_general = true;

Having said since this ticket might be lying around for a while, we may want to consider closing this if we do not expect error anymore.

[spilchen]

Closing this as I believe it was fixed with #126033