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

DateTimeImmutable error with default value CURRENT_TIMESTAMP #2859

Closed
tok-88 opened this issue Sep 12, 2017 · 12 comments
Closed

DateTimeImmutable error with default value CURRENT_TIMESTAMP #2859

tok-88 opened this issue Sep 12, 2017 · 12 comments

Comments

@tok-88
Copy link
Contributor

tok-88 commented Sep 12, 2017

I have just changed all my DateTime to DateTimeImmutable and I experienced an issue with the ones where I had a CURRENT_TIMESTAMP default value:

 @ORM\Column(type="datetime_immutable", options={"default"="CURRENT_TIMESTAMP"})

The issue occurs when I use the schema-tool to update my database and I get the following error:

'ALTER TABLE accounts CHANGE created_on created_on DATETIME DEFAULT 'CURRENT_TIMESTAMP' NOT NULL'

So I learned that the schema-tool tries to execute a query with a string as default value, which is the reason for the error. If I use the DateTime type instead, it will execute it as expected.
I went digging and I found the method 'getDefaultValueDeclarationSQL' in the AbstractPlatform class, where I found a solution for the issue.

public function getDefaultValueDeclarationSQL($field)
{
    $default = empty($field['notnull']) ? ' DEFAULT NULL' : '';

    if (isset($field['default'])) {
        $default = " DEFAULT '".$field['default']."'";
        if (isset($field['type'])) {
            if (in_array((string) $field['type'], array("Integer", "BigInt", "SmallInt"))) {
                $default = " DEFAULT ".$field['default'];
            } elseif (in_array((string) $field['type'], array('DateTime', 'DateTimeTz')) && $field['default'] == $this->getCurrentTimestampSQL()) {
                $default = " DEFAULT ".$this->getCurrentTimestampSQL();
            } elseif ((string) $field['type'] == 'Time' && $field['default'] == $this->getCurrentTimeSQL()) {
                $default = " DEFAULT ".$this->getCurrentTimeSQL();
            } elseif ((string) $field['type'] == 'Date' && $field['default'] == $this->getCurrentDateSQL()) {
                $default = " DEFAULT ".$this->getCurrentDateSQL();
            } elseif ((string) $field['type'] == 'Boolean') {
                $default = " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
            }
        }
    }
    return $default;
}

And if I add 'DateTimeImmutable' to the following line:

} elseif (in_array((string) $field['type'], array('DateTime', 'DateTimeTz', 'DateTimeImmutable')) && $field['default'] == $this->getCurrentTimestampSQL()) {
    $default = " DEFAULT ".$this->getCurrentTimestampSQL();
}

It works as intended. Any chance this can be implemented? I haven't seen any documentation about this being intentional, so I figured it wasn't.

@Ocramius
Copy link
Member

Duplicate of doctrine/orm#6698 ?

@tok-88
Copy link
Contributor Author

tok-88 commented Sep 12, 2017

I don't believe so no. This is only an issue with the 'newly' released DateTimeImmutable and works fine with DateTime.

EDIT:
It seems like the solution might work for me as well, so I'll just put this on hold untill it has been released 👍

@Majkl578
Copy link
Contributor

This should be fixed by #2835, but we should consider a separate fix for 2.6 since that PR can't go to 2.6,...

@tok-88
Copy link
Contributor Author

tok-88 commented Sep 13, 2017

It has been fixed by #2835, but since I have no timeline for when that PR will be released, a fix for 2.6 would be welcomed with open arms from my part.

@Ocramius
Copy link
Member

The fact that #2835 fixes this as a side-effect is not OK for this patch to go through: needs a dedicated test and an ad-hoc fix for 2.6 if you need it there.

@Ocramius
Copy link
Member

And yep, still using (string) casts ;-)

@tok-88
Copy link
Contributor Author

tok-88 commented Sep 13, 2017

I gave it a go at #2861 with (string) casts ;-)
I also added DateImmutable, but since there was no test case for Date, I didn't figure DateImmutable needed one.

@tok-88 tok-88 changed the title DateTimeImmutable error with default value CURRENT_TIMESTAMP (solution inside) DateTimeImmutable error with default value CURRENT_TIMESTAMP Sep 15, 2017
@lcobucci lcobucci self-assigned this Nov 18, 2017
@lcobucci lcobucci added the Bug label Nov 18, 2017
@lcobucci lcobucci added this to the 2.6.3 milestone Nov 18, 2017
@spacetraveller
Copy link

spacetraveller commented May 12, 2018

Thanks for breaking the ability to have the option of having a CURRENT_TIMESTAMP set to default. We use Symfony to handle migrations and have other technologies which user the DB, and now due to this "fix" (or downgrade), I'm unable to set a default timestamp into the schema. Not happy about this "fix" at all, why was it removed ? Makes no sense and is a BAD decision, giving LESS flexibility to us developers.

@Majkl578
Copy link
Contributor

@spacetraveller Please open new issue for your problem referencing this one. If possible, also provide a test case for the broken use case. Thanks.

@spacetraveller
Copy link

Hey Majk578, please see #3142 for the detaiils of the issue.

@Ocramius
Copy link
Member

Reminder to not use default with mapped fields, as it breaks the symmetry between written and read data.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants