Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xPDO v2.5.3 - Issue with Datetime Default Values of Current_timestamp() #130

Open
deprolou opened this issue Jan 23, 2018 · 18 comments
Open

Comments

@deprolou
Copy link

Hey There,
I've been using xPDO as our ORM for 4 years now, so first of all thanks :)

We are having a problem with newer version of mysql (v10.2 MariaDB) as you may know, the default value for datetime and timestamps now is current_timestamp() instead of current_timestamp

This actually breaks the add method for xpdo, and there was no feasible quick workaround available other than downgrading the SQL Server.

I hope there's a workaround or a quick fix for this :)

@deprolou
Copy link
Author

I found out the issue, will just leave it here if anyone ever got to face this.

The db server is MariaDB 10.2 , was saving the default values of datetime of CURRENT_TIMESTAMP() to a lower case of current_timestamp() , which was causing the issue , as the xpdo classes does not ignore case sensitivity.

So i added the current_timestamp() in the xpdodriver class under mysql folder in the array _currentTimestamps.

Thanks :)

@wshawn
Copy link

wshawn commented Jan 23, 2018

I do not believe xPDO would be the issue here. xPDO should not be ignoring anything to do with the case.

cAsE !== casE !== CASE !== Case

Per the MariaDB docs: https://mariadb.com/kb/en/library/timestamp/
The database engine handles the case sensitivity as it is case-agnostic.

How are you implementing this?

$xpdoObject->save() handles everything for you. I would not think you should need to interact with MariaDB at all, i.e., xPDO is an ORM / ORB.

If you would like, take a look at my tutorial on modUser: Programmatically working with the modUser Object

@deprolou
Copy link
Author

deprolou commented Mar 1, 2018

Hey wshawn,
Sorry for not getting back on this!

Am not sure why this version of MariaDB did not handle the case sensitive default value of date time.

And i ended up amending the currentTimeStamps array in the xPDO mysql driver located at om/mysql/xpdodriver.class.php, and added current_timestamp() to the array.

Thanks

@deprolou deprolou closed this as completed Mar 1, 2018
@wshawn
Copy link

wshawn commented Mar 3, 2018

You should never edit the core files as they may be overwritten during updates.

@tartakxg
Copy link

tartakxg commented May 19, 2020

I believe that this issue has not been resolved.
10.3.22-MariaDB uses lowercase "current_timestamp()"

By using it in schema:
<field key="createdon" dbtype="timestamp" phptype="timestamp" null="false" default="current_timestamp()" />
I get error "Incorrect datetime value: '0000-00-00 00:00:00' for column createdon"

Why don't you add lowercase value to xpdodriver.class.php ?

@JoshuaLuckers
Copy link
Contributor

Does it work if you set CURRENT_TIMESTAMP as value for the default attribute instead of current_timestamp()?

@JoshuaLuckers JoshuaLuckers reopened this May 19, 2020
@wshawn
Copy link

wshawn commented May 19, 2020

Was the databases defined as _ci to handle the case issues in the table definitions?

@tartakxg
Copy link

tartakxg commented May 20, 2020

JoshuaLuckers Yes it works if set "CURRENT_TIMESTAMP" in schema. But I use UiCMPGenerator which automatically creates schema from DB.

@tartakxg
Copy link

wshawn please explain what you mean?

@JoshuaLuckers
Copy link
Contributor

JoshuaLuckers Yes it works if set "CURRENT_TIMESTAMP" in schema. But I use UiCMPGenerator which automatically creates schema from DB.

Thanks for reporting back, I guess this is an issue with UiCMPGenerator and not xPDO?

@tartakxg
Copy link

Why not? it is that simple to add "current_timestamp ()" to array, isn't it? I think other tools can also automatically get default value "current_timestamp ()" from DB.

@Mark-H
Copy link
Collaborator

Mark-H commented May 20, 2020

Why not both? :D

UiCMPGenerator apparently creates syntax that's unsupported, but there does seem to be merit to making xPDO support the additional syntax too.

@JoshuaLuckers
Copy link
Contributor

When displayed in the INFORMATION_SCHEMA.COLUMNS table, a default CURRENT TIMESTAMP is displayed as CURRENT_TIMESTAMP up until MariaDB 10.2.2, and as current_timestamp() from MariaDB 10.2.3, due to to MariaDB 10.2 accepting expressions in the DEFAULT clause.

Source: https://mariadb.com/kb/en/now/#description

@JoshuaLuckers
Copy link
Contributor

I think adding current_timestamp() is a good short term solution. In the long term I believe it's better to have a custom MariaDB driver but that's a different discussion.

@JoshuaLuckers
Copy link
Contributor

JoshuaLuckers commented May 20, 2020

If I'm not mistaken a workaround that doesn't require to modify the source:
This requires an instance of xPDO

$xpdo->getDriver()->$_currentTimestamps[] = 'current_timestamp()';

@tartakxg
Copy link

tartakxg commented May 21, 2020

JoshuaLuckers Thanks for advice. Tell me, please, where do you recommend adding this line so that there is support for all custom packages at once?

@wshawn
Copy link

wshawn commented May 21, 2020

wshawn please explain what you mean?

ci is for case-insensitive sorting and comparison. It tells the database to ignore case. As this issue is a bit old, it should have been mentioned that minimum versions of the database engine would have to be in place to support CURRENT_TIMESTAMP. xPDO handles it on the database manager class it uses to speak to PDO. An example from v.3 schema:

<field key="date_modified" dbtype="timestamp" phptype="timestamp" null="false" default="CURRENT_TIMESTAMP" attributes="ON UPDATE CURRENT_TIMESTAMP" />

You should never use internal MySQL functions (i.e. current_timestamp()) for this. That is what the ORM is for.

@wshawn
Copy link

wshawn commented May 21, 2020

If I'm not mistaken a workaround that doesn't require to modify the source:
This requires an instance of xPDO

$xpdo->getDriver()->$_currentTimestamps[] = 'current_timestamp()';

If you are working with Timezones, etc. Just have your code create the timestamp. You can override the save function in any xpdoObject for this purpose. It is a trivial thing.

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

No branches or pull requests

5 participants