Feature discussion: Auto-sync table fields

[vmasdani]

Hi. I just noticed that there are currently no feature which allows us to automatically add new fields to a table. For example, if I have this model

class Project extends Model {
  static table = "projects";
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, autoIncrement: true, type: DataTypes.INTEGER },
    name: DataTypes.TEXT,
    date: DataTypes.DATETIME,
  };

  static transactions() {
    return this.hasMany(Transaction);
  }
}

and when I add the newField field

class Project extends Model {
  static table = "projects";
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, autoIncrement: true, type: DataTypes.INTEGER },
    name: DataTypes.TEXT,
    date: DataTypes.DATETIME,

    /* This field */

    newField: DataTypes.INTEGER

    /* This field */ 
  };

  static transactions() {
    return this.hasMany(Transaction);
  }
}

and then we run

await db.sync();

The newField doesn't get automatically added. To add new field, we'll have to pass { drop: true } and remove the whole table. Every ORM I've come across like GORM (Golang) or JPA (Java) has this auto-add fields feature without dropping the table and I think it's crucial.

Are there any plans to implement this yet or is someone already working on it? If not, I'd like to try making this feature.

I don't think this feature is trivial and implementing this can be tricky especially with relationships, so I'll need lots of feedback as I'm working on this.

[eveningkid]

Hey,

Thanks for suggesting this is a great discussion to have :)

I agree with and here's my thoughts:

• Every time we create a table, it should check if a table exists and if so, see if any field is missing. If there's an easy way of making this, why not
• We could have a migration system, maybe based on deno-nessie (but doesn't not support mongodb)

I usually like to look at Eloquent ORM for Laravel, since they really have a top-class API. Maybe it can be done without too much hassle using both deno-nessie and adopting a similar API to Eloquent.

Before starting coding, feel free to share here you would ideally write this from a user pov: what should be the user experience?

[vmasdani]

Hi,

I looked at deno-nessie today. While it's packed with a lot of features unfortunately it's not what I had in mind, because nessie relies heavily on CLI tools, while I want to minimise CLI usage since the reflection system in JS/TS is already powerful enough to run auto-migration with the already declared and linked models. Same case with PHP's artisan method which uses lots of CLI tools, though I think we can borrow some of the concepts from Eloquent (and I believe DenoDB took a lot of inspiration from Eloquent, syntax and concept-wise).

What I had in mind is similar to Golang's gorm and Java/Spring Boot's JPA (in the Create the @Entity Model section), they don't need migration files and CLI tools, only model declarations.

For example scenario, we have this DenoDB model:

class Customer extends Model {
  static table = 'customers';
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, autoIncrement: true, type: DataTypes.INTEGER },
    phoneNumber: DataTypes.INTEGER,
    name: DataTypes.STRING
  };
}

About the user experience, I plan to not make any changes with the current DenoDB syntax.

So with a single

db.sync()

the synchronisation script will automatically:

1. Look for all of the linked models and relationships, and fill in the missing fields in the table schema
2. Throw errors if there are fields previously declared as a type then changed to another type, for example
   • A field called phoneNumber field was declared as DataTypes.INTEGER
   • We run db.sync()
   • Then we change the phoneNumber field to DataTypes.TEXT
   • We run db.sync() again
   • Error is thrown
   • To add the new field, we need to rename it to phoneNumberString or something like that, because phoneNumber has already been declared
3. Not deleting the field which was previously defined but we deleted them in the model, for example
   • The phoneNumber field was deleted from the Customer field
   • The phoneNumber field is not removed from the database, so when we add the phoneNumber field again in the model, the data in database is not lost

About MongoDB, I don't particularly feel that migration is needed since MongoDB aims to be more flexible, not rigid like SQL. Just checking if the collection exists is enough I think, and we don't really need to check if each records have all the fields in a model.

What do you think about this concept?

[eveningkid]

Hi,

I really understand your approach and agree that it should be as simple as possible.

A few notes:

• Migrations are great and clean for rollbacks. Imagine you want to rollback your database to a certain state, all you need is to replay previous migrations. It's commonly used in companies
• Just like you, I like magic and simplicity
• denodb shouldn't be opinionated or hide too much of what it does, especially for schema changes

What I liked about gorm is this method called AutoMigrate(), it looks a lot like your idea. So, here are my thoughts:

• You seem interested about this issue and am sure have the motivation to make it
• You could start developing your idea as db.experimentalAutoMigrate()
• Based on how satisfying it is, we can think of ways to merge it with db.sync (maybe through options)
• We could get inspiration from other ORMs that deal with this

Again, I don't want "migration users" to be disoriented with this decision. Things can stay separated while we experiment/implement this, then merged with the main user experience.

Hope this makes sense :)

Let me know if that sounds like a plan to you

[vmasdani]

Hi,

Yes, the AutoMigrate function is exactly what I'm trying to implement here.

I understand if DenoDB is trying to be as unopiniated as possible, and I agree that things should be separated/marked as experimental as we're working on this (through db.experimentalAutoMigrate()), then after things are working as intended we'll talk about how to integrate it with db.sync()through options.

It's true that migration system & auto-sync function have both merits and downsides, & users should have the freedom to pick one of the methods & it should not be enforced.

If it's all well, I'm going to try to make the implementation in a separate function. I'll be sure to give updates about the progress
along the way👍

[eveningkid]

Yes, all good. I'll do my best to get back to you as soon as you share progress with me :)

Try to have fun doing it, there's no rush.

Thanks for helping with this, have a good day

[jrdx0]

Guys! Sorry to invited my self to the discussion. I'm so excited to see where this functionality will be go! If you need help, count on me.

[eveningkid]

Yes, btw @vmasdani even if you have a draft ready at some point, please share it so you can get early feedback too.

Good to know I can count on you too @stillalivx!

This feature seems to be required by other folks so let's see how we can all push this forward :)

[vmasdani]

Hey @eveningkid! sorry for the really late reply. I haven't really made a progress since I had a few problems with my development machine. A few days ago my 10-year old HDD died and I had to replace it with a new drive, but it's all solved now.

And since a long holiday is coming up in a few days (Eid) I can focus and get up to speed with the library's code in a few days, and I think you guys can expect some news in a week or two.

Thank you guys for waiting, really appreciate it

[eveningkid]

Sure, no problem really @vmasdani :)

[vmasdani]

Hi, I've done some work on my fork vmasdani/denodb. The function is experimentalAutoMigrate. I've successfully detected missing fields using select field like this:

// https://github.com/vmasdani/denodb/blob/287a975371ea87185594940ebf884768183041e0/lib/model.ts#L208
this._queryBuilder
  .select(key)
  .table(
    this.table,
  ).get();

If the field does not exist, it will throw an error and will try to create the missing field/column. (Note the [object Object] bug while mapping the primary key, haven't fixed that yet)

I tried an attempt to run an alter table query. However I learned that QueryType does not support alter table and add column yet so I had to add the query type and description here and here.

After that, I also noticed that this library uses dex for dialect translation, so It'll take some time for me to learn how to map QueryDescription to dex's query builder for the alter table.

// https://github.com/vmasdani/denodb/blob/287a975371ea87185594940ebf884768183041e0/lib/model.ts#L221
const alterQuery = this._queryBuilder
  .removeSelect()
  .alterTable(this.table)
  .addColumn(key)
  .columnType(this.fields[key].toString());

I haven't made tests but you can use this code to try the experimentalAutoMigrate function with this code:

// main.ts
import {
  Database,
  DataTypes,
  Model,
  PostgresConnector,
  SQLite3Connector,
} from "./denodb/mod.ts";

const connection = new SQLite3Connector({
  filepath: "db.sqlite3",
});

const db = new Database(connection);

class Flight extends Model {
  static table = "flights";
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, autoIncrement: true, type: DataTypes.INTEGER },
    departure: DataTypes.STRING,
    destination: DataTypes.STRING,
    flightDuration: DataTypes.FLOAT,
  };

}

db.link([Flight]);

await db.experimentalAutoMigrate();

Run with deno run --allow-all main.ts

I'll give more updates when I finished integrating QueryDescription into dex. Should I open a PR for this progress, by the way?

[eveningkid]

This is a really great job Muhammad, please open a PR and make it a draft (if you don't see the option it's alright).

I'd like to make a few comments but let's keep both separated, you're right :)

[vmasdani]

@eveningkid I'll just name the title draft since there are no options ;) https://github.com/eveningkid/denodb/pull/259

[vmasdani]

Hi @eveningkid , I have made some progress with this. I have successfully executed alterTable with dex and the missing fields were successfully automatically made.

But I have problems matching column type with Dex's SQL type, like here with (I followed Dex's basic usage as an example). I am using table.text(query.addColumn) for a placeholder.

Do I have to match all the SQL column types to dex's column types? If yes, I'll do just that. What I meant by matching the types is like this:

switch (query.columnType) {
          case "text":
            table.text(query.addColumn);
            break;

          case "string":
            table.string(query.addColumn);
            break;

          case "integer":
            table.integer(query.addColumn);
            break;

          default:
            break;
}  

[vmasdani]

Oh my, I just discovered that I can just do it like this

table[query.columnType](query.addColumn);

I guess I learn new things everyday. I'll just implement it later

[jrdx0]

Hey, @vmasdani! In what DB engine are you testing your changes?

[vmasdani]

@stillalivx I am using sqlite3. But I think it shouldn't matter much what database this auto-migrate feature uses because Dex will translate the query builder to the correct SQL dialects which DenoDB currently supports (postgresql, mysql, mariadb, sqlite3).

But still, it would be nice to have tests in other databases than sqlite3.

[eveningkid]

Hey Muhammad,

I'll find time next week to review all your changes.

I'm sorry you've had to wait for so long, really.

Have a great weekend :)

[vmasdani]

Sure @eveningkid please take your time! And please take it easy too since this isn't too urgent 😉

[vmasdani]

hello @eveningkid , really really sorry because I did not notice your comments in https://github.com/eveningkid/denodb/pull/259 , I did not know that you have made some really useful reviews there, I thought you haven't responded in 3 months as I missed Github's email notification on the PR for some reason

Will continue the progress of this feature pretty soon. Again, really sorry for my clumsiness 😅

[ralyodio]

is this project dead? I can't fathom an ORM where you cannot modify the fields after you create the initial table. Tell me I'm missing something.

[AbyssalInteractive]

Any news?

[EmreSURK]

Thank you for the great project but the migration mechanism is the heart of the ORMs. We need a reliable migration mechanism and the mechanism should avoid data loss in any case no matter what.

I strongly believe we completely remove the 'drop' parameter to prevent any mistakes.