Doctrine 1

Can not set a default value of 'CURRENT_TIMESTAMP' in a table definition for mysql

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.2.3
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    XAMP Windows

Description

Hi

I recently I discovered that I needed o put a default value of 'CURRENT_TIMESTAMP' on some of my fields. This was breaking the build of my mysql base for 2 different reasons – for one columns with a type of timestamp were being switched instead to have a type of datetime, and for two the words: CURRENT_TIMESTAMP were being quote encapsulated in the create table statement.

I looked around a bit and couldn't find a solution so I made a patch to fix the issue. This patch however broke some test cases that were expecting datetimes to be returned instead of timestamps (so i fixed the tests).

I may be breaking something that I am not aware of by doing this, or perhaps there was another solution to the problem that I could not find – if any one has any input on this I would appreciate it.

I will post my patch in this thread but it is worth mention that it contains several bug fixes and a few new features which I have posted in other threads several months ago but never heard back regarding (most notably, beyond bug fixes it introduces a new hydration type and allows the disabling of the some times useful some times problematic limit subquery feature of doctrine).

Thanks to all who have any input.

Will Ferrer

  1. Text File
    DC_847_fix_BC.patch
    11/Sep/10 7:12 PM
    5 kB
    will ferrer
  2. Text File
    DC_847_fix.patch
    02/Sep/10 7:16 PM
    5 kB
    will ferrer

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
will ferrer added a comment - - edited

Here is the patch

Show
will ferrer added a comment - - edited Here is the patch
Hide
Permalink
will ferrer added a comment -

Found a small error in my last patch (some debug code I hadn't removed) and added some more comments (links to jira above each change) to clarify the fixes and new features the patch has in it.

Show
will ferrer added a comment - Found a small error in my last patch (some debug code I hadn't removed) and added some more comments (links to jira above each change) to clarify the fixes and new features the patch has in it.
Hide
Permalink
will ferrer added a comment -

individual patch for this issue with out other features included

Show
will ferrer added a comment - individual patch for this issue with out other features included
Hide
Permalink
Jonathan H. Wage added a comment -

Hi, this one breaks the tests and backwards compatibility. Previously if you have a timestamp field in Doctrine, it would create a datetime column in mysql. Now it is creating a timestamp column. I don't think we can make this change in a stable version.

Show
Jonathan H. Wage added a comment - Hi, this one breaks the tests and backwards compatibility. Previously if you have a timestamp field in Doctrine, it would create a datetime column in mysql. Now it is creating a timestamp column. I don't think we can make this change in a stable version.
Hide
Permalink
Jonathan H. Wage added a comment -

At any rate, it is breaking the tests still. If we do decide to make the change, can you run all the tests and fix any of the other failures?

Show
Jonathan H. Wage added a comment - At any rate, it is breaking the tests still. If we do decide to make the change, can you run all the tests and fix any of the other failures?
Hide
Permalink
will ferrer added a comment - - edited

Hi Jon

I added more test case fixes to the patch. I can see how this would still cause backwards compatibility issues though – any one who built their db using the none patched version of doctrine would end up with different field types if they rebuilt using the patched version and this could affect the functionality of their existing code.

Thanks for the help .

Hope you are well.

Will

Show
will ferrer added a comment - - edited Hi Jon I added more test case fixes to the patch. I can see how this would still cause backwards compatibility issues though – any one who built their db using the none patched version of doctrine would end up with different field types if they rebuilt using the patched version and this could affect the functionality of their existing code. Thanks for the help . Hope you are well. Will
Hide
Permalink
Jonathan H. Wage added a comment -

Can you think of anyway it could be tweaked so that it is BC?

Show
Jonathan H. Wage added a comment - Can you think of anyway it could be tweaked so that it is BC?
Hide
Permalink
will ferrer added a comment -

Good question...

Here is an idea – how about I put a flag in the options for the field that changes the behavior. So if some one wants to use real timestamps instead of datetime fields they could set an option of: "useRealTimestamps : true" on the field.

I think that would be a good solution because by default everything would work as does is now (providing BC) but users could switch it over if they required the additional functionality that timestamps provide.

What do you think?

Will

Show
will ferrer added a comment - Good question... Here is an idea – how about I put a flag in the options for the field that changes the behavior. So if some one wants to use real timestamps instead of datetime fields they could set an option of: "useRealTimestamps : true" on the field. I think that would be a good solution because by default everything would work as does is now (providing BC) but users could switch it over if they required the additional functionality that timestamps provide. What do you think? Will
Hide
Permalink
will ferrer added a comment -

Hi Jon

Here is a backwards compatible patch using the method I proposed in my last comment.

To make a timestamp field use a type of timestamp (instead of datatime) include an option of: userealtimestamp=>true

Let me know if you think this method will work out.

Hope you are well.

Will

Show
will ferrer added a comment - Hi Jon Here is a backwards compatible patch using the method I proposed in my last comment. To make a timestamp field use a type of timestamp (instead of datatime) include an option of: userealtimestamp=>true Let me know if you think this method will work out. Hope you are well. Will
Hide
Permalink
will ferrer added a comment -

I found some issues with this patch when I moved it off my local machine our production server – there was a case sensitivity issue that I have since resolved. Here is the fixed version.

Show
will ferrer added a comment - I found some issues with this patch when I moved it off my local machine our production server – there was a case sensitivity issue that I have since resolved. Here is the fixed version.

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    will ferrer
Vote (0)
Watch (0)

Dates

  • Created:
    Updated: