Alter table doesn't work when timestamp column has a NOT NULL constraint

[okrsnak]

Same issue was already discussed in #436 but it is marked as solved but without a solution (other than drop the database and recreate without altering)

I have a news table

CREATE TABLE `news` (
	`id` serial AUTO_INCREMENT PRIMARY KEY NOT NULL,
	`order` int NOT NULL,
	`url` varchar(500) NOT NULL,
	`heading` varchar(256) NOT NULL,
	`title` varchar(256) NOT NULL,
	`description` varchar(500) NOT NULL,
	`published_from` timestamp NOT NULL,
	`published_to` timestamp NOT NULL,
	`created_at` timestamp NOT NULL DEFAULT (now()),
	`updated_at` timestamp NOT NULL DEFAULT (now())
);

and when I try to alter the table

ALTER TABLE `news` MODIFY COLUMN `id` varchar(256) NOT NULL;

I receive an error

target: emporium-dev.-.primary: vttablet: rpc error: code = InvalidArgument desc = Invalid default value for 'created_at' (errno 1067) (sqlstate 42000) (CallerID: pn343zlaka789ulu4ii0): Sql: "alter table news modify column id varchar(256) not null", BindVars: {REDACTED}

I found out (as stated also in the previous discussion) that the issue is NOT NULL constraint for timestamp column.
When I remove the NOT NULL for created_at column, the error message just changes to updated_at.
When I remove the NOT NULL constraint for both columns, the alter command works.

[orware]

Hello @okrsnak,

I took a closer look here on this one for and I believe I have a solution that I can share.

It appears that either of the following should work for your needs:

ALTER TABLE `news`
	MODIFY COLUMN `id` varchar(256) NOT NULL,
	MODIFY COLUMN `created_at` timestamp NOT NULL DEFAULT current_timestamp(),
	MODIFY COLUMN `updated_at` timestamp NOT NULL DEFAULT current_timestamp();
	
ALTER TABLE `news`
	MODIFY COLUMN `id` varchar(256) NOT NULL,
	MODIFY COLUMN `created_at` timestamp NOT NULL DEFAULT (now()),
	MODIFY COLUMN `updated_at` timestamp NOT NULL DEFAULT (now());

Interestingly, I was wondering if you had come across this situation when using our deploy request process initially, and when testing that out myself I didn't encounter any issues, but I did observe that even though I only ran the single ALTER TABLE for the id column, the deploy request process automatically included an adjustment to the DEFAULT values for those other two columns as shown below:

When that deploy request succeeded, I checked on your database and noticed that it didn't appear that you were using the deploy request process yet and your branch was still set as a development one, which would allow schema changes to be made directly on it.

So I tested things out again this way and created the table and then ran the alter and I was able to encounter the same issue you shared:

From there, I tested to see if making an alter on those other columns to use CURRENT_TIMESTAMP() could help, but like you noticed in some of your tests, it would flip-flop to the other timestamp column in the error when I tried to do this:

Instead, it seemed that to be successful in this situation we need to combine the changes all together into a single statement using either of the two approaches I shared above:

In most cases, this wouldn't be necessary, but for some reason this situation requires it to be done in a combined manner currently.

I'll likely file an issue internally sharing this since it could potentially be a bug on our end, but for now the above option should be a workable solution in a case like this. I would expect that if a table had more timestamp columns that they would also have to be included too.

[okrsnak]

Hello @orware,
Thank you for the answer.
You are correct, I didn't use the deploy request process but I made the changes directly via web console.
I tested your solution and it works, thanks for the help.

[saborrie]

Just want to jump in and say that I encountered this issue too. I am using DrizzleORM.

-- my table already had these in:
`created_at` timestamp NOT NULL DEFAULT (now()),
`updated_at` timestamp NOT NULL DEFAULT (now())

-- SQL from drizzle, does not work:
ALTER TABLE `task_lists` ADD `slug` varchar(256);
ALTER TABLE `task_lists` ADD `owner_id` varchar(256);

-- working replacement based on this thread:
ALTER TABLE task_lists 
	MODIFY COLUMN `created_at` timestamp NOT NULL DEFAULT (now()),
	MODIFY COLUMN `updated_at` timestamp NOT NULL DEFAULT (now()),
	ADD slug varchar(256),
	ADD `owner_id` varchar(256);

Has this been filed as a bug? It's not ideal to have to manually fix migrations like this.

[orware]

Hello @saborrie,

This is the first time I am seeing the issue occurring in relation to usage of an ORM, but are you also not making use of the deploy request process currently?

That seemed to be a factor in that initial reply I wrote up above since the deploy request process seemed to pick up on the need to include those columns automatically.

It could still be considered a bug/limitation if you are currently operating with the "safe migrations" feature turned off and you aren't making use of the deploy request process at this time, but it would be helpful to go ahead and confirm that first and then I can go ahead and submit a related bug request internally with the details we already have here in the discussion.

[saborrie]

Hi @orware, thanks for your reply.

I think I've confused things by using the word "migration" when I said "It's not ideal to have to manually fix migrations like this." - I was referring to the migration files that my ORM creates, not the PlanetScale deploy request system. Although I've used an ORM to generate the SQL code to make schema changes, I could have written it by hand and encounter the same issue.

---

If I create a non-production branch, add a task_lists table with:

...
`created_at` timestamp NOT NULL DEFAULT (now())
...

Then this bug prevents me from running

ALTER TABLE task_lists 
	ADD slug varchar(256);

The workaround we have here is to include the modify column again like this:

ALTER TABLE task_lists 
	MODIFY COLUMN `created_at` timestamp NOT NULL DEFAULT (now()),
	ADD slug varchar(256);

I guess the other workaround is to tear down my non-production branch's schema and recreate it in one go. Which I understand is sort of the idiomatic way to use planetscale, and obviates the need for ORM style migration files entirely.

I'm new to using planetscale, so I've not completely figured out how best to organise my workflow with it yet. But this does seem like a bug, even if it can be worked around, it's still kinda annoying and it would be worth it for planetscale to fix it.

[orware]

Hello @saborrie,

That's definitely understood as the situation wouldn't occur when the safe migrations feature is enabled and using the deploy request workflow.

I was able to walk through my earlier tests once more to confirm that when using a database with a single branch and the safe migrations feature stays disabled that the issue can occur.

Afterwards I submitted an internal bug report on this issue for correction sometime in the future if it is confirmed to be a bug, which does seem to be the case since I also would expect to be able to run those ALTER TABLE statements with just the columns being added/modified even if I had timestamp columns with NOT NULL constraints present in the table.

Requiring users to know that they would need to include the timestamp columns that have NOT NULL constraints within the ALTER TABLE changes doesn't lead to the best user experience and likely works without that extra requirement within a regular MySQL 8 instance, so I'm hopeful that this is something we may be able to get addressed down the road for users.

This appears to be getting encountered a bit more commonly now likely due to us not requiring safe migrations be enabled by default as we used to, which allows this situation to occur due to the direct schema changes occurring on a database's primary branch.