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

underscore / camelcase option should control all fields and table name generation #6423

Closed
alexey2baranov opened this issue Aug 12, 2016 · 51 comments

Comments

@alexey2baranov
Copy link

@alexey2baranov alexey2baranov commented Aug 12, 2016

It would be great if sequelize will generate PostgreSQL DDL with lower_case_underscored table name and column names. By now underscore option manage foreign keys and create_at, modifed_at fields only.

In short

CREATE TABLE IF NOT EXISTS "Male" ("CamelCase" VARCHAR(255), "created_at" TIMESTAMP WITH TIME ZONE NOT NULL, "updated_at" TIMESTAMP WITH TIME ZONE NOT NULL);

should become

CREATE TABLE IF NOT EXISTS male (camel_case VARCHAR(255), "created_at" TIMESTAMP WITH TIME ZONE NOT NULL, "updated_at" TIMESTAMP WITH TIME ZONE NOT NULL);

my global config is

{
    "username": "",
    "password": "",
    "database": "",
    "host": "127.0.0.1",
    "dialect": "postgres",
    "define":{
      "paranoid":false,
      "timestamps":true,
      "freezeTableName": true,
      "underscored": true
    }
}
@alexey2baranov alexey2baranov changed the title model underscore should produce underscored table_name and field_name's underscore option should produce underscored_table_name and underscored_field_name's Aug 12, 2016
@sushantdhiman
Copy link
Contributor

@sushantdhiman sushantdhiman commented Aug 12, 2016

Sounds like a reasonable feature to me, @janmeier ?

@janmeier
Copy link
Member

@janmeier janmeier commented Aug 12, 2016

I don't think this is needed - If users want underscored names, they can simply define them as such, or use the field option.

The underscored option is for controlling auto-generated column names, but when a user tells us about the column fooBar, we call it fooBar ;)

This can easily be solved with a beforeDefine hook

@sushantdhiman
Copy link
Contributor

@sushantdhiman sushantdhiman commented Aug 12, 2016

Correct, but there is lots of work involved with that 😆 , I would prefer one single option based control.

If anyone can share their experiences from other ORMs like ActiveRecord (Rails) or Eloquent (Laravel / PHP), that will be really helpful to make this decision.

@sushantdhiman sushantdhiman changed the title underscore option should produce underscored_table_name and underscored_field_name's underscore / camelcase option should control all fields and table name generation Aug 12, 2016
@felixfbecker
Copy link
Contributor

@felixfbecker felixfbecker commented Aug 12, 2016

I also prefer underscored columns and tables and camelCase attributes and PascalCase model names. I think the ORM should allow to map this easily.

@maxsummers
Copy link

@maxsummers maxsummers commented Jan 3, 2017

It took me a long time to figure out why underscored option didn't convert my camelCase column names to under_scored ones.

I would also prefer to declare my columns as camelCased ones to be automatically converted later to underscored ones in SQL.

@elvisgs
Copy link

@elvisgs elvisgs commented May 5, 2017

+1

@quocvu
Copy link

@quocvu quocvu commented Jun 1, 2017

I also agree w/ @alexey2baranov. I know we use of the field option, but that makes the model more verbose. Typically, when we name a field fooBar, it is to keep it consistent w/ the rest of the javascript code and not that we really want fooBar as a column name.

The current behavior is not desirable. Who would want mixed column naming scheme in their tables?

At the minimum, the documentation should be fixed. It doesn't say underscored only applies to auto-generated fields.

Converts all camelCased columns to underscored if true. Will not affect timestamp fields named explicitly by model options and will not affect fields with explicitly set field option
@pgollangi
Copy link

@pgollangi pgollangi commented Jun 5, 2017

+1

@stale
Copy link

@stale stale bot commented Aug 12, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@alexey2baranov
Copy link
Author

@alexey2baranov alexey2baranov commented Aug 12, 2017

Up

@stale stale bot removed the stale label Aug 12, 2017
@elvisgs
Copy link

@elvisgs elvisgs commented Aug 12, 2017

+1

@maxnachlinger
Copy link

@maxnachlinger maxnachlinger commented Aug 15, 2017

I took @janmeier 's sugggestion and accomplished this via the following beforeDefine hook:

