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

Unique Keys - Multi-column #53

Closed
kingmesal opened this issue Oct 13, 2022 · 5 comments · Fixed by #55
Closed

Unique Keys - Multi-column #53

kingmesal opened this issue Oct 13, 2022 · 5 comments · Fixed by #55
Labels
enhancement New feature or request

Comments

@kingmesal
Copy link
Contributor

Wanted to see if you have any ideas on how to structure a multi-column unique key?

The following model:

        username: {
            type: DataTypes.STRING,
            unique: true,
            notNull: true,
        },
        session: {
            type: DataTypes.STRING,
            unique: true,
            notNull: true,
        },

Producing a table like:

CREATE TABLE users (
usename text not null unique,
session text not null unique
);

produces a table which does not allow the same username to be inserted into multiple records.

In reality what is needed is:

        username: {
            type: DataTypes.STRING,
            notNull: true,
        },
        session: {
            type: DataTypes.STRING,
            notNull: true,
        },
        uniqueKeys: { [username, session], ... },

which when generated would produce

CREATE TABLE users (
usename text not null,
session text not null,
    UNIQUE (username, session) // This means the combination must be unique.
);

The benefit of this type of approach means foreign key definition support would be able to piggy back. Foreign keys have a few more options, but the basics are the most important part...

@kingmesal kingmesal added the enhancement New feature or request label Oct 13, 2022
@Skye-31
Copy link
Contributor

Skye-31 commented Oct 14, 2022

I'm not a fan of having the uniqueKeys syntax within the columns definition, what if a user wanted to have a column called that?

Here are some alternate suggestions:

Unique keys as a string

  • Non-breaking solution
  • May be a little messier?
{
  username: {
    type: DataTypes.STRING,
    unique: "some-arbitrary-key",
  },
  session: {
    type: DataTypes.STRING,
    unique: "some-arbitrary-key", // the same key as above, indicating these are related, this isn't passed through to sqlite, it's just used for the orm to identify that those two keys are related
  },
  someOtherColumn: {
    type: DataTypes.STRING,
    unique: true, //this is still functional for a single unique column
  },
}

The above syntax is already similar to the way Sequelize does this, and I like the familiarity of it, however I could see it becoming quickly confusing to read when you have a higher number of columns.

Refactoring unique keys, primary keys etc into the first parameter of the model

  • Breaking, however may cleanup syntax if it's currently confusing?
  • Will require a lot more docs changes
  • Does look nicer admittedly

This could look something like the following:

const users = new Model({
  tableName: "users",
  D1Orm: myOrmInstance,
  primaryKeys: "username", //string|string[] (we can strictly type this to only include columns shown below),
  autoIncrement?: string, // column name that we want to apply this to
  uniqueKeys: [ ["id"], ["username", "session"] ], //(string[][], can also be enforced to be keys of the columns),
  // in this case, "id" would be unique to itself, and username and session would be unique to each other
}, {
  id: {
    type: DataTypes.INTEGER,
    notNull: true,
    //does not include primaryKey, unique, or autoIncrement here anymore
  },
  username: {
    type: DataTypes.STRING,
    notNull: true,
  },
  session: {
    type: DataTypes.STRING,
    notNull: true
  }
});

@kingmesal
Copy link
Contributor Author

I like the example you shared following sequelize

{
  username: {
    type: DataTypes.STRING,
    unique: "user-key",
  },
  uid: {
    type: DataTypes.STRING,
    unique: ["user-key", "session-combo"],
  },
  session: {
    type: DataTypes.STRING,
    unique: ["session-combo"],
  },
  someOtherColumn: {
    type: DataTypes.STRING,
    unique: true, //this is still functional for a single unique column
  },
}

The reasons I think this works better is:

  • it is less prone to errors
  • aligns more closely with the model

A table may only have 1 primary key (single field, or multiple field). Defining that on the field is most logical. Each column may only have a single foreign key. Since each field may participate in multiple unique keys (same column, multiple unique combinations). In this case unique, is, well, unique. Boolean, string or array of strings.

@Skye-31
Copy link
Contributor

Skye-31 commented Oct 14, 2022

While I see cases for both, I'm inclining towards the second approach currently. Say you have a table with 50 columns - it's a lot more tedious to look through each of those to locate which is the primary key, unique keys etc, over seeing it in one central place in the table.

It's also a lot clearer of the relationship between plural unique columns, I had to read the codeblock you just sent about 3 times to understand the behaviour you wanted (and even then I'm not 100% on it - it sounds like you want username to be unique, as well as session as well as both of them? - which is the same as just having each one with unique:true).

My key focus of this library is clarity - I want a user to know what their definition is doing immediately, and why. Defining these things in a central place definitely improves that.

@kingmesal
Copy link
Contributor Author

Being pragmatic, if someone has 50+ columns they are likely to hit ctrl-f ... In 20+ years of engineering, finding and consuming an ORM definition in this way is not the problem. Reducing errors is the biggest concern. That would be the number one reason to NOT separate them.

For unique keys, most people are in an IDE and so highlighting the key label will auto highlight all cases.

Here are some examples that can be built for unique keys...
Any single column unique = only one row in the entire table allowed to have that field value.
Arbitrary multi-column unique keys ... the combination of (column A, column B...) is a unique entry.

Our intention is not to question why people would overlap a single column into two different unique key sets, but they are not the same as just marking the column as a whole unique.

@isaac-mcfadyen
Copy link
Contributor

isaac-mcfadyen commented Oct 14, 2022

This could look something like the following:

From Skye's example here, it looks like that's what @kingmesal is describing.

You can have:

  • a single unique key ["id"]
  • multi-column unique key ["username", "id"]

And by simply putting the same column in two arrays (so an array of [["id"], ["username", "id"]]) you can have overlapping unique keys.

Personally I prefer this array-like approach because it's all in one place: if I glance at the array, I instantly understand that id as well as a combination of id and username must be unique, versus looking through all the columns.

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

Successfully merging a pull request may close this issue.

3 participants