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

DATE parse function crashes in SQLite #17287

Closed
3 of 6 tasks
Skywt2003 opened this issue Apr 16, 2024 · 1 comment
Closed
3 of 6 tasks

DATE parse function crashes in SQLite #17287

Skywt2003 opened this issue Apr 16, 2024 · 1 comment
Labels
pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: bug

Comments

@Skywt2003
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 SQLite, if a field stores a timestamp as a number and is set to type DATE, it will cause Sequelize to crash when performing findAll().

TypeError: date.includes is not a function

Reproducible Example

Here is the link to the SSCCE for this issue:

https://github.com/Skywt2003/sequelize-sscce

It seems that the problem is caused by the lack of type checking inside the data-types.js file. In detail, here's the relevant code:

// sequelize/lib/dialects/sqlite/data-types.js line 37-45
  class DATE extends BaseTypes.DATE {
    static parse(date, options) {
      if (!date.includes("+")) {
        return new Date(date + options.timezone);
      }
      return new Date(date);
    }
  }

Obviously, the parse function assumes the date argument is a string with includes method. However when date is a number, it crashes.

Maybe a toString() method should be added here. Referring to the MariaDB version of parse function in DATE:

// sequelize/lib/dialects/mariadb/data-types.js line 51-62
    static parse(value, options) {
      value = value.string();
      if (value === null) {
        return value;
      }
      if (momentTz.tz.zone(options.timezone)) {
        value = momentTz.tz(value, options.timezone).toDate();
      } else {
        value = new Date(`${value} ${options.timezone}`);
      }
      return value;
    }

What do you expect to happen?

Sequelize should handle timestamp data (in the format of number) correctly, at least without crashing.

What is actually happening?

Sequelize crashes when performing a findAll query.

TypeError: date.includes is not a function
    at parse (/[...]/node_modules/sequelize/lib/dialects/sqlite/data-types.js:40:17)
    at Query.applyParsers (/[...]/node_modules/sequelize/lib/dialects/sqlite/query.js:289:14)
    at /[...]/node_modules/sequelize/lib/dialects/sqlite/query.js:112:80
    at /[...]/node_modules/lodash/lodash.js:13469:38
    at /[...]/node_modules/lodash/lodash.js:4967:15
    at baseForOwn (/[...]/node_modules/lodash/lodash.js:3032:24)
    at Function.mapValues (/[...]/node_modules/lodash/lodash.js:13468:7)
    at /[...]/node_modules/sequelize/lib/dialects/sqlite/query.js:93:18
    at Array.map (<anonymous>)
    at Query._handleQueryResponse (/[...]/node_modules/sequelize/lib/dialects/sqlite/query.js:92:25)

Environment

  • Sequelize version: 6.37.1
  • Node.js version: v21.5.0
  • Database & Version: SQLite
  • Connector library & Version: sqlite3 5.1.7

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.

@Skywt2003 Skywt2003 added pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: bug labels Apr 16, 2024
@ephys
Copy link
Member

ephys commented Apr 16, 2024

This is basically the same issue as #16355 (comment)

The data type expects a string with the ISO format, which is what we insert. If you want to use this data type, you'll need to either either

  • migrate the data that is in your database to use the expected format,
  • make the attribute use DataTypes.INTEGER,
  • or create a custom data type

@Skywt2003 Skywt2003 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: bug
Projects
None yet
Development

No branches or pull requests

2 participants