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

Better support for automatic row versions using the xmin column #8

Open
roji opened this issue May 12, 2016 · 21 comments
Open

Better support for automatic row versions using the xmin column #8

roji opened this issue May 12, 2016 · 21 comments

Comments

@roji
Copy link
Member

roji commented May 12, 2016

From @cremor on November 5, 2014 6:47

The default entity versioning scheme of Entity Framework when used with SQL server is to use a column with the data type 'rowversion'. This column type make sure that each update of the row automatically gets a new version. As far as I know, there is nothing similar in PostgreSQL. Of course we could create triggers for all tables, but some users seem to use the xmin system column to avoid that. Sample mapping:

Property(x => x.Version)
   .HasColumnName("xmin")
   .IsConcurrencyToken()
   .HasDatabaseGeneratedOption(DatabaseGeneratedOption.Computed);

This works fine (at least as far as my few tests show), but only if you don't want to use code first or migrations because then EF tries to create a user column named "xmin" which of course fails. My suggestion here is to ignore columns named "xmin" in DDL generation when it is mapped as a computed concurrency token.

Another problem is the type of the xmin column. I couldn't find something specific about it in the PostgreSQL documentation, but according to a Stack Overflow answer it is an unsigned 32 bit integer. This is a problem because (1) EF doesn't support unsigned properties and (2) if you try to map a (signed) 64 bit integer property Npgsql adds a cast to the SQL which is rejected by PostgreSQL. I have no idea if there is a way to get unsigned properties working without changes in EF itself but if not Npgsql shouldn't add those casts to the SQL if a 64 bit integer is used for the version property.

Copied from original issue: npgsql/npgsql#411

@roji
Copy link
Member Author

roji commented May 12, 2016

From @margusbirk on January 31, 2016 17:21

+1
When using code first and migrations a manual work-around would be to include the xmin column in the model, generate the migrations and then remove the part that creates the column from the migration xmin = c.Int(nullable: false).
It's possible to insert data and it seems to work, but when trying to retrieve data then the cast from the Postgresql type xid fails (tested with int and long).

Are there any alternatives other than managing update triggers?

@roji
Copy link
Member Author

roji commented May 12, 2016

Sorry for taking so long to reply, and thanks for this - I wasn't aware of the usage of xmin as an automatically-updated rowversion.

In Entity Framework Core there's the concept of shadow properties, which seems perfect for this - I've opened #1016 for that.

For EF6, ignoring xmin in DDL generation seems OK (and is probably easy to do), but I don't know enough about possible unsigned support and probably won't have much time to dive into it... Am marking this up for grabs for now.

@cremor
Copy link

cremor commented Jun 30, 2016

Was something changed with the handling of the xmin column in Npgsql 3.x or EntityFramework6.Npgsql 3.x? Because my sample mapping code works fine with version 2.2.7, but doesn't work with the current version 3.1.5/3.1.0. I'm getting an exception that says xid can't be cast to Int32, just like you reported in your first comment in this issue.

I'd really like to update a project to the current version of Npgsql, but I'd like to avoid creating all that triggers for versioning.

@roji
Copy link
Member Author

roji commented Jul 1, 2016

@cremor am not sure if your question is EF6 related or just Npgsql in general, but xid is a uint rather than an int; can you give that a try?

@cremor
Copy link

cremor commented Jul 1, 2016

As far as I know, EF6 still doesn't support unsigned data types. Or has this changed? At least EF6 throws an exception that says something about supported primitive types when I try to map my version as uint.

My problem is with Npgsql or EntityFramework6.Npgsql. Using Npgsql 2.2.7 and Npgsql.EntityFramework 2.2.7 the mapping code shown in the first post of this issue works when used with an int version property. Updating to Npgsql 3.1.5 and EntityFramework6.Npgsql 3.1.0 breaks it.

@roji
Copy link
Member Author

roji commented Jul 1, 2016

Rereading everything, this is what this issue is about. I'm not sure anymore exactly how things worked in 2.2.7, but Npgsql 3.x maps xid to its proper value type, which is uint.

One possible fix here would be to relax things and allow Npgsql to return xid (and other unsigned PostgreSQL types?) as int. However, this means changing things at the Npgsql level because a higher-level (EF6) isn't competent enough to deal with reality, I'd rather find another solution if at all possible.

If someone has any idea, please feel free to suggest.

@cremor
Copy link

cremor commented Jul 1, 2016

I initially created the issue because DDL generated by EF was wrong since it contained the xmin column. But back then everything else worked fine. Maybe it would make sense to split the issue which now happens in Npgsql 3.x to a new Github issue so that this one could stay clean for the DDL improvement?

About your suggestion: I don't know if EF Core will (or already does) support unsigned data types. But EF6 won't support them ever. So I think there needs to be special handling for EF6 (and older) in Npgsql.

@omatrot
Copy link

omatrot commented Jul 13, 2016

@cremor, @roji
I agree with you, and I've done that in a customized version of the old provider to meet my needs with EF5/Edmx ;-) I'm also the author of the SO question mentioned above.

