fix now() not working for new migrations default value #18845

LordSimal · 2025-08-21T12:02:53Z

@nook24 reported in slack, that

->addColumn('entry_time', 'datetime', [
    'timezone' => true,
    'default' => Literal::from('now()'),
    'limit'   => null,
    'null'    => false,
])

doesn't work in the new migrations plugin. It returned DEFAULT 'now()' which is not valid.

Reason behind that is the fact, that we didn't handle those cases separately till now. We only checked for current_timestamp

As far as I have checked in most DBMSs now() and current_timestamp are equal

src/Database/Schema/PostgresSchemaDialect.php

src/Database/Schema/SqliteSchemaDialect.php

nook24 · 2025-08-22T14:22:09Z

At least for MySQL, you could have pretty wild defaults such as DEFAULT (JSON_ARRAY()) and more https://dev.mysql.com/doc/refman/8.4/en/data-type-defaults.html

Instead of keeping a list, I would suggest that if the value of default is typeof Literal just keep the string as is and do not add any SQL escaping.

Only escape the value, if it's a string. I guess this is how the old migration handled it.

markstory · 2025-08-22T19:07:34Z

Instead of keeping a list, I would suggest that if the value of default is typeof Literal just keep the string as is and do not add any SQL escaping.

Yes, this would be ideal and is something I missed when porting code from migrations to cakephp/database.

LordSimal · 2025-08-23T15:47:57Z

But would such a change be doable in a patch release? This change at least can be relased in the next patch release IMHO, but the Literal change should be done in the next minor since it could change already existing behavior.

dereuromark · 2025-08-23T15:55:19Z

This change at least can be relased in the next patch release IMHO

But then you should maybe use the stricter approach mark suggested:

strtolower($column['default']) === 'now()'

Co-authored-by: Mark Story <[email protected]>

markstory · 2025-08-24T03:19:08Z

But would such a change be doable in a patch release? This change at least can be relased in the next patch release IMHO, but the Literal change should be done in the next minor since it could change already existing behavior.

Yes, I was also thinking about this today. I don't think we can do the change I proposed in a patch release. However, I would like to include it in 5.3 alongside the other schema system improvements & fixes. How about we merge this as a short term solution, and we can add more scenarios as they come up. I'll work on building a solution for using QueryExpression as default values for all column types, and migrations can manage the the Literal -> QueryExpression transformation logic.

markstory · 2025-08-24T03:36:38Z

tests/TestCase/Database/Schema/SqliteSchemaDialectTest.php

+                ['type' => 'datetime', 'null' => false, 'default' => 'now()'],
+                '"created_now" DATETIME NOT NULL DEFAULT current_timestamp',


Should this translation be happening? Is now() not portable?

sqlite doesn't have a now() function like MySQL does.

sqlite> SELECT current_timestamp; 2025-08-24 14:22:32 sqlite> SELECT now(); Parse error: no such function: now SELECT now(); ^--- error here sqlite> SELECT now; Parse error: no such column: now SELECT now; ^--- error here

Do we want to commit to having a translation for literal values? I would expect that if I use a literal, I get what I provided without the framework changing it.

True, but then we need some DBMS agnostic way to represent those values which is not a magic string.

I expect a migrations plugin to define my schema once and no matter which DBMS is used behind it, it should create the correct schema (if the features are supported of course)

I expect a migrations plugin to define my schema once and no matter which DBMS is used behind it, it should create the correct schema (if the features are supported of course)

But the now() function isn't something that is supported by all database vendors. Given that the context of this issue is for literal expressions I don't think we should be providing translations for those literals, as we won't be able to do that in the future when literal support is expanded.

Basically I'm with you that a patch release should never introduce any breaking changes. But the current documentation describes Literal::from() as it used to be

Yeah this was a miss on my part when I was doing the phinx cleanup. The diff in #18858 is pretty minimal and could be merged into 5.x instead of 5.next. That change along with a fix for migrations should get us back to a working state for Literal values more broadly.

nook24 · 2025-08-25T10:56:01Z

But would such a change be doable in a patch release? This change at least can be relased in the next patch release IMHO, but the Literal change should be done in the next minor since it could change already existing behavior.

Yes, I was also thinking about this today. I don't think we can do the change I proposed in a patch release.

Basically I'm with you that a patch release should never introduce any breaking changes. But the current documentation describes Literal::from() as it used to be: https://book.cakephp.org/migrations/4/en/writing-migrations.html#custom-column-types-default-values

I'm using "cakephp/migrations": "@stable" in my composer.json and have version 4.7.0 installed (CakePHP 5.2.6). In the past I had cakephp/migrations version 3.9.0 with CakePHP 4.6.1 as I had to support PHP 7 for a while.

With the changes from cakephp/cakephp#18858 these changes should resolve the regressions around `Literal` values raised in cakephp/cakephp#18845

arthusantiago · 2025-08-26T02:00:13Z

I have a similar problem to this, but with uuid. My code:
->addColumn('id_secundario', 'uuid', ['default' => Literal::from('uuid_generate_v4()')])

When i execute the migration:

SQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for type uuid: "uuid_generate_v4()" no caractere 100
Query: CREATE TABLE "public"."sessions" ("id" VARCHAR(40) NOT NULL, "id_secundario" UUID NOT NULL DEFAULT 'uuid_generate_v4()', "data" BYTEA DEFAULT NULL, "expires" INT DEFAULT NULL, "user_id" INT DEFAULT NULL, "user_agent" VARCHAR(256) DEFAULT NULL, "created" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, "modified" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, CONSTRAINT "sessions_pkey" PRIMARY KEY ("id"));

My config:

Cakephp: 5.1.0
DBMS: Postgres

markstory · 2025-08-26T02:46:54Z

@arthusantiago What version of migrations are you using?

arthusantiago · 2025-08-26T12:40:27Z

@arthusantiago What version of migrations are you using?

"cakephp/migrations": "4.7.1",

markstory · 2025-08-27T04:32:58Z

"cakephp/migrations": "4.7.1",

@arthusantiago How is that possible with cakephp 5.1? The requirements for 4.7.1 require at least 5.2. Any fixes I'd be able to make should only apply to new code added in 5.2 as well.

markstory · 2025-08-27T04:34:10Z

With no additional feedback on #18858 I'd like to merge that and the related migrations change so that we can get a fix out sooner.

arthusantiago · 2025-08-27T12:56:27Z

@markstory
I'm sorry, Mark. I was looking in the wrong place. I updated my dependencies. Running the 'composer licenses' command, I get the following result:

markstory · 2025-08-28T00:30:17Z

I'm sorry, Mark. I was looking in the wrong place. I updated my dependencies.

Ok. That output makes more sense. 👍

LordSimal · 2025-08-31T17:31:10Z

With https://github.com/cakephp/cakephp/pull/18858/files being merged and released with 5.2.7, I'd say wen can close this as the desired behavior can be achieve via using a QueryExpression

* Fix Literal default values With the changes from cakephp/cakephp#18858 these changes should resolve the regressions around `Literal` values raised in cakephp/cakephp#18845 * Bump cake version to test against QueryExpression fix * Revert "Bump cake version to test against QueryExpression fix" This reverts commit a7ee824.

fix now() not working for new migrations default value

9f2bf6b

LordSimal added this to the 5.2.7 milestone Aug 21, 2025

LordSimal added the defect label Aug 21, 2025

LordSimal requested a review from markstory August 21, 2025 12:33

markstory reviewed Aug 22, 2025

View reviewed changes

src/Database/Schema/PostgresSchemaDialect.php Outdated Show resolved Hide resolved

src/Database/Schema/SqliteSchemaDialect.php Outdated Show resolved Hide resolved

markstory self-assigned this Aug 22, 2025

LordSimal and others added 2 commits August 23, 2025 18:01

Apply suggestions from code review

8824ac9

Co-authored-by: Mark Story <[email protected]>

unify checking logic for current_timestamp and now to be more strict

3455264

LordSimal force-pushed the 5.x-fix-now-in-defaults branch from e547501 to 3455264 Compare August 23, 2025 16:15

markstory reviewed Aug 24, 2025

View reviewed changes

markstory mentioned this pull request Aug 24, 2025

Schema default expression #18858

Merged

markstory added a commit to cakephp/migrations that referenced this pull request Aug 25, 2025

Fix Literal default values

9cd64aa

With the changes from cakephp/cakephp#18858 these changes should resolve the regressions around `Literal` values raised in cakephp/cakephp#18845

markstory mentioned this pull request Aug 25, 2025

Fix Literal default values cakephp/migrations#891

Merged

markstory modified the milestones: 5.2.7, 5.2.8 Aug 31, 2025

LordSimal closed this Aug 31, 2025

LordSimal deleted the 5.x-fix-now-in-defaults branch August 31, 2025 17:31

		['type' => 'datetime', 'null' => false, 'default' => 'now()'],
		'"created_now" DATETIME NOT NULL DEFAULT current_timestamp',

fix now() not working for new migrations default value #18845

fix now() not working for new migrations default value #18845

Uh oh!

Conversation

LordSimal commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nook24 commented Aug 22, 2025

Uh oh!

markstory commented Aug 22, 2025

Uh oh!

LordSimal commented Aug 23, 2025

Uh oh!

dereuromark commented Aug 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markstory commented Aug 24, 2025

Uh oh!

markstory Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

LordSimal Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

markstory Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

LordSimal Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

markstory Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

nook24 commented Aug 25, 2025

Uh oh!

arthusantiago commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markstory commented Aug 26, 2025

Uh oh!

arthusantiago commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markstory commented Aug 27, 2025

Uh oh!

markstory commented Aug 27, 2025

Uh oh!

arthusantiago commented Aug 27, 2025

Uh oh!

markstory commented Aug 28, 2025

Uh oh!

LordSimal commented Aug 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

LordSimal commented Aug 21, 2025 •

edited

Loading

dereuromark commented Aug 23, 2025 •

edited

Loading

arthusantiago commented Aug 26, 2025 •

edited

Loading

arthusantiago commented Aug 26, 2025 •

edited

Loading