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

Adding a shortcut to check for entity existence when creating and deleting indexes and constraints #1801

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TheSquidCombatant
Copy link

@TheSquidCombatant TheSquidCombatant commented May 14, 2024

Now you can write something like this:

    Create
        .Index("IX_Users_CreateDate")
        .OnTable("Users")
        .OnColumn("CreateDate").Ascending()
        .WithOptions().IfNotExists(); // no boilerplate code anymore

Instead of this:

    var exists = Schema
        .Table("Users")
        .Index("IX_Users_CreateDate")
        .Exists();

    if (!exists) Create
        .Index("IX_Users_CreateDate")
        .OnTable("Users")
        .OnColumn("CreateDate").Ascending();

Implements a feature request #1802

@jzabroski
Copy link
Collaborator

jzabroski commented May 19, 2024

  1. Why did you add these additional dependencies to the IMigration interface
  2. Why do the builders now take an extra dependency on the context and migration itself?

It seems like this PR creates some pseudo-circular dependencies I'd like to avoid. I don't have any suggestions yet on how to rework the PR to remove these as I reviewed this on my phone.

Can you also summarize exactly how you handle the anonymous constraint name scenario? In my mind, it would utilize the portable INFORMATION_SCHEMA part of ANSI SQL standard, but it gets hairy in scenarios where user provides their own constraint or in Sybase/SQL Server anonymous constraints generated by legacy code. Anonymous constraints could be considered out of scope since the existing API doesn't handle them well, but I'm curious to hear your thoughts.

Note, Sqlite does not support INFORMATION_SCHEMA. It supports something similar via [sqlite_schema] (https://www.sqlite.org/schematab.html).

@jzabroski jzabroski changed the title Adding a shortcut to check for entity existence when creating and deleting indexes and constraints Adding a shortcut to check for entity existence when creating and deleting indexes and constraints Note, Sqlite does not support INFORMATION_SCHEMA. It supports something similar via [sqlite_schema] (https://www.sqlite.org/schematab.html). May 19, 2024
@jzabroski jzabroski changed the title Adding a shortcut to check for entity existence when creating and deleting indexes and constraints Note, Sqlite does not support INFORMATION_SCHEMA. It supports something similar via [sqlite_schema] (https://www.sqlite.org/schematab.html). Adding a shortcut to check for entity existence when creating and deleting indexes and constraints May 19, 2024
@TheSquidCombatant
Copy link
Author

TheSquidCombatant commented May 19, 2024

@jzabroski

  1. I couldn't mock MaigrationBase properly. So, It forced me extend IMaigration.
  2. This made it possible to implement the desired functionality with minimal code changes.
  3. I won't mind if you come up with a better architecture and change my implementation.
  4. My implementation does not use any new entity existence check functionality, but relies entirely on the existing ISchemaExpressionRoot.

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

Successfully merging this pull request may close these issues.

None yet

3 participants