Basically, I've modified the provider like the following:

  1. Modified the Visit methods in SqlInsert and SqlUpdate generators to detect xmin column by name.
  2. Use RETURNING instead of SELECT to grab computed values by modifying Insert/UpdateExpression.WriteSql().

I've also created a custom tool to remap the xmin column in case of a model update from the database, by inserting the following for each table/entity:
Storage Model:
<Property Name="xmin" Type="int8" />

Conceptual:
<Property Name="xmin" Type="Int64" Nullable="false" annotation:StoreGeneratedPattern ="Computed" ConcurrencyMode ="Fixed" />

Mapping:
<ScalarProperty Name="xmin" ColumnName="xmin" />
As you can see, the column is hosted in a variable of type long in client code:

public long xmin
        {
            get { return _xmin; }       
            set
            {
                if (_xmin != value)
                {
                    _xmin = value;
                }
            }
        }
        private long _xmin
            ;

I have no experience with EF6 because I'm also using second level caching with EF5.
Anyway, this is working fine so far (almost 4 years now).

@roji
Copy link
Member Author

roji commented Jul 14, 2016

@omatrot, I'm guessing there's little interest nowadays in adding features that are specific to EF5... However, if there's a way to make things work on EF6 that might be interesting (don't know much about the differences). It's also important for things to work in code-first, not just model-first.

I find it interesting that you didn't run into trouble mapping xmin, which is a uint PostgreSQL type, to long - people have run into issues with that. The other issue people have had is when using migrations - EF6 tries to create the column. The second issue should be fixable by making the provider simply ignore xmin for migration purposes, the uint problem I'm less sure about.

@rwasef1830, maybe you're interested in taking a look.

@rwasef1830
Copy link
Contributor

rwasef1830 commented Jul 14, 2016

@roji I also ran into this a while back when trying to use EF built-in optimistic concurrency, but I worked around it by using my own logic and not using xmin. I'll take a look when I get some time.

@roji
Copy link
Member Author

roji commented Jul 20, 2016

For those interested I've implemented (but not yet released) first-class support for xmin-based optimistic concurrency in EF Core, see npgsql/efcore.pg#19.

@Shkarlatov
Copy link

There are 2 things that prevent us from using the code first:

  1. ef6 tries to create a table with a column xmin
  2. xmin is uint

My decision:

  1. add convention for ConcurrencyCheckAttribute
  2. Write custom SqlGenerator which overrides the table creation function and removes the column marked with the ConcurrencyCheck attribute
  3. in the model, change the type of xmin to text
    internal class CustomSqlGenerator : NpgsqlMigrationSqlGenerator
    {
        protected override void Convert(CreateTableOperation createTableOperation)
        {
            var columns = createTableOperation.Columns;
            var delete = columns.Where(x => x.Annotations.Any(y => y.Key == "Pg_Ignore")).FirstOrDefault();
            columns.Remove(delete);
            base.Convert(createTableOperation);
        }
    }
    class Configuration : DbConfiguration
    {
        public Configuration()
        {
            this.SetDatabaseInitializer(new CreateDatabaseIfNotExists<MyContext>); // or you custom initializer
            SetDefaultConnectionFactory(new NpgsqlConnectionFactory());
            SetMigrationSqlGenerator("Npgsql", () => new CustomSqlGenerator());
            SetProviderFactory("Npgsql", NpgsqlFactory.Instance);
            SetProviderServices("Npgsql", NpgsqlServices.Instance);
        }
    }
    public class Item
    {
        public int ItemId { get; set; }
        public string Title { get; set; }
        [ConcurrencyCheck]
        public string Version { get; private set; }
    }
[DbConfigurationType(typeof(Configuration))]
    public class MyContext : DbContext
    {
        public DbSet<Item> Items { get; set; }

        public MyContext(string nameOrConnectionString) : base(nameOrConnectionString)
        {
        }

        protected override void OnModelCreating(DbModelBuilder modelBuilder)
        {
            modelBuilder.
                Entity<Item>().
                Property(x => x.Version).
                IsConcurrencyToken().
                HasColumnName("xmin").
                HasDatabaseGeneratedOption(DatabaseGeneratedOption.Computed).
                HasColumnType("text");

            modelBuilder.
                Conventions.
                Add(new AttributeToColumnAnnotationConvention<ConcurrencyCheckAttribute, string>("Pg_Ignore", (p, attributes) => attributes.Single().ToString()));
            base.OnModelCreating(modelBuilder);
        }
    }

TestConf: Win7 x64, Postgres 9.6.6, .NET4.6.1, EF6.2.0, npgsql 3.2.6, ef6.npgsql 3.1.1

@roji
Copy link
Member Author

roji commented Dec 21, 2017

I'd prefer having a hardcoded list of system columns in the SQL generator, much like how the EF Core migrations SQL generator handles this. This would be better than a special case for [ConcurrencyCheck].

I also don't understand defining the type as text instead of uint... does that actually work? Isn't long better?

@Shkarlatov
Copy link

I agree that the list is better. About types : if define prop with type uint or long its don't work. I get exception about can't cast uint32 to int64 or what Db is not supported unsingned types. After i tried cast xid to bigint in db

CREATE CAST(xid as bitint)  WITH INOUT AS IMPLICIT

But when inserting values i get exception because i don't create bitint to xid.
If create cast as xid::text is work! Npgsql allow default cast xid to text.
I retrieve values as string and send as string. View modelbuilder for details.

@Shkarlatov
Copy link

Shkarlatov commented Dec 22, 2017

About system column:
if you make an attribute and convention

    public class SystemColumnConvention : Convention
    {
        public SystemColumnConvention()
        {
 this.Properties()
                .Where(x => x.GetCustomAttributes(false).OfType<SystemColumnAttribute>().Any())
                .Configure( c => c.HasColumnName(c.ClrPropertyInfo.GetCustomAttribute<SystemColumnAttribute>().Value).HasColumnAnnotation("SystemColumn",string.Empty).HasDatabaseGeneratedOption(DatabaseGeneratedOption.Computed)); 
        }
    }

    [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
    public class SystemColumnAttribute : Attribute
    {
        public SystemColumnAttribute(string column_name)
        {
            Value = column_name;
        }
        public string Value { get; }
    }

Mark entity class with attribute

    public class Item
    {
        public int ItemId { get; set; }
        public string Title { get; set; }
        [SystemColumn("xmin")]
        public string Version { get; private set; }
    }

And CustomSQLGenerator

      public class SystemColumnSqlGenerator : NpgsqlMigrationSqlGenerator
    {
        protected override void Convert(CreateTableOperation createTableOperation)
        {
            var columns = createTableOperation.Columns;
            columns.
                Where(x => x.Annotations.Any(ann => ann.Key == "SystemColumn")).ToList().
                ForEach(x => columns.Remove(x));
            base.Convert(createTableOperation);
        }
    }

In DbContext in OnModelCreating add new convention (modelBuilder.Conventions.Add(new SystemColumnConvention());)

@roji
Copy link
Member Author

roji commented Dec 22, 2017

IMHO there's no reason to require users to manually add an attribute to their model to specify system columns... The list of PostgreSQL system columns is small and well-known, and can be hard-coded list inside the EF provider... It's up to the user to map their version property to the PostgreSQL xmin column (via the [Column] attribute)...

@Shkarlatov
Copy link

I don `t know why developers do not exclude system columns when creating a database or migrating. I just find a workaround. Problem in this class.
Need review for exclude system column in next functions:

  • CreateTableOperation
  • AddColumnOperation
  • DropColumnOperation
  • AlterColumnOperation
  • RenameColumnOperation

@Shkarlatov
Copy link

OFFTOP
maybe you can help me: how do I force EF to generate "default" in requests for updates and insertions?
Example for what it is needed:

create table Item (
id smallint primary key,
....
LastUpdatedUser text not null default current_user
);

Now, in a transaction, call

 update  Item Set LastUpdatedUser = default where ....

I get the user name.

I tried using entity framework interceptors but there is too little documentation and I did not understand :(

I like ColumnAttribute in linq2db

[Column(SkipOnInsert = true, SkipOnUpdate = true)]

@Bouke
Copy link

Bouke commented Aug 20, 2020

I tried various property types for the code-first model. When using uint or UInt32, it fails when generating the migration:

System.InvalidOperationException: The property 'RowVersion' is not a declared property on type 'ScheduledTask'. Verify that the property has not been explicitly excluded from the model by using the Ignore method or NotMappedAttribute data annotation. Make sure that it is a valid primitive property.

When using Int32 (also 32 bits), it fails when reading from the database:

System.InvalidOperationException: 'The 'RowVersion' property on '...' could not be set to a 'System.UInt32' value. You must set this property to a non-null value of type 'System.Int32'. '

When using Int64 (can represent UInt32), it fails the same:

System.InvalidOperationException: 'The 'RowVersion' property on 'ScheduledTask' could not be set to a 'System.UInt32' value. You must set this property to a non-null value of type 'System.Int64'. '

As noted before, using string doesn't given any exceptions. Any suggestions on making it a proper numeric type in the backing model, to avoid type conversion?

Bouke added a commit to Bouke/doc that referenced this issue Aug 28, 2020
roji pushed a commit to npgsql/doc that referenced this issue Sep 2, 2020
roji pushed a commit to npgsql/doc that referenced this issue Sep 2, 2020
@ramialheshan
Copy link

Hello! I have a problem! I use Devart PostgresSql provider and I have a trigger on the table CTR_ContactInfo which I connect with another table CTR_Person by one-to-many and when I delete an item I want to use cascade for deleting. My problem is. When I delete an item CTR_ContactInfo after this work a trigger and update xmin in CTR_Person table. That's why I can't delete the item from the table CTR_Person. How I can decide my issue?

My queries:
This creates by DBContext

DELETE FROM public."CTR_ContactInfo"
WHERE "ID" = :p0 AND xmin = :p1

DELETE FROM public."CTR_Person"
WHERE "ID" = :p0 AND xmin = :p1

@roji
Copy link
Member Author

roji commented Dec 21, 2020

@ramialheshan this repo is for the Npgsql EF6 provider, not Devart's. Please open a ticket with the Devart support.

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

7 participants