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?