Just an idea to avoid limitations in date intervals to days and months on more flexible DBMS ... #62

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
5 participants

cariboo commented Sep 27, 2011

Different implementation of the date/time manipulation functions so that those are no more limited to days and months intervals in PostgreSQL without impacting over DBMS (and can be improved later by developers with sufficient skills on the other DBMS)

This pull request should be associated with the one i sent on the doctrine main lib.

Different implementation of the date/time manipulation functions so t…
…hat those are no more limited to days and months intervals in PostgreSQL without impacting over DBMS (and can be improved later by developers with sufficient skills on the other DBMS)
lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
+ return 'DATE_ADD(' . $date . ', INTERVAL ' . $days . ' DAY)';
+ } else if ($unit == "month") {
+ return 'DATE_ADD(' . $date . ', INTERVAL ' . $months . ' MONTH)';
+ } else {
@stof

stof Sep 27, 2011

Member

please fix the indentation. You should use spaces, not tabs

lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
- public function getDateAddMonthExpression($date, $months)
- {
- return 'DATE_ADD(' . $date . ', INTERVAL ' . $months . ' MONTH)';
+ if ($unit == "day") {
@stof

stof Sep 27, 2011

Member

what is $unit ?

lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
+ if ($unit == "day") {
+ return 'DATE_SUB(' . $date . ', INTERVAL ' . $days . ' DAY)';
+ } else if ($unit == "month") {
+ return 'DATE_SUB(' . $date . ', INTERVAL ' . $months . ' MONTH)';
@stof

stof Sep 27, 2011

Member

same issues for this method

lib/Doctrine/DBAL/Platforms/OraclePlatform.php
- return '(' . $date . '-' . $days . ')';
+ if ($unit == "day") {
+ return '(' . $date . '+' . $days . ')';
+ } else if ($unit == "month") {
@stof

stof Sep 27, 2011

Member

same issues here. The indentation is wrong and $unit is undefined

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
- return "DATE(" . $date . ",'+". $months . " month')";
+ if ($unit == "day") {
+ return "DATE(" . $date . ",'+". $days . " day')";
+ } else if ($unit == "month") {
@stof

stof Sep 27, 2011

Member

same here

Member

stof commented Sep 27, 2011

Another point: DBAL is stable so the BC should be kept. So you should not remove the previous methods

cariboo commented Sep 28, 2011

I have made the corrections you requested : sorry for the tabs (bad config of my editor) and for the evil cut and paste (i have not other DBMS than PostgreSQL to test).
I have a question about the BC of DBAL : the functions i have removed are no more called and are in the platform specific implementations, not in the "public interface" of the library so they should not be called directly, should they ?

Member

stof commented Sep 28, 2011

DBAL is not only for the ORM. It is a separate project that may be used by other people outside the ORM. Also, the platforms are part of the public interface as they are the way to get the needed SQL to be cross-platform.

cariboo commented Sep 28, 2011

Ok, thanks for the explanation. I get back those functions right now.

Owner

beberlei commented Oct 28, 2011

Did you run the tests with this? I suppose they fail now.

Also what does this patch achieve? It seems to me really nothing by default. DBAL is about abstracting database differences, and I think only day and month can be done by all of them.

Member

asm89 commented Mar 11, 2012

Closing this as the common denominator between all DBAL supported RDMSes seems to be date function with day and month.

@asm89 asm89 closed this Mar 11, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment