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

Custom types on arrays in posgres generates the wrong name when using underscores in columns #13204

Closed
2 of 7 tasks
nahog opened this issue Apr 16, 2021 · 1 comment · Fixed by #13210
Closed
2 of 7 tasks
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). Great SSCCE This issue has a great SSCCE/MCVE/reprex posted and therefore deserves extra attention! :) status: understood For issues. Applied when the issue is understood / reproducible. type: bug

Comments

@nahog
Copy link
Contributor

nahog commented Apr 16, 2021

Issue Description

When using underscored column names in pg enum array types, bulkUpdate queries fails (Actually this will happen for any field uses the "field" property to rename the column, as the data-type file in the postgres dialect is not using the correct "overwritten" propery)

What are you doing?

Here is the link to the SSCCE for this issue: sequelize/sequelize-sscce#158

What do you expect to happen?

I want that when using a model with underscored columns on the table in a postgres database, custom types in array are casted to the proper type when doing queries (in particular bulkCreate)

What is actually happening?

For a simple model like:

        BookDetails.init({
            uniqueName: {
                type: Sequelize.DataTypes.STRING(100),
                unique: true,
                allowNull: false,
                primaryKey: true
            },
            originalCategories: {
                type: Sequelize.DataTypes.ARRAY(Sequelize.DataTypes.ENUM({
                  values: ['drama', 'comedy'],
                })),
              },
        }, {
            underscored: true,
            modelName: 'BookDetails',
            sequelize
        });

when executing the bulkCreate I'm expecting that the data is inserted, but instead fails

        await BookDetails.bulkCreate([{
            uniqueName: 'test',
            originalCategories: ['drama']
        }]);

the error output is clear

INSERT INTO "book_details" ("unique_name","original_categories","created_at","updated_at") VALUES ('test',ARRAY['drama']::"enum_book_details_originalCategories"[],'2021-04-16 19:38:40.298 +00:00','2021-04-16 19:38:40.298 +00:00') RETURNING "unique_name","original_categories","created_at","updated_at";

enum_book_details_originalCategories is not the proper type in the database, it should be enum_book_details_original_categories respecting the underscoring.

Additional context

Environment

  • Sequelize version: 6.6.2
  • Node.js version: v12.22.1
  • Operating System: Ubuntu 20.04

Issue Template Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.
  • I think this problem happens only for the following dialect(s): postgres
  • I don't know, I was using PUT-YOUR-DIALECT-HERE, with connector library version XXX and database version XXX

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.

Dev notes

I locally patched my code with a simple sed command:
sed -i 's/options\.field\.fieldName/options\.field\.field/g' ./node_modules/sequelize/lib/dialects/postgres/data-types.js

And from what I followed in the code is just a change on line 483 at lib/dialects/postgres/data-types.js instead of
Utils.generateEnumName(options.field.Model.getTableName(), options.field.fieldName), the code should read Utils.generateEnumName(options.field.Model.getTableName(), options.field.field), (I debuged it to confirm and the sed patch works locally).

So the change is realively easy, my only concern is extra testing that may be needed.

nahog added a commit to nahog/sequelize that referenced this issue Apr 16, 2021
@papb
Copy link
Member

papb commented Jun 26, 2021

Congratulations. This was one of the best issue reports I've seen in a while. Sorry to take so long to give you the deserved attention!! Thank you very very much!

This could become a textbook example of well written issue. 💯

@papb papb added dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). Great SSCCE This issue has a great SSCCE/MCVE/reprex posted and therefore deserves extra attention! :) status: understood For issues. Applied when the issue is understood / reproducible. type: bug labels Jun 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). Great SSCCE This issue has a great SSCCE/MCVE/reprex posted and therefore deserves extra attention! :) status: understood For issues. Applied when the issue is understood / reproducible. type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants