nodejh / sequelize-automate

Automatically generate bare sequelize models from your database.
MIT License
116 stars 22 forks source link

Exclude `id` attribute in model if using PostgreSQL? #9

Closed athenawisdoms closed 4 years ago

athenawisdoms commented 4 years ago

Hello, I am new to PostgreSQL and sequelize so the following might be the intended behavior and not a real issue.

sequelize-automate generates model files that include the id attribute when the dialect chosen is postgres, such as:

const {
  DataTypes
} = require('sequelize');

module.exports = sequelize => {
  const attributes = {
    id: {
      type: DataTypes.INTEGER,
      allowNull: false,
      defaultValue: "nextval(my_data_id_seq::regclass)",
      comment: null,
      primaryKey: false,
      field: "id"
    },
    score: {
      type: DataTypes.INTEGER,
      allowNull: true,
      defaultValue: null,
      comment: null,
      primaryKey: false,
      field: "score"
    },
  };
  const options = {
    tableName: "my_data",
    comment: "",
    indexes: []
  };
  const MyDataModel = sequelize.define("my_data_model", attributes, options);
  return MyDataModel;
};

When inserting a row to the my_data table using my_data_model.insert(payload), we get the error:

Unhandled rejection SequelizeDatabaseError: invalid input syntax for integer: "nextval(my_data_id_seq::regclass)"

Some researching of similar issues show that

  1. the id column does not need to be defined in the sequelize model , because ...
  2. PostgreSQL will automatically handle the auto-incrementing id column if it exists.

Manually removing the id attribute from the model generated by sequelize-automate appears to get everything working as expected, with sequelize/PostgreSQL automatically populating the id column with an auto-incrementing integer.

Question: If my understanding is correct, sequelize-automate should exclude the id attribute in the models that it generates if using PostgreSQL. Is there an option to do this? Or should sequelize-automate do this automatically?

Thanks again!

nodejh commented 4 years ago

@athenawisdoms I agree with you and I'll fix it.

nodejh commented 4 years ago

Hi @athenawisdoms , thanks for your feedback. I have fixed this issue, the new version is sequelize-automate@1.2.0.

PostgreSQL use serial to create auto-increment column: set default value to nextval(${table}_${field_seq::regclass) for auto-increment column. So the sequelize-automate@1.1.1 (old version) just set defaultValue: "nextval(my_data_id_seq::regclass)", and it's incorrect in sequelize.

There are two solutions to solve it:

Solution A: just remove id as you said.

Solution B: set autoIncrement to true and remove defaultValue: "nextval(my_data_id_seq::regclass)",.

I think Solution B is better, it's more semantics and clear.

For example, the postgresql table is

CREATE TABLE "user"(
    id SERIAL PRIMARY KEY NOT NULL,
    name TEXT NOT NULL,
    age INT
);

and the model is

const {
  DataTypes
} = require('sequelize');

module.exports = sequelize => {
  const attributes = {
    id: {
      type: DataTypes.INTEGER,
      allowNull: false,
      defaultValue: null,
      comment: null,
      primaryKey: true,
      field: "id",
      // set autoIncrement to true, because the `id` is an auto-increment column
      autoIncrement: true
    },
    name: {
      type: DataTypes.TEXT,
      allowNull: false,
      defaultValue: null,
      comment: null,
      primaryKey: false,
      field: "name",
      autoIncrement: false
    },
    age: {
      type: DataTypes.INTEGER,
      allowNull: true,
      defaultValue: null,
      comment: null,
      primaryKey: false,
      field: "age",
      autoIncrement: false
    }
  };
  // ...
athenawisdoms commented 4 years ago

Your solution B is indeed better, and the updated sequelize-automate@1.2.0 fixed this problem.

Thank you for the explanation as well. Truly appreciate your excellent work here.