php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74013 DateTime not able to handle DateInterval on february
Submitted: 2017-01-30 11:23 UTC Modified: 2017-02-01 09:55 UTC
From: etienne dot chabert at gmail dot com Assigned:
Status: Not a bug Package: date_time (PECL)
PHP Version: 7.0.15 OS: OSX
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: etienne dot chabert at gmail dot com
New email:
PHP Version: OS:

 

 [2017-01-30 11:23 UTC] etienne dot chabert at gmail dot com
Description:
------------
This ticket is related to : https://bugs.php.net/bug.php?id=73628 but your ticket system didn't allow me to change the state of the ticket to "Re-Opened" -> (You aren't allowed to change a bug to that state.)

-------------------------------------------------------------------------------------

Well, I don't know how to be more clear about this bug, I gave you 2 cases where the bug occurred plus a test script to check if your patch fixed it or not, but I will do my best to be more specific because, yes I am still concerned by this issue :

Actually this is how DateTime / DateInterval are handling tricky February month and the result is quite dangerous for people who are doing date comparison.

So what I expect to be a normal behavior for this tricky case ? Well, I don't know but no going back to the past would be a good evolution ! Not having a 'true' result on : (2017-01-31 + 1 Month) > (2017-02-01 + 1 Month) would be nice too isn't it ? 

Best regards,


Test script:
---------------
if ((new DateTime("2016-11-30"))->add(new DateInterval("P3M")) > (new DateTime("2016-12-01"))->add(new DateInterval("P3M")))
  throw new Exception("WTF");
echo "You fixed it";


Expected result:
----------------
(new DateTime("2017-01-28"))->add(new DateInterval("P1M"))->format('Y-m-d') // expected result -> 2017-02-28
(new DateTime("2017-01-29"))->add(new DateInterval("P1M"))->format('Y-m-d') // expected result -> 2017-03-01
(new DateTime("2017-01-30"))->add(new DateInterval("P1M"))->format('Y-m-d') // expected result -> 2017-03-01
(new DateTime("2017-01-31"))->add(new DateInterval("P1M"))->format('Y-m-d') // expected result -> 2017-03-01
(new DateTime("2017-02-01"))->add(new DateInterval("P1M"))->format('Y-m-d') // expected result -> 2017-03-01

Actual result:
--------------
(new DateTime("2017-01-28"))->add(new DateInterval("P1M"))->format('Y-m-d') // actual result -> 2017-02-28
(new DateTime("2017-01-29"))->add(new DateInterval("P1M"))->format('Y-m-d') // actual result -> 2017-03-01
(new DateTime("2017-01-30"))->add(new DateInterval("P1M"))->format('Y-m-d') // actual result -> 2017-03-02     <<<<<<<<<<<<<<<<<<< BIG PROBLEM HERE ???
(new DateTime("2017-01-31"))->add(new DateInterval("P1M"))->format('Y-m-d') // actual result -> 2017-03-03     <<<<<<<<<<<<<<<<<<< BIG PROBLEM HERE ???
(new DateTime("2017-02-01"))->add(new DateInterval("P1M"))->format('Y-m-d') // actual result -> 2017-03-01     


Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-01-30 13:20 UTC] etienne dot chabert at gmail dot com
This is the code I actually using to detect / fix this kind of "dates jump" while waiting for an official patch :  

$dateCheck = new DateTime($date->format('Y-m-01'));
$date->add(new DateInterval($relative));
$dateCheck->add(new DateInterval($relative));
if ($date->format('m') > $dateCheck->format('m'))
   $date->setDate($date->format('Y'), $date->format('m'), $date->format('01'));

Hope it helps even if it's definitely the dumb way to fix it.
 [2017-01-30 13:24 UTC] etienne dot chabert at gmail dot com
There was a little mistake in my previous code (it was working with it)

$date = new DateTime($fromDate);
$dateCheck = new DateTime($date->format('Y-m-01'));
$date->add(new DateInterval($relative));
$dateCheck->add(new DateInterval($relative));
if ($date->format('m') > $dateCheck->format('m'))
  $date->setDate($date->format('Y'), $date->format('m'), '01');
 [2017-01-30 15:00 UTC] andrew dot nester dot dev at gmail dot com
I've just added PR to fix this, but this change definitely requires some discussion and  some conclusion.
 [2017-01-30 15:30 UTC] etienne dot chabert at gmail dot com
Nice patch, even I am still questioning myself about the right answer in such case

(new DateTime("2017-01-29"))->add(new DateInterval("P1M"))->format('Y-m-d') // Should return, '2017-02-28' or '2017-03-01'

I think there is no 'Good' answer to this, and we should probably try to stick on how other popular languages deal with it (JS, Ruby, etc.)
 [2017-01-30 15:48 UTC] andrew dot nester dot dev at gmail dot com
I've implemented it like it's working in Python (and other languages too)
Your expression should return '2017-02-28'
 [2017-01-31 13:23 UTC] derick@php.net
-Status: Open +Status: Not a bug
 [2017-01-31 13:23 UTC] derick@php.net
There is no bug here. This is the expected behaviour. Nearly a decade ago who picked this way over other ways, as an analogy to the gnu date functions.

We can not change this now due to BC reasons, even if we wanted this.
 [2017-02-01 09:55 UTC] etienne dot chabert at gmail dot com
Saying that having a condition returning 'true' with the following code is not a bug sound very weird to me : 
((new DateTime("2016-01-31"))->add(new DateInterval("P1M")) > (new DateTime("2016-02-01"))->add(new DateInterval("P1M")))

But I understand your point for BC issues.

A "good" solution now could be to add a really BIG warning in the documentation of DateTime / DateInterval about this, with an official piece of code to overload the default behaviour.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Sat Apr 20 04:01:25 2019 UTC