denodb icon indicating copy to clipboard operation
denodb copied to clipboard

problem with db.sync()

Open ralyodio opened this issue 3 years ago • 18 comments

error: Uncaught (in promise) SqliteError: index email already exists

I have a subscriptions table with email as a field.

Using db.sync() causes this error. I'm not sure if I need it. It appears to work without it, but I'm not sure.

Can someone clarify the point of db.sync()

ralyodio avatar Jul 21 '21 00:07 ralyodio

import { Model, DataTypes } from "https://deno.land/x/denodb/mod.ts";
import { db } from '../db.ts';

class Subscriptions extends Model {
  static table = 'subscriptions';
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, default: crypto.randomUUID() },
    email: {
      type: DataTypes.STRING,
      unique: true,
    },
  };
}

db.link([Subscriptions]);
db.sync();

export default Subscriptions

This is the code casuing the crash:

$ deno run  --allow-all index.ts
Check file:///home/ettinger/www/iptvfish/iptvfish-api/index.ts
error: Uncaught (in promise) TypeError: table[type] is not a function
      instruction = table[type](...fieldNameArgs);
                               ^
    at addFieldToSchema (https://deno.land/x/[email protected]/lib/helpers/fields.ts:81:32)
    at TableBuilder._fn (https://deno.land/x/[email protected]/lib/translators/sql-translator.ts:152:15)
    at TableBuilder.toSQL (https://raw.githubusercontent.com/aghussb/dex/master/lib/schema/tablebuilder.js:45:12)
    at SchemaCompiler_SQLite3.createTableIfNotExists (https://raw.githubusercontent.com/aghussb/dex/master/lib/schema/compiler.js:89:25)
    at SchemaCompiler_SQLite3.toSQL (https://raw.githubusercontent.com/aghussb/dex/master/lib/schema/compiler.js:72:26)
    at SchemaBuilder.toSQL (https://raw.githubusercontent.com/aghussb/dex/master/lib/schema/builder.js:81:43)
    at SchemaBuilder.Target.toQuery (https://raw.githubusercontent.com/aghussb/dex/master/lib/interface.js:9:21)
    at SchemaBuilder.toString (https://raw.githubusercontent.com/aghussb/dex/master/lib/schema/builder.js:77:15)
    at SQLTranslator.translateToQuery (https://deno.land/x/[email protected]/lib/translators/sql-translator.ts:225:25)
    at SQLite3Connector.query (https://deno.land/x/[email protected]/lib/connectors/sqlite3-connector.ts:54:36)

ralyodio avatar Jul 21 '21 02:07 ralyodio

bump

chovyprognos avatar Jul 22 '21 18:07 chovyprognos

This is likely because you have not specified a type for id. Try this instead:

/* … */
    id: {
      ...DataTypes.string(36),
      primaryKey: true
    },
/* … */

schuelermine avatar Jul 29 '21 22:07 schuelermine

That code uses ES spread syntax. If you don’t want to use that, do this:

/* … */
    id: {
      type: DataTypes.STRING,
      length: 36,
      primaryKey: true
    },
/* … */

schuelermine avatar Jul 29 '21 22:07 schuelermine

As far as I can tell, the documentation also seems to describe putting defaults in their own static field, like this:

/* … */
  static defaults = {
    id: "00000000-0000-0000-0000-000000000000"
  }
/* … */

schuelermine avatar Jul 29 '21 22:07 schuelermine

Note that it is likely not a good idea to specify a default ID. While your code may look like it generates a random UUID for every entry, it actually generates one UUID when the model is defined, which is used throughout. It may even be invalid to provide a default primary key.

schuelermine avatar Jul 29 '21 22:07 schuelermine

bump

@chovyprognos

That’s not how GH issues work

schuelermine avatar Jul 29 '21 22:07 schuelermine

is there any kind of "beforeCreate" hook to populate an id field? or should I do that in my controller.

ralyodio avatar Sep 05 '21 03:09 ralyodio

I also still get an error with await db.sync() -- this feature doesn't seem to work on existing tables...but if i add a table how can I run it?

error: SqliteError: index email already exists
    at DB._error (file://$deno$/bundle.js:53139:16)
    at DB.query (file://$deno$/bundle.js:53027:24)
    at file://$deno$/bundle.js:67011:44
    at Array.map (<anonymous>)
    at SQLite3Connector.query (file://$deno$/bundle.js:67010:36)
    at Database1.query (file://$deno$/bundle.js:67301:51)
    at Function.createTable (file://$deno$/bundle.js:65925:38)
    at Database1.sync (file://$deno$/bundle.js:67285:25)
    at file://$deno$/bundle.js:67422:11

ralyodio avatar Sep 05 '21 03:09 ralyodio

I have the same issue, db.sync() throws an error when drop set to false.

slim-hmidi avatar Jan 07 '22 13:01 slim-hmidi

What is the error you get when you run sync?

DomThePorcupine avatar Jan 08 '22 08:01 DomThePorcupine

I have 3 models User, Product and Review. I don't want to erase the tables when I start the server because drop set to true removes everything. with drop: false I got relation username is already exists.

username is a column from users table and it contains a constraint to guarantee that is unique. With all model fields with unique set to true I got this issue.

slim-hmidi avatar Jan 08 '22 09:01 slim-hmidi

I think sync() should verify first if the table exists or not then creates it. Currently without any options it creates the models automatically.

slim-hmidi avatar Jan 08 '22 09:01 slim-hmidi

I think that's what it's doing under the hood (CREATE IF NOT EXISTS):

database.ts


async sync(options: SyncOptions = {}) {
    if (options.drop) {
      for (let i = this._models.length - 1; i >= 0; i--) {
        await this._models[i].drop();
      }
    }

    ...
    
    for (const model of this._models) {
      await model.createTable();
    }
  }

model.ts

static async createTable() {
    ....
    const createQuery = this._options.queryBuilder
      .queryForSchema(this)
      .table(this.table)
      .createTable(
        this.formatFieldToDatabase(this.fields) as ModelFields,
        this.formatFieldToDatabase(this.defaults) as ModelDefaults,
        {
          withTimestamps: this.timestamps,
          ifNotExists: true,
        },
      )
      .toDescription();

    await this._options.database.query(createQuery);

   ....
  }

I'd be interested to see how I could reproduce the error you're seeing, if you'd care to share some code snippets

DomThePorcupine avatar Jan 08 '22 09:01 DomThePorcupine

I took a look before at that function. To reproduce the issue you can create those models:

import { DataTypes, Model } from "https://deno.land/x/[email protected]/mod.ts";
import {
  Database,
  PostgresConnector,
} from "https://deno.land/x/[email protected]/mod.ts";


class UserModel extends Model {
  static table = "users";
  static timestamps = true;

  static fields = {
    id: { primaryKey: true, autoIncrement: true, type: DataTypes.INTEGER },
    username: { type: DataTypes.STRING, unique: true },
    email: DataTypes.STRING,
    password: DataTypes.STRING,
  };
  }
  
  class ProductModel extends Model {
  static table = "products";
  static timestamps = true;

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




const connector = new PostgresConnector({
  database: "db",
  host:  "127.0.0.1",
  username: "postgres",
  password: "postgres",
  port:  5432,
});

const db = new Database(connector);


const syncDB = async () => {
  db.link([UserModel, ProductModel])
  await db.sync()
}

First time, it will work correctly but if you restart the server or reload it with denon you will get an issue.

This is the error I get:

NOTICE: relation "users" already exists, skipping
error: Uncaught (in promise) PostgresError: relation "username" already exists
          error = new PostgresError(parseNoticeMessage(current_message));
                  ^
    at Connection.#simpleQuery (https://deno.land/x/[email protected]/connection/connection.ts:626:19)
    at async Connection.query (https://deno.land/x/[email protected]/connection/connection.ts:875:16)
    at async PostgresConnector.query (https://deno.land/x/[email protected]/lib/connectors/postgres-connector.ts:76:22)
    at async Database.query (https://deno.land/x/[email protected]/lib/database.ts:240:21)
    at async Function.createTable (https://deno.land/x/[email protected]/lib/model.ts:172:5)
    at async Database.sync (https://deno.land/x/[email protected]/lib/database.ts:210:7)
  

slim-hmidi avatar Jan 08 '22 09:01 slim-hmidi

Ah yeah, it looks like the table creation generates two commands:

create table if not exists "users" ("id" serial primary key, "username" varchar(255) not null, "email" varchar(255), "password" varchar(255), "created_at" timestamptz not null default CURRENT_TIMESTAMP, "updated_at" timestamptz not null default CURRENT_TIMESTAMP);

and

alter table "users" add constraint "username" unique ("username")

The second one is the one that's failing. Seems like we could check if the constraint already exists before adding it to the query - but I'd guess it'd be easier for denodb to implement some form of migrations instead of building up the sync method...

DomThePorcupine avatar Jan 09 '22 01:01 DomThePorcupine

Thanks for that, so I need to do a workaround to accomplish that until it will be taken into consideration in the future.

slim-hmidi avatar Jan 09 '22 10:01 slim-hmidi

I created a migration file to accomplish my task (it's not clean but it does the work) and I think that the sync method is overengineering and contains a lot of logic that makes things not easy to do. It is better to rethink the design of this function to not contain a lot of details in the background. And the migrations can be simple enough by creating a cli like knex, one for up and the other for down, and user will trigger them manually whenever he needs to update the database.

slim-hmidi avatar Jan 09 '22 12:01 slim-hmidi