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

[NFR] Datetime column type #524

Closed
SliceOfLife opened this Issue Apr 5, 2013 · 23 comments

Comments

@SliceOfLife
Copy link

SliceOfLife commented Apr 5, 2013

What about adding "datetime" column type for Model? By default the "@var \Datetime" converted to string.

@niden

This comment has been minimized.

Copy link
Member

niden commented Apr 5, 2013

@SliceOfLife I am not sure I understand what you mean. Could you please explain with an example what you are trying to do ?

@SliceOfLife

This comment has been minimized.

Copy link
Author

SliceOfLife commented Apr 5, 2013

For example, I have a Mysql DB and table with datetime column.
I want to write the Model class with \DateTime field:

class SomeModel extends \Phalcon\Mvc\Model 
{
    /**
     * @Column(type="datetime", nullable=false)
     *
     * @var \DateTime
     */
    public $datetime_field;

    public function initialize()
    {
        $this->datetime_field = new \DateTime();
    }
}

But I can't use the "datetime" column type, it's very sad. This code throws error like "Object of class DateTime������w� could not be converted to string".

@niden

This comment has been minimized.

Copy link
Member

niden commented Apr 5, 2013

Hmmm for some reason my comment disappeared.

Either way.

This is not a blocker and the desired functionality can be achieved with a minor change in the code. We can add it as a NFR but it will take a low priority one and we also need to test the impact that this will have on the framework as far as performance is concerned.

You can achieve what you need with:

$now = new \DateTime();
$this->datetime_field = $now->format('Y-m-d H:i:s');

or without using datetime

$this->datetime_field = date('Y-m-d H:i:s');
@SliceOfLife

This comment has been minimized.

Copy link
Author

SliceOfLife commented Apr 5, 2013

It will be a cool feature.
By the way, I use an integer timestamp and Mysql INT(11) type for now.

@SliceOfLife SliceOfLife closed this Apr 5, 2013

@SliceOfLife SliceOfLife reopened this Apr 5, 2013

@niden

This comment has been minimized.

Copy link
Member

niden commented Apr 5, 2013

I follow the same practice too (using integers). Much faster to search/index, you can perform easy math operations with them (to find differences etc.) and for me at least it is a lot more logical. You also escape the different formatting conventions i.e. m/d/y, y.m.d etc.

@dalu

This comment has been minimized.

Copy link
Contributor

dalu commented Apr 14, 2013

hmm what about the year 2038 problem on 32bit machines?

@niden

This comment has been minimized.

Copy link
Member

niden commented Apr 14, 2013

@dalu I haven't personally used a 32bit machine in a few years and are seeing them disappear. However, the easy solution on that would be again numeric fields. You will just need to have two per date (if you want to store hours/minutes/seconds)

For the date I would store this:

20130414 (which is 2013-04-14)

and for the time the same thing

175432 (which is 17:54:32)

This way you have no issues with 32 bit machines. :)

Having said that, with the way technology is going, I doubt we will have any 32bit machines by that year.

@niden

This comment has been minimized.

Copy link
Member

niden commented Apr 16, 2013

Closing this as we will not be implementing automatic Datetime fields in Phalcon in the very near future. Perhaps in 2.0+

@niden niden closed this Apr 16, 2013

@lfbittencourt

This comment has been minimized.

Copy link

lfbittencourt commented Dec 5, 2014

+1 for this!

@tmihalik

This comment has been minimized.

Copy link
Contributor

tmihalik commented Jan 14, 2015

+1

1 similar comment
@AndreBaumeier

This comment has been minimized.

Copy link

AndreBaumeier commented Mar 26, 2015

+1

@loopsframework

This comment has been minimized.

Copy link
Contributor

loopsframework commented Jun 9, 2015

+1
I would welcome it if Phalcon translates the DateTime object to the format expected by the database backend. With this layer of abstraction, code should become more portable.

@f3l1x

This comment has been minimized.

Copy link

f3l1x commented Oct 6, 2015

👍

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Oct 6, 2015

@niden

Closing this as we will not be implementing automatic Datetime fields in Phalcon in the very near future. Perhaps in 2.0+

Now is 2.0+ :)

@MLukman

This comment has been minimized.

Copy link
Contributor

MLukman commented Nov 14, 2015

I vote for this feature implemented about now.

@StudioMaX

This comment has been minimized.

Copy link

StudioMaX commented Mar 10, 2016

+1

@reloaded

This comment has been minimized.

Copy link

reloaded commented Mar 5, 2017

+1

@sergeyklay sergeyklay added this to the 4.0.0 milestone Mar 8, 2017

@efimBistrov

This comment has been minimized.

Copy link

efimBistrov commented Apr 25, 2017

+1

@kornerita

This comment has been minimized.

Copy link

kornerita commented Aug 16, 2017

Hi,

I've found this thread and I would like to add the following:

  • Regarding the performance impact, there is already a "castOnHydrate" option to disable if you don't need it or if you want performance (by default it comes disabled).
  • For this implementation there should be methods on the Phalcon\Db\Dialect to establish the default date format returned by the engine for each date type (i.e. date or timestamp) and methods to transform the result from those to a \DateTime object. Note: I think here is the actual casting.

My personal opinion about this, is that an ORM should abstract you on the date format the database engine is using and should use native language date manipulations. This adds transparency when you want or need to change a database engine.

Regards,
Federico.

@sergeyklay sergeyklay self-assigned this Aug 18, 2017

@stale

This comment has been minimized.

Copy link

stale bot commented Apr 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Apr 16, 2018

@StudioMaX

This comment has been minimized.

Copy link

StudioMaX commented Apr 16, 2018

This is still relevant.

@stale stale bot removed the stale label Apr 16, 2018

@dalu

This comment has been minimized.

Copy link
Contributor

dalu commented Apr 17, 2018

Glad I moved on to Go seeing this. 5 years later and still no resolution lol

@niden niden referenced this issue Oct 31, 2018

Merged

[#13543] Add more pdo types #13562

3 of 3 tasks complete
@niden

This comment has been minimized.

Copy link
Member

niden commented Oct 31, 2018

@niden niden closed this Oct 31, 2018

@niden niden referenced this issue Oct 31, 2018

Open

Update 4.x Documents #1935

0 of 10 tasks complete

@niden niden added this to Done in 4.0 Release Dec 7, 2018

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