Skip to content

Conversation

@qnnp-me
Copy link

@qnnp-me qnnp-me commented Dec 8, 2025

  • Change start_time column type to datetime
  • Change end_time column type to datetime

- Change start_time column type to datetime
- Change end_time column type to datetime
@lutdev
Copy link

lutdev commented Dec 8, 2025

Just for my curious, what problem should these changes fix?)

@qnnp-me
Copy link
Author

qnnp-me commented Dec 8, 2025

Just for my curious, what problem should these changes fix?)

for "2038-01-19 03:14:07 UTC"

@dereuromark
Copy link
Member

In general datetime nowadays is sure a better default for normal use.
I wonder though if this is BC in this case.

- Introduce a FeatureFlags class to control the type of timestamp columns
- Select DATETIME or TIMESTAMP type based on the addTimestampsUseDateTime flag
- Update the timestamp column creation logic in the schema table to support dynamic type selection
@qnnp-me
Copy link
Author

qnnp-me commented Dec 9, 2025

In general datetime nowadays is sure a better default for normal use. I wonder though if this is BC in this case.

Overall, using DATETIME as the default is indeed a better choice for general use today. Regarding your concern about backward compatibility (BC), this has been addressed in the code. By introducing the FeatureFlags::$addTimestampsUseDateTime flag, the system can now dynamically choose between DATETIME and TIMESTAMP column types, ensuring that existing behaviors remain unaffected while allowing the improved default for new implementations.

@lutdev
Copy link

lutdev commented Dec 9, 2025

@qnnp-me what if it would be possible to change column type automatically?) if feature flag is provided, of course. To update the existing table in the database.

@qnnp-me
Copy link
Author

qnnp-me commented Dec 9, 2025

@lutdev I agree with your approach. For new projects, setting the preferred column type early via the feature flag is ideal. For existing projects, automatically changing column types based on the flag could be risky and unexpected. Instead, developers can manually create and run a migration script when needed to update their tables. This keeps control with the developer and avoids unintended changes to production databases.

@MasterOdin MasterOdin changed the title refactor(db): Change timestamp column types to datetime Use timestamp column type for phinxlog if addTimestampsUseDateTime flag disabled Dec 12, 2025
Copy link
Member

@MasterOdin MasterOdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the docs for the feature flag to indicate that it also modifies the behavior of creating the phinxlog table in https://github.com/cakephp/phinx/blob/0.x/docs/en/configuration.rst#feature-flags as well as its docstring (

* @var bool Should Phinx create datetime columns for addTimestamps instead of timestamp?
).

@MasterOdin
Copy link
Member

When I put out the next version with this PR, I will include in the notes on updating the phinxlog column types manually for those on older versions. I agree that I don't think it's worth trying to automatically migrate folks.

@qnnp-me qnnp-me requested a review from MasterOdin December 13, 2025 01:04
…mments

- Added explanation in the configuration documentation regarding the impact of the `add_timestamps_use_datetime` setting on the schema table
- Updated the comments for `addTimestampsUseDateTime` in the FeatureFlags class to clarify its effect on the schema table
- Ensured consistency between the documentation and code comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants