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

[BUG] Cannot create foreign keys referencing tables in non-default schema #4722

Closed
Hypnodude opened this issue Oct 13, 2024 · 4 comments
Closed

Comments

@Hypnodude
Copy link

Oqtane Info

Version - 5.2.3
Render Mode - Any
Interactivity - Any
Database - SQL Server

Describe the bug

The Oqtane.Migrations.ForeignKey struct does not contain a property for the PrincalSchema, so it is only capable of handling foreign keys in the default schema.

Expected Behavior

EFCore allows for the specification of a ForeignKey in a non-default schema using the PrincalSchema property, but this is not captured in Oqtane and passed to EFCore.
Oqtane should expose the PrincialSchema property on the ForeignKey structure to allow it to be captured and passed to the underlying EFCore module.

Steps To Reproduce

Create a couple of tables in a different (non-default) schema (eg. fees). and try create a foreign key reference from 1 table to another.
LookupTable:

    {
        private const string _entityTableName = "AccountTypes";
        private readonly PrimaryKey<AccountTypeEntityBuilder> _primaryKey = new("PK_AccountTypes", x => x.Id);
        private readonly MigrationBuilder _migrationBuilder;

        public AccountTypeEntityBuilder(MigrationBuilder migrationBuilder, IDatabase database) : base(migrationBuilder, database)
        {
            Schema = "fees";
            EntityTableName = _entityTableName;
            PrimaryKey = _primaryKey;
            _migrationBuilder = migrationBuilder;
        }

        protected override AccountTypeEntityBuilder BuildTable(ColumnsBuilder table)
        {
            Id = AddAutoIncrementColumn(table, "Id");
            Description = AddStringColumn(table, "Description", 100, true);
            RowOrder = table.Column<short>(name: ActiveDatabase.RewriteName("RowOrder"), type: "smallint");
            Inactive = AddBooleanColumn(table, "Inactive", false, false);

            return this;
        }
        public OperationBuilder<AddColumnOperation> Id { get; set; }

        public OperationBuilder<AddColumnOperation> Description { get; set; }

        public OperationBuilder<AddColumnOperation> RowOrder { get; set; }

        public OperationBuilder<AddColumnOperation> Inactive { get; set; }
    }

Data Table referencing the lookup table:

    {
        private const string _entityTableName = "Accounts";
        private readonly PrimaryKey<AccountEntityBuilder> _primaryKey = new("PK_Accounts", x => x.Id);
        private readonly ForeignKey<AccountEntityBuilder> _accountTypeForeignKey = new("FK_Accounts_AccountTypes", x => x.TypeId, "AccountTypes", "Id", "fees", ReferentialAction.Restrict);
        private readonly MigrationBuilder _migrationBuilder;

        public AccountEntityBuilder(MigrationBuilder migrationBuilder, IDatabase database) : base(migrationBuilder, database)
        {
            Schema = "fees";
            EntityTableName = _entityTableName;
            PrimaryKey = _primaryKey;
            ForeignKeys.Add(_accountTypeForeignKey);
            _migrationBuilder = migrationBuilder;
        }

        protected override AccountEntityBuilder BuildTable(ColumnsBuilder table)
        {
            Id = AddAutoIncrementColumn(table, "Id");
            TypeId = AddIntegerColumn(table, "TypeId", false);
            AccountCode = AddStringColumn(table, "AccountCode", 25, true);
            Description = AddStringColumn(table, "Description", 255, true);
            RowOrder = table.Column<short>(name: ActiveDatabase.RewriteName("RowOrder"), type: "smallint");
            Inactive = AddBooleanColumn(table,"Inactive", false, false);

            return this;
        }

        public OperationBuilder<AddColumnOperation> Id { get; set; }

        public OperationBuilder<AddColumnOperation> TypeId { get; set; }

        public OperationBuilder<AddColumnOperation> AccountCode { get; set; }

        public OperationBuilder<AddColumnOperation> Description { get; set; }

        public OperationBuilder<AddColumnOperation> RowOrder { get; set; }

        public OperationBuilder<AddColumnOperation> Inactive { get; set; }
    }

Anything else?

@Hypnodude
Copy link
Author

I've fixed this on my local system so that I can continue to work, but I'm unable to push my changes and create a PR.
I've uploaded /attached the changes I've applied locally.
4722.zip

@sbwalker
Copy link
Member

@Hypnodude when you say "EFCore allows for the specification of a ForeignKey in a non-default schema using the PrincalSchema property" I am guessing that you are referring to SQL Server only? Oqtane supports multiple databases - MySQL, PostgreSQL, SQLite - and some of them do not support schemas.

@Hypnodude
Copy link
Author

@Hypnodude when you say "EFCore allows for the specification of a ForeignKey in a non-default schema using the PrincalSchema property" I am guessing that you are referring to SQL Server only? Oqtane supports multiple databases - MySQL, PostgreSQL, SQLite - and some of them do not support schemas.

Hi @sbwalker,
Looking at the ForeignKey.cs file - it does not support the PrincipalSchema property which EFCore uses for the migrations.
It could be that it just relates to SQL Server - I haven't tried any of the other databases, but the solution I proposed simply adds another constructor to the ForeignKey struct - one that allows passing in the Principal Schema and then assigning it to the property.
The other changes relate to passing the property to EFCore when running the migrations.
This shouldn't break any other databases - providing you do not specify a different schema for the foreign key - existing behaviour would be retained.

sbwalker added a commit that referenced this issue Oct 14, 2024
fix #4722 - support PrincipalSchema when creating foreign keys (credit @Hypnodude)
@Hypnodude
Copy link
Author

Thanx @sbwalker - much appreciated

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

No branches or pull requests

2 participants