const decamelize = require('decamelize');

// convert camelCase fields to underscored
sequelize.addHook('beforeDefine', (attributes) => {
  Object.keys(attributes).forEach((key) => {
    // typeof check provided by @devalnor 
    if (typeof attributes[key] !== "function" ) {
       attributes[key].field = decamelize(key);
    }
  });
});
@dagumak
Copy link

@dagumak dagumak commented Aug 26, 2017

The ORM should be in camel case because we are working with JavaScript here so it's more convenient to write everything using camel case. However, it makes sense to store the database names, table names, and column names as underscore.

@dagumak
Copy link

@dagumak dagumak commented Aug 26, 2017

@felixfbecker Is this something in the roadmap?

@felixfbecker
Copy link
Contributor

@felixfbecker felixfbecker commented Aug 26, 2017

I stepped down as a maintainer don't know

@tomgao365
Copy link

@tomgao365 tomgao365 commented Sep 7, 2017

+1

1 similar comment
@devalnor
Copy link

@devalnor devalnor commented Sep 11, 2017

+1

@devalnor
Copy link

@devalnor devalnor commented Sep 11, 2017

@maxnachlinger your hook will not work with data-types functions.

This will prevent the bug, but it's not a solution.

// Convert camelCase fields to underscored
sequelize.addHook('beforeDefine', (attributes, options) => {
    Object.keys(attributes).forEach((key) => {
        if (typeof attributes[key] !== "function" ) {
            attributes[key].field = decamelize(key);
        }
    });
});
@netsgnut
Copy link
Contributor

@netsgnut netsgnut commented Sep 12, 2017

Would really like an option to convert column names to snake cases, as mentioned by @sushantdhiman here.

For the record, the above beforeDefine hook works like a charm (thanks!). However one of the minor downsides is that if one uses sequelize-cli as well, in migrator definitions we would have to define columns in snake case manually instead of allowing the hook to automatically convert to snake case - so that would be models in camelCase but migrations in snake_case.

@kurtzilla
Copy link

@kurtzilla kurtzilla commented Oct 10, 2017

The beforeDefine hook is not working for me. Could someone show me an implementation? Perhaps I am using the wrong sequelize version (4.13.6)?

@sushantdhiman sushantdhiman added this to the 5.0 milestone Oct 16, 2017
@napalm272
Copy link

@napalm272 napalm272 commented Oct 16, 2017

I agree with @quocvu, please at least fix the documentation to not say

Converts all camelCased columns to underscored if true.

@pateketrueke
Copy link

@pateketrueke pateketrueke commented Mar 17, 2018

This would affect also FKs being generated?

A model could have UserId as FK and user_id as another field.

What could be wrong using UserId if we set everything as underscored?

@dw1284
Copy link

@dw1284 dw1284 commented Mar 29, 2018

I agree with everybody else's sentiments. Convert field names from camel to snake on the way in, and snake to camel on the way out. This feature is pretty standard in other ORMs these days.

rogerc pushed a commit to rogerc/sequelize that referenced this issue Apr 1, 2018
Provides snake_case for all property names by setting field to snake_case
but keeping the name as camelCase.

The alternative is to manually define a hook.

sequelize.addHook('beforeDefine', (attributes) => {
    Object.keys(attributes).forEach((name) => {
        if (typeof attributes[name] !== 'function') {
            attribute = attributes[name];
            const _underscored = attribute.underscored === undefined ? sequelize.options.define.underscored : attribute.underscored;
            console.log("NAME:", name, _underscored, attribute._underscored === undefined, sequelize.options.define.underscored, attribute.underscored);
            if (attribute.field === undefined && _underscored !== undefined) {
                attribute.field = sequelize.Utils.underscoredIf(name, _underscored);
            }
        }
    });
});
@rogerc rogerc mentioned this issue Apr 2, 2018
5 of 5 tasks complete
@rogerc
Copy link
Contributor

@rogerc rogerc commented Apr 2, 2018

In case the pull request doesnt go in, you can use the code below which is similar to the hook mentioned several times but it uses the config settings rather than assume all needs to be snake_case.

