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

Model.create() in MySQL does not return null value when defaultValue hasn't defined #14671

Open
3 of 6 tasks
mytharcher opened this issue Jun 22, 2022 · 6 comments
Open
3 of 6 tasks

Comments

@mytharcher
Copy link

Issue Creation Checklist

  • I understand that my issue will be automatically closed if I don't fill in the requested information
  • I have read the contribution guidelines

Bug Description

In MySQL, if not provide explicit defaultValue: null to the model definition, the created result will not contain the column key.

But in PG it work right (with the null value in result);

Reproducible Example

Here is the link to the SSCCE for this issue:

const db = new Sequelize('mysql://root@localhost/test');

const Book = db.define('Books', {
  title: {
    type: Sequelize.STRING,
    defaultValue: null
  },
  price: {
    type: Sequelize.INTEGER
  }
});

await db.sync({ force: true });

const created = await Book.create();
// in pg here the result will contain `price` key as `null` value
console.log(created.get()); // without `price` key

const found = await Book.findOne();
console.log(found.get()); // with `price` key as `null` value

What do you expect to happen?

{
  id: 1,
  title: null,
  price: null,
  createdAt: 2022-06-22T12:43:13.000Z,
  updatedAt: 2022-06-22T12:43:13.000Z
}

What is actually happening?

{
  title: null,
  id: 1,
  updatedAt: 2022-06-22T12:43:13.333Z,
  createdAt: 2022-06-22T12:43:13.333Z
}

Environment

  • Sequelize version: 6.11.0
  • Node.js version: v16.15.0
  • If TypeScript related: TypeScript version:
  • Database & Version: MySQL 8.0.28 & PostgresQL 14.3
  • Connector library & Version: mysql2@2.3.3 & pg@8.7.3

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 will need guidance.
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in resolving my issue.

Indicate your interest in the resolution of this issue by adding the 👍 reaction. Comments such as "+1" will be removed.

@WikiRik
Copy link
Member

WikiRik commented Jun 24, 2022

Do I understand correctly that your expectation is that created.get() equals found.get()?

@ephys
Copy link
Member

ephys commented Jun 24, 2022

note for me (or anyone who wants to pick this up) I think this could be easy to fix: In Model.normalizeAttributes, set the defaultValue to null if the attribute allows null and no default value has been specified

@mytharcher
Copy link
Author

@WikiRik Exactly! My bad for not describing it so clearly.

What my expectation is created.get() to contain null values which not explicit defined by defaultValue: null. And I only found this in MySQL, so it may be a dialect based bug.

@WikiRik
Copy link
Member

WikiRik commented Jun 24, 2022

No worries, your explanation was fine. Just wanted to double check. As @ephys mentioned it probably is an easy fix. I'll try to check it out over the weekend

@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2022

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Jul 9, 2022
@ephys ephys added type: bug and removed stale labels Jul 9, 2022
@WikiRik WikiRik removed their assignment Sep 12, 2023
@WikiRik
Copy link
Member

WikiRik commented Sep 12, 2023

As can be seen in the linked PR, there are a few things blocking that so this might not get fixed in the short term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants