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

Database scaffolding produces empty method call and no ValueGeneratedOn*() call when using datetime/timestamp columns #877

Closed
Slushnas opened this issue Oct 16, 2019 · 4 comments · Fixed by #896
Assignees
Milestone

Comments

@Slushnas
Copy link

Steps to reproduce

Create the table:

create table TimestampTable(
    Id int not null,
    Ts timestamp default '0000-00-00 00:00:00' on update CURRENT_TIMESTAMP,
    Dt datetime default '0000-00-00 00:00:00' on update CURRENT_TIMESTAMP,
    primary key (Id)
);

Run the scaffolder:

dotnet ef dbcontext scaffold 'ConnectionString' Pomelo.EntityFrameworkCore.MySql -o Test

The issue

The scaffolder produces the following:

protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<TimestampTable>(entity =>
            {
                entity.Property(e => e.Id).HasColumnType("int(11)");

                entity.Property(e => e.Dt)
                    .HasColumnType("datetime")
                    .HasDefaultValueSql("'0000-00-00 00:00:00'")
                    .();

                entity.Property(e => e.Ts)
                    .HasColumnType("timestamp")
                    .HasDefaultValueSql("'0000-00-00 00:00:00'")
                    .();
            });

            OnModelCreatingPartial(modelBuilder);
        }

        partial void OnModelCreatingPartial(ModelBuilder modelBuilder);
}

Issue 1: The default value strings have added single quotes which may be a problem but that behavior is already detailed in Issue #703.

Issue 2: The datetime and timestamp properties have an empty method call .() appended which causes build errors.

Issue 3: I would expect the scaffolder to append .ValueGeneratedOnUpdate() or .ValueGeneratedOnAddOrUpdate() to the property definition based on the syntax here: https://dev.mysql.com/doc/refman/8.0/en/timestamp-initialization.html. Perhaps there is a bug with this functionality that is causing issue 2?

Also - I'm not sure if the OnModelCreatingPartial() method is supposed to exist here or not as it doesn't appear to be used.

Further technical details

MySQL version: 10.1.41-MariaDB-0ubuntu0.18.04.1
Operating system: Ubuntu 18.04
Pomelo.EntityFrameworkCore.MySql version: 3.0.0-rc1.final
Microsoft.AspNetCore.App version: 3.0

@lauxjpn lauxjpn added this to the 3.0.0 milestone Oct 16, 2019
@lauxjpn
Copy link
Collaborator

lauxjpn commented Oct 16, 2019

Issue 2 & 3 are likely to be related.

Also - I'm not sure if the OnModelCreatingPartial() method is supposed to exist here or not as it doesn't appear to be used.

The point of this method is, that you can implement it in a partial class. If you implement it, it will be called as part of OnModelCreating(). Otherwise, it will not be called.

@Slushnas
Copy link
Author

Thanks, I had wondered if it was there as a placeholder in case it was needed.

@lauxjpn
Copy link
Collaborator

lauxjpn commented Oct 24, 2019

Issue 1 is not an issue, but the correct behavior. Issue #703 is just a special case, where single quotes are invalid.

For issue 2/3, I opened dotnet/efcore#18579 upstream, as this seems to be a bug in EF Core.

In your specific case, we can actually solve this by safely applying ValueGenerated.OnAddOrUpdate (because you also specify a default value), which will be handled by EF Core.

lauxjpn added a commit to lauxjpn/Pomelo.EntityFrameworkCore.MySql that referenced this issue Oct 24, 2019
Introduce a workaround for the missing EF Core handling of `ValueGenerated.OnUpdate`.
Fixes PomeloFoundation#877
lauxjpn added a commit that referenced this issue Oct 27, 2019
* Add support to reverse engineer views.
Add comments for tables and columns.
Improve handling of default values.

* Handle the `CURRENT_TIMESTAMP` default value for `timestamp` columns correctly.
Fixes #703

* Correctly implement `CURRENT_TIMESTAMP` with `ON UPDATE` clauses.
Introduce a workaround for the missing EF Core handling of `ValueGenerated.OnUpdate`.
Fixes #877

* Remove unnecessary code

Can probably remove this code as it only applies to Release Candidate not General Availability versions.

https://bugs.mysql.com/bug.php?id=89793

>The NON_UNIQUE column in the INFORMATION_SCHEMA.STATISTICS table had
type BIGINT prior to MySQL 8.0, but became VARCHAR in MySQL 8.0 with
the introduction of the data dictionary. The NON_UNIQUE column now
has an integer type again (INT because the column need not be as
large as BIGINT).

* Correctly map `unsigned` database types with precision, scale, size or display width to CLR types.

* Fix table/view determination.

* Support views in `MySqlDatabaseCleaner`.

* Fix some Timestamp/RowVersion issues.
Still depends on dotnet/efcore#18592
Addresses #792
@mguinness
Copy link
Collaborator

The upstream bug is fixed and will be in the 3.1 release.

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

Successfully merging a pull request may close this issue.

3 participants