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

Date older than 1970-01-01 returning as false #91

Open
sameer-shelavale opened this issue May 31, 2017 · 19 comments
Open

Date older than 1970-01-01 returning as false #91

sameer-shelavale opened this issue May 31, 2017 · 19 comments
Assignees

Comments

@sameer-shelavale
Copy link
Contributor

hi,
I am facing this strange problem,
I have a Date field in my OrientDB database which is storing a birthdates.
I am able to see the dates using OrientDB Browser.
When I execute a query using PHPOrient, those dates older than 1970-01-01 are returned as false. However for newer dates it returns the DateTime Object with correct value.
Am I missing something here?
I am using a latest dev-master and have tested with orientdb 2.1.9 and 2.2
Right now I am forced to use dob.format('yyyy-MM-dd')

@Ostico
Copy link
Owner

Ostico commented May 31, 2017

Hib @sameer-shelavale ,

can you send a code snippet to create/reproduce the issue?

@Ostico
Copy link
Owner

Ostico commented Jun 2, 2017

I tried on my side with OrientDb 2.2.0 and this script:
https://gist.github.com/Ostico/4187e07a0f85cb509c4cf98cd8de06fd
it works fine.

@andreyvk
Copy link

andreyvk commented Jun 14, 2017

@Ostico im facing the same problem. Basically what happened is that user manually entered a date in our system and format was not an acceptable one. From the Studio date showed as 31/12/0305 (clearly some parsing issues in our software). So PHPOrient library threw an exception on that record :

PHP Fatal error: Call to a member function setTimeZone() on a non-object in /home/ubuntu/shared/www/nsure/lib/vendor/ostico/phporient/src/PhpOrient/Protocols/Binary/Serialization/CSV.php on line 241

I've looked at the CSV.php code and debugged it. The line $collected = \DateTime::createFromFormat( 'U', substr( $collected, 0, -3 ) ); indeed returned false. Plz, take a look

@Ostico
Copy link
Owner

Ostico commented Jun 14, 2017

Hi @andreyvk ,
this is a bug clearly not related with the driver, anyway please perform a select in the OrientDB with its command line console and show me the internal format of the date field.

@andreyvk
Copy link

andreyvk commented Jun 14, 2017

Hi @Ostico ,

Here's the date field for that record as is:

orientdb {db=test}> select arrivalDate from MyClass where id='0ef5fd3c-7449-4ac0-9dfb-e4363df7a4d3'

+----+-------------------+
|#   |arrivalDate        |
+----+-------------------+
|0   |0305-12-31 00:00:00|
+----+-------------------+

You could be right though. It might be serialization problem on the DB side. In which case we'll just forward this issue to the OrientDB team.

@andreyvk
Copy link

andreyvk commented Jul 6, 2017

@Ostico

Have you had a chance to look at the problem? We are having more issues now with clients because of this.

Btw, here's how to reproduce it (we are using Orient 2.2.19):

create class Test extends V
create property Test.dated date
insert into Test content {"dated": "1969-12-31"}

and then

try {
    $client = new \PhpOrient\PhpOrient( 'localhost', 2424 );
    $client->username = 'admin';
    $client->password = 'admin';
    $client->connect();
    $client->dbOpen( 'test', 'admin', 'admin' );

    $records = $client->query("select from Test");

    //output (DOES NOT GET TO THIS LINE)
    print_r($records);
}
catch(\Exception $e) {
    print_r($e->getMessage());
}

throws exception and prints

PHP Fatal error:  Call to a member function setTimeZone() on a non-object in /home/ubuntu/shared/www/nsure/lib/vendor/ostico/phporient/src/PhpOrient/Protocols/Binary/Serialization/CSV.php on line 241

Please fix this asap if possible. I hope this helps. Thanks in advance!

@andreyvk
Copy link

andreyvk commented Jul 6, 2017

@Ostico I found smth interesting abt DateTime::createFromFormat(...). It indeed has problems converting negative time into date (see https://stackoverflow.com/questions/23103451/php-how-to-create-date-before-the-epoch-1970-using-date-instead-of-datetime). Moreover, implementation using substr( $collected, 0, -3 ) returns false when $collected is 0 (or less than 3 characters long for that matter). I'd suggest the following implementation instead:

//starting phporient/src/PhpOrient/Protocols/Binary/Serialization/CSV.php line 239
$dt = new \DateTimeZone( date_default_timezone_get() );

//instead of substr( $collected, 0, -3 );
$ts = (int) ($collected / 1000); 

//instead of \DateTime::createFromFormat( 'U', substr( $collected, 0, -3 ) );
$collected = new \DateTime();
$collected->setTimestamp($ts);
$collected->setTimeZone( $dt );

Plz let me know what u think

@andreyvk
Copy link

hi @Ostico,

Have you had any chance to look at this?

@Ostico
Copy link
Owner

Ostico commented Jul 18, 2017

Hi @andreyvk,

I can't do anything at moment because i've no internet connection nor pc.
I'm really sorry about that, but i think there is to wait 2 or 3 weeks more.

@Ostico Ostico self-assigned this Jul 18, 2017
@andreyvk
Copy link

@Ostico cool. I'll hang on until that. Thanks!

@andreyvk
Copy link

@Ostico, hi. Are you back now? Can you please take a look at the issue?

@Ostico
Copy link
Owner

Ostico commented Aug 22, 2017

Hi @andreyvk , @sameer-shelavale
i tried to reproduce this bug again and i've not found any issue, it works fine for me.
@andreyvk , i tried your code on OrientDB version 2.2.19 and 2.2.25 with PHP 7.0.22 and i got this result:

Array
(
    [0] => PhpOrient\Protocols\Binary\Data\Record Object
        (
            [rid:protected] => PhpOrient\Protocols\Binary\Data\ID Object
                (
                    [cluster] => 17
                    [position] => 0
                )

            [oClass:protected] => Test
            [version:protected] => 1
            [oData:protected] => Array
                (
                    [dated] => DateTime Object
                        (
                            [date] => 1969-12-30 23:00:00.000000
                            [timezone_type] => 3
                            [timezone] => UTC
                        )

                )

        )

)

@Ostico
Copy link
Owner

Ostico commented Aug 23, 2017

@andreyvk regarding this:
#91 (comment)

i have to check better how the dates before the unix epoch are returned by OrientDB because i can use your suggested implementation only for 64bit platforms.

Anyway, for me this implementation works as is... I never got this issue. Are you sure that the field is DateTime in OrientDB?

Even if your software permit the wrong insertion in the database, OrientDB should not accept this kind of wrong dates.

@andreyvk
Copy link

@Ostico, orientdb cannot and should not prohibit storing any dates technically. what if database is about some historical values, for instance? dates can be old, right? :)

I am not sure abt 32-bit systems. But all i know is that PHP 5.5 has the issue and it's valid (and that is the version we are currently running on production server). You can try that on http://phptester.net by setting PHP version to 5.5 and running this code there:

$datetime = \DateTime::createFromFormat('U', "-1");
var_dump($datetime);

Also me and the other guy too are sure that Orient has problems with dates below 1970 and the field is DATETIME or DATE. That's a valid issue.

@Ostico
Copy link
Owner

Ostico commented Aug 28, 2017

Ok, i will try with an older php version

@sameer-shelavale
Copy link
Contributor Author

sameer-shelavale commented Aug 30, 2017

@Ostico I have windows 7 64bit and PHP 5.5.33 and this issue is still haunts me.
The solution by @andreyvk of using setTimestamp() instead of createFromFormat() works.

I have date of birth of a user as 1953-04-11 00:00:00 which translates to
$collected = "-527853600000";
on phporient/src/PhpOrient/Protocols/Binary/Serialization/CSV.php line 240
but createFromFormat() returns false for negative timestamps.

@Ostico
Copy link
Owner

Ostico commented Aug 30, 2017

@andreyvk i checked, this fails until php 5.6.

@sameer-shelavale the question for me is not the DateTime method used to build the correct date but the implementation needed to handle the timestamps:

//instead of substr( $collected, 0, -3 );
$ts = (int) ($collected / 1000); 

The explicit cast to integer doesn't works on 32bit if the timestamp is lower than -2147483648 so it's better to use substr, it works for all PHP versions.

Moreover this will not work on 32bit systems:

$collected->setTimestamp($ts);

because of the internal limitation of integers, setTimestamp will fail also.

I must to do a double implementation by checking the PHP version.

@andreyvk
Copy link

andreyvk commented Aug 30, 2017

@Ostico, you can probably use the following approach (need to test on all PHP versions though, but i think it works):

$time = '-12345';
$datetime = new DateTime("@$time");

However, the use of substr( $collected, 0, -3 ) in your code might return false (or null ?), if $collected is less than 3 characters long. Also, if $collected, for example, is "-100", then substr will return "-". In which case this DateTime("@$time") fails. So, maybe smth like this will work:

$collected = substr( $collected, 0, -3 );
if(empty($collected) || $collected === "-")
    $collected = "0";
$datetime = new DateTime("@$collected");

How about that?

@sameer-shelavale
Copy link
Contributor Author

@Ostico I think casting to int is not really necessary.
I tried
$collected->setTimestamp( '-527853600' );
and it works well.

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

No branches or pull requests

3 participants