[5.0-rc2] Undo GetColumnName behavior change

[AndriySvyryd]

Fixes #22608

Description

In 5.0 complex mapping support was introduced allowing to map an entity type to multiple tables, views, functions and queries. This subtly changed the meaning of GetColumnName return type from a table column name to a base column name to be used for any mapping.

Customer Impact

GetColumnName is used commonly for batch column name processing and the value returned for properties in owned types is different now compared to 3.1

How found

Customer reported on RC1.

Test coverage

This PR adds a test for the affected scenario.

Regression?

Yes.

Risk

Low. The modified method isn't used internally after this change, so there is no danger of other features being affected.

[AndriySvyryd]

I obsoleted these methods as using them for the new complex mappings will not yield the expected results.

[ghost]

Hello @AndriySvyryd!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[tivtag]

I obsoleted these methods as using them for the new complex mappings will not yield the expected results.

How would I receive the StoreObjectIdentifier for the new GetColumnName overload?

Here is my example usage of GetColumnName, which now raises an Obsolete warning.

private static void ConfigureOwnedValueProperties( ModelBuilder modelBuilder )
{
    foreach( IMutableEntityType entityType in modelBuilder.Model.GetEntityTypes() )
    {                
        IEnumerable<IMutableProperty> properties = entityType.GetProperties();

        foreach( IMutableProperty prop in properties )
        {
            Type clrType = prop.DeclaringType.ClrType;

            if( clrType == typeof( OrganisationId ) )
            {
                string currentColumnName = prop.GetColumnName();
                if( currentColumnName.EndsWith( "_Value", StringComparison.Ordinal ) )
                {
                    string alternativeColumnName = currentColumnName.Replace( "_Value", string.Empty );
                    prop.SetColumnName( alternativeColumnName );
                }
            }
        }
    }
}

[AndriySvyryd]

How would I receive the StoreObjectIdentifier for the new GetColumnName overload?

var table = StoreObjectIdentifier.Create(entityType, StoreObjectType.Table);

We will publish documentation for this soon.

[mikes-gh]

@AndriySvyryd

var tableIdentifier = StoreObjectIdentifier.Create(table, StoreObjectType.Table);

returns StoreObjectIdentifier? (nullable)

Which makes it hard to use in for example

p.SetColumnName(p.GetColumnName(tableIdentifier).ToLower());

which requires StoreObjectIdentifier (not nullable)

[AndriySvyryd]

@mikes-gh That's by design. An entity type might not be mapped to any table. And GetColumnName can also return null if the property is not mapped to the given table.

[mikes-gh]

@AndriySvyryd Thanks
Is GetColumnName() deprecated (compiler warning) in 5.0 final or is this rolled back too?
SetColumnName(string name) does not show as depreciated which seems inconsistent.

[AndriySvyryd]

@mikes-gh Agree, we'll deprecate SetColumnName(string) as well in the next release. We'll also introduce a better way of creating naming conventions, so none of these methods would need to be called.

[mikes-gh]

@AndriySvyryd
Re naming conventions this is what I am doing to share a model between SQLServer and postgres in case it helps you understand how people are using this stuff.

using System.Collections.Generic;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;

namespace DataAccess.Contexts
{
    public partial class AesContext : DbContext
    {
        partial void OnModelCreatingPartial(ModelBuilder   modelBuilder)
        {
            if (Database.IsNpgsql())
            {
                var tables = modelBuilder.Model.GetEntityTypes();
                IList<IMutableProperty> timestamps = new List<IMutableProperty>();
                foreach (var table in tables)
                {
                    table.SetTableName(table.GetTableName().ToLower());
                    foreach (var p in table.GetProperties())
                    {
                        p.SetColumnName(p.GetColumnName().ToLower());
                        if (p.Name == "TimeStamp")
                        {
                            timestamps.Add(p);
                        }
                        
                        if(p.GetColumnType() == "money")
                        {
                            p.SetColumnType("numeric");
                        }
                    }

                    var entityTypeBuilder = modelBuilder.Entity(table.ClrType);
                    entityTypeBuilder.UseXminAsConcurrencyToken();
                    foreach (var timestamp in timestamps)
                    {
                        entityTypeBuilder.Ignore(timestamp.Name);
                    }
                }
            }
            else
            {
                var tables = modelBuilder.Model.GetEntityTypes();
                IList<IMutableProperty> xmins = new List<IMutableProperty>();
                foreach (var table in tables)
                {
                    foreach (var p in table.GetProperties())
                    {
                        p.SetColumnName(p.GetColumnName().ToLower());
                        if (p.Name == "xmin")
                        {
                            xmins.Add(p);
                        }
                    }

                    var entityTypeBuilder = modelBuilder.Entity(table.ClrType);
                    foreach (var xmin in xmins)
                    {
                        entityTypeBuilder.Ignore(xmin.Name);
                    }
                }
            }
        }
    }
}