sequelize.addHook('beforeDefine', (attributes) => {
    Object.keys(attributes).forEach((name) => {
        if (typeof attributes[name] !== 'function') {
            attribute = attributes[name];
            const _underscored = attribute.underscored === undefined ? sequelize.options.define.underscored : attribute.underscored;
            if (attribute.field === undefined && _underscored !== undefined) {
                attribute.field = sequelize.Utils.underscoredIf(name, _underscored);
            }
        }
    });
});
rogerc pushed a commit to rogerc/sequelize that referenced this issue Apr 2, 2018
Added define tests to validate correct output using underscored.
rogerc pushed a commit to rogerc/sequelize that referenced this issue Apr 2, 2018
Updated docs to make reference to new behaviour.
rogerc pushed a commit to rogerc/sequelize that referenced this issue Apr 2, 2018
…elize#6423 - PR sequelize#9260

Provides consistent behaviour with new implementation for createdAt, updatedAt and deletedAt.
Added checks to test.
Removed unnecessary manipulation during init and just doing it in refreshAttributes
@knoxcard
Copy link
Contributor

@knoxcard knoxcard commented Apr 4, 2018

@rogerc - I currently use MariaDB, but good to know if I ever switch to PostgreSQL, one less thing to worry about!

@didac-wh
Copy link

@didac-wh didac-wh commented Apr 4, 2018

I'm afraid that, although those cool hooks work when"force: true", they do not when using alter: true.
Since Sequelize checks the db schema usinc describeTable() it can't do the decamelize/camelize process without editing the package code.

Does anyone have a solution for this (apart from editing the Sequelize source code)?

@sushantdhiman
Copy link
Contributor

@sushantdhiman sushantdhiman commented Apr 7, 2018

I would like to propose / share how I envision implementation of this change. I would love to hear comments before this actually gets implemented.


There will be only one option underscored (underscoredAll and underscored will be merged)

Attributes

Setting Model.options.underscored: true will

  1. set field to a name constructed by underscoring attribute. So attribute myColumn will have field my_column
  2. If an attribute already got field defined, underscore will not override field for that attribute

Model.tableName

  1. If Model.options.tableName is defined use it
  2. if Model.options.freezeTableName: true set table name to model name
  3. Otherwise pluralize Model.name then underscore it if underscored: true

Auto-generated Attributes (timestamps / version / foreignKeys)

OLD (Current)

If Model.options.underscored: true, then all attributes are generated with underscored name. like Model.rawAttributes.created_at / updated_at / foreign_key` etc

NEW (Proposed)

All attributes will be generated with camelcased name like Model.rawAttributes.createdAt / updateAt / foreignKey etc, but field will be set to underscored name like created_at / updated_at / foreign_key if Model.options.underscored: true

TLDR;

All attributes will be generated with camelcase naming by default, specifying underscore: true will set field by underscoring attribute name


Still using custom attribute name

  1. Foreign keys, you will be able to pass association.options.foreignKey where applicable that attribute name will be used.
// define Task.user_id, use of `user_id` attribute as foreignKey on Task

User.hasMany(Task, { foreignKey: 'user_id' });
Task.belongsTo(User, { foreignKey: 'user_id' });
  1. Timestamps
sequelize.define('Model', {
  name: Sequelize.STRING
}, {
  createdAt: 'created_at', // will define attribute created_at on Model
  updatedAt: 'updated_at' // will define attribute updated_at on Model
})
@rogerc
Copy link
Contributor

@rogerc rogerc commented Apr 7, 2018

Sounds good. My PR I think covered all attributes except foreignKeys. Table name was left untouched.

@pickfire
Copy link

@pickfire pickfire commented Mar 13, 2019

I am surprised that underscored does not affect association and user still need to use { as: 'something_underscored' }, maybe this specification should mentioned it? It is not really underscored mode that what have been expected.

@up9cloud
Copy link
Contributor

@up9cloud up9cloud commented May 27, 2019

Memo for whom upgrade v4 to v5 with underscored: true and get unexpected field names (createdAt, updatedAt, deletedAt) from query result:

TL; DR

In v4 you can just set underscored: true and ignore those

{
    createdAt: 'created_at',
    updatedAt: 'updated_at',
    deletedAt: 'deleted_at'
}

In v5, you HAVE TO add them to your define options, otherwise you'll get camel cased field names in your query result 😭 .

Details

v4 original

$ npx sequelize --version
Sequelize CLI [Node: 10.15.3, CLI: 5.4.0, ORM: 4.43.2]
// db/foo.js
module.exports = function (sequelize, DataTypes) {
  const tableName = 'foo'
  return sequelize.define(tableName, {
    id: { type: DataTypes.INTEGER.UNSIGNED, primaryKey: true, autoIncrement: true }
  })
}
// ./db/index.js
const fs = require('fs')
const path = require('path')
const Sequelize = require('sequelize')
const sequelize = new Sequelize({
  ...
  define: {
    paranoid: true,
    underscored: true,
    freezeTableName: true
    // no need to add createdAt, ... options
  }
})
let db = { sequelize }
fs
  .readdirSync(__dirname)
  .filter(file => {
    if (
      file.indexOf('.') === 0 ||
      file === path.basename(__filename)
    ) {
      return false
    }
    return true
  })
  .forEach(file => {
    let model = sequelize.import(path.join(__dirname, file))
    db[model.name] = model
  })
module.exports = db

v5

$ npx sequelize --version
Sequelize CLI [Node: 12.3.1, CLI: 5.4.0, ORM: 5.8.6]
// ./db/foo.mjs
import Sequelize from 'sequelize'
export default function (sequelize, DataTypes) {
  const tableName = 'foo'
  class model extends Sequelize.Model { }
  model.init({
    id: { type: DataTypes.INTEGER.UNSIGNED, primaryKey: true, autoIncrement: true }
  }, {
    sequelize,
    modelName: tableName,
    tableName
  })
  return model
}
// ./db/config.mjs
export default {
  ...
  define: {
    paranoid: true,
    underscored: true,
    freezeTableName: true
  }
}
// ./db/index.mjs
import Sequelize from 'sequelize'
import config from './config.mjs'
import foo from './foo.mjs'
const sequelize = new Sequelize(config)
const DataTypes = Sequelize.DataTypes
let db = {
  sequelize,
  foo: foo(sequelize, DataTypes)
}
export default db

v5 fixed

// ./db/fixedDb.mjs
import Sequelize from 'sequelize'
import config from './config.mjs'
import foo from './foo.mjs'

const fixedConfig = {
  ...config,
  define: {
    ...config.define,
    // You HAVE TO add those...
    createdAt: 'created_at',
    updatedAt: 'updated_at',
    deletedAt: 'deleted_at'
  }
}
const sequelize = new Sequelize(fixedConfig)
const DataTypes = Sequelize.DataTypes
let db = {
  sequelize,
  foo: foo(sequelize, DataTypes)
}
export default db

Test

// ./test.mjs
import db from './db/index.mjs'
import fixedDb from './db/fixedDb.mjs'
async function main () {
  await db.foo.create()

  let r = await db.foo.findOne()
  console.log(r.get({ plain: true }))

  let rr = await fixedDb.foo.findOne()
  console.log(rr.get({ plain: true }))
}
main()
$ node --experimental-modules ./test.mjs
{
  id: 1,
  createdAt: 2019-05-27T17:33:17.000Z,
  updatedAt: 2019-05-27T17:33:17.000Z,
  deletedAt: null
}
{
  id: 1,
  created_at: 2019-05-27T17:33:17.000Z,
  updated_at: 2019-05-27T17:33:17.000Z,
  deleted_at: null
}

@leodutra
Copy link

@leodutra leodutra commented May 28, 2019

The underscore is not common in JavaScript properties, so makes more sense to consider camel case first.

@papb
Copy link
Member

@papb papb commented Aug 29, 2019

Related: #11225

@CarlosDanielDev
Copy link

@CarlosDanielDev CarlosDanielDev commented Jan 28, 2020

module.exports = {
  dialect: 'mysql',
  host: 'localhost',
  username: 'root',
  password: 'root',
  database: 'awesome',
  port: 3306,
  define: {
    timestamps: true,
   cameCased: true,
   underscoredAll: true,
  }
};

How can I change underscoredAll to Camel Case ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet