-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
DBAL-22: Saving UTC Offset in Postgres DATETIME messes with PHP DateTime and Timezones #1394
Comments
Comment created by @beberlei: throwing an exception is a no-go in my opinion, it would completely cripple an application if there is some change in the DB. There are no checks if an integer field is really in int, or if the db has a varchar field making the (int) conversion fail. This should be ignored and is responsibility of the user to get right. I am looking into the format, however we have several unit-tests for this working correctly, i am a bit confused. |
Comment created by @beberlei: The question is rather, should we deprecate the usage of timezones alltogether? This is a good read on the topic: http://derickrethans.nl/storing-date-time-in-database.html |
Comment created by @beberlei: ok the current state is, we use "TIMESTAMP(0) WITH TIMEZONE", which matches the currently given format "Y-m-d H:i:sO" |
Comment created by auroraeosrose: The "TIMESTAMP(0) WITH TIMEZONE" is a very UNCOMMON time field Usually timstamp without time zone is used Also the difference between integer field is really in int and db has a varchar field making the (int) is different for one reason The timestamp is returning a datetime object - not a scalar What happens when you try to use something in PHP that's not an object as an object? It fatal errors If the integer field is not really an integer when you get your data back from the db and you attempt to use it, PHP is not going to throw a fatal error at you. Your application might be angry, but that's another story. I would almost rather you simply use strtotime and pass that to the datetime class for postgresql since it will work on all 'versions" of postgresql's timestamp, regardless if it has microseconds or timezone information. Then you're not locking all users of doctrine into using one timestamp format. |
Comment created by auroraeosrose:
Quick test - why rely on createFromFormat when the datetime constructor is smart enough to handle all formats? |
Comment created by @beberlei: One reason is probably performance, using the constructor takes double the time:
Since the DateFormat is pretty static, therefore using createFromFormat() seems an obvious choice. This is of course mainly a good reason for MySQL where you cannot change the format, and Postgres, Oracle and DB2 allow changes in precision and formatting of the date which make this decision a little bit more complicated. |
Comment created by auroraeosrose: I think everyone BUT mysql allow changes in the precision and formatting of the date - heck SQL Server is the worst for "make the date be whatever you want" I think this would fall into "premature optimization" - yes it's a great speed improvement for mysql, but it's at the expense of all other DBs... |
Comment created by @beberlei: A Doctrine\DBAL\DBALException due to a wrong date in the database probably leads to a fatal error in any userland code, or would anybody catch all the errors? how would you handle them? We could think of changing the type to be without timezone, that would be a pretty massive BC though since it affects users database schemas. Another option would be a configurable format per Postgres/Oracle/DB2 Platform:
However Type instances are flyweight instances, that means it would have to be the format of all Doctrine DateTime typed columns. However this way you would at least have full control over the format, not depending on any Doctrine 2 interpretation. You can of course add your own datetime type and overwrite the existing or use different ones. Maybe we should supply two DateTime types. |
Comment created by auroraeosrose: I don't think an exception is a good idea However a configurable would probably be a good idea... For databases that talk to other things, not just PHP via Doctrine, dictating the format of the datetime fields is not an option ;) |
Comment created by @beberlei: I changed the title to reflect the real issue of this ticket, using Postgresql with WITH TIMEZONE is rather problematic with regard to DateTime and DateTimeZone Handling inside PHP, see the following code snippets (http://pastie.org/1009033) and Dericks post (http://derickrethans.nl/storing-date-time-in-database.html). We should change the default behaviour in Postgres (and Oracle) to be WITHOUT TIMEZONE, since this is the 90% use-case. Additionally it may create less bugs when we don't fiddle with the Timezone used for creation. |
Comment created by @beberlei: Ok we also discussed errors when conversion failed and added a ConversionException. We also implemented a new type DateTimeTz. This is both currently in my feature branch: http://github.com/doctrine/dbal/tree/[DBAL-22](http://www.doctrine-project.org/jira/browse/DBAL-22) |
Comment created by @beberlei: This is a copy of a mail going to the doctrine-dev and doctrine-user lists some minutes ago: Hello Doctrine 2 + Postgres and Oracle Users, Both Postgres and Oracle currently save the Date Offset for DateTime As a result we have to change the DateTime type implementations of Required changes inside Doctrine DBAL Package:
What does that mean to you as a user? POSTGRES: There are two solutions if you use Postgres:
ORACLE: Oracle does not permit changing the types when there is already data in Planed Schedule for the changes:
I will send an additional notice to the lists when we will bump the ORM Sorry for the inconvenience regarding this issue, but we feel very Thanks goes to Elisabeth Smith for bringing this issue to the table and greetings, |
Comment created by @beberlei: Merged into Master now |
Issue was closed with resolution "Fixed" |
Comment created by drevolution: Hi, on master branch of DBAL package in PostgreSqlPlatform::getDateTimeTzFormatString() I see this:
But PostgreSQL stores timestamps with microseconds, so format should be more likely:
Yes, Doctrine converts DateTimeTz from PHP value to database value without microseconds, but if I will use some database function (for example now()) for get timestamp and I will store it to the database directly and than I will read this values through Doctrine, it will cause an error. Is there any reason for not using default PostgreSQL format of timestamp with microseconds? |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Jira issue originally created by user @guilhermeblanco:
The DateTime type is currently unusable in PostgreSQL due to a wrong parsing format.
PostgreSqlPlatform::getDateTimeFormatString() should be changed from:
Y-m-d H:i:sO
to
Y-m-d H:i:s.uO
According to http://twitter.com/auroraeosrose/statuses/16154268629
Also, there's no way to check if a convert From and To PHP/Database worked correctly. Maybe we should introduce an exception when a DBAL\Type conversion fails, otherwise, there's no way to figure it out what happened.
The text was updated successfully, but these errors were encountered: