Interactions-as-a-Service / d1-orm

A simple, strictly typed ORM, to assist you in using Cloudflare's D1 product
https://docs.interactions.rest/d1-orm/
Apache License 2.0
161 stars 10 forks source link

Unique Keys - Multi-column #53

Closed kingmesal closed 2 years ago

kingmesal commented 2 years ago

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...

Skye-31 commented 2 years ago

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

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

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 commented 2 years ago

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:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

This could look something like the following:

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

You can have:

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.