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

Datetime bug in query builder #3461

Open
chaffeqa opened this issue Oct 2, 2019 · 5 comments
Open

Datetime bug in query builder #3461

chaffeqa opened this issue Oct 2, 2019 · 5 comments
Labels

Comments

@chaffeqa
Copy link
Contributor

chaffeqa commented Oct 2, 2019

Environment

Knex version: 0.18.3
Database + version: mysql Ver 8.0.15 for osx10.14 on x86_64 (Homebrew)
OS: osx10.14

Bug

Simple query involving Date object seems to fail to pass compilation, though toString() generates valid SQL

knex("Entry").count("*").where("createdAt", ">", new Date(nowAt - oneMinute)).toSQL().toNative()
{
  sql: 'select count(*) from `Entry` where `createdAt` > ?',
  bindings: [ 2019-10-01T23:24:23.872Z ]
}
knex("Entry").count("*").where("createdAt", ">", new Date(nowAt - oneMinute)).toString()
'select count(*) from `Entry` where `createdAt` > "2019-10-01T23:24:23.872Z"'
await knex("Entry").count("*").where("createdAt", ">", new Date(nowAt - oneMinute)).debug()
{
  method: 'select',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [ 2019-10-01T23:24:23.872Z ],
  __knexQueryUid: 'b24ffdbd-162c-4a0a-a35e-5fecb9cc2b3e',
  sql: 'select count(*) from `Entry` where `createdAt` > ?'
}
Thrown:
Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1
    at createQueryBuilder (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/util/make-knex.js:257:26)
    at DataLayerEmails.knex (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/util/make-knex.js:13:12)
    at repl:1:40
    at repl:2:4
    at Script.runInContext (vm.js:137:20)
    at REPLServer.defaultEval (repl.js:386:29)
    at bound (domain.js:420:14)
    at REPLServer.runBound [as eval] (domain.js:433:12)
    at REPLServer.onLine (repl.js:700:10)
    at REPLServer.emit (events.js:214:15)
    at REPLServer.EventEmitter.emit (domain.js:476:20)
    at REPLServer.Interface._onLine (readline.js:316:10) {
  code: 'ER_PARSE_ERROR',
  errno: 1064,
  sqlState: '42000',
  sqlMessage: "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1",
  originalStack: "Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1\n" +
    '    at Packet.asError (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/packets/packet.js:708:17)\n' +
    '    at Query.execute (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/commands/command.js:28:26)\n' +
    '    at Connection.handlePacket (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/connection.js:408:32)\n' +
    '    at PacketParser.onPacket (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/connection.js:70:12)\n' +
    '    at PacketParser.executeStart (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/packet_parser.js:75:16)\n' +
    '    at Socket.<anonymous> (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/mysql2/lib/connection.js:77:25)\n' +
    '    at Socket.emit (events.js:209:13)\n' +
    '    at Socket.EventEmitter.emit (domain.js:476:20)\n' +
    '    at addChunk (_stream_readable.js:305:12)\n' +
    '    at readableAddChunk (_stream_readable.js:286:11)\n' +
    '    at Socket.Readable.push (_stream_readable.js:220:10)\n' +
    '    at TCP.onStreamRead (internal/stream_base_commons.js:182:23)\n' +
    'From previous event:\n' +
    '    at Client_MySQL2._query (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/dialects/mysql/index.js:125:12)\n' +
    '    at Client_MySQL2.query (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/client.js:158:17)\n' +
    '    at Runner.query (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/runner.js:136:36)\n' +
    '    at /Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/runner.js:40:23\n' +
    'From previous event:\n' +
    '    at Runner.run (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/runner.js:26:16)\n' +
    '    at Builder.Target.then (/Users/quinnchaffee/code/cbs/fantasy-picks/node_modules/knex/src/interface.js:14:43)\n' +
    '    at processTicksAndRejections (internal/process/task_queues.js:93:5)'
}
@elhigu
Copy link
Member

elhigu commented Oct 2, 2019

Yep, looks like a bug. Knex should not change Date() to string.

https://runkit.com/embed/ai9im80r6iin

Parameter should be left untouched by knex and passed as Date object to the driver.

@elhigu elhigu added the bug label Oct 2, 2019
@Fylipp
Copy link

Fylipp commented Oct 3, 2019

@elhigu When querying an Oracle database the parameter value becomes null. This completely breaks the Oracle compatibility for me. Is this an issue that can be resolved swiftly? This is a complete deal-breaker for my use case.

@chaffeqa
Copy link
Contributor Author

chaffeqa commented Oct 3, 2019

FYI after more debugging, i think its related to something babel is doing in transpiling... may be doing something that breaks detection of Date object?

@chaffeqa
Copy link
Contributor Author

chaffeqa commented Oct 3, 2019

@Fylipp I was able to get around it by doing:

await knex("Entry").count("*").where("createdAt", ">", (new Date(nowAt - oneMinute)).toISOString())

@elhigu
Copy link
Member

elhigu commented Oct 3, 2019

@chaffeqa Yep... I never have used date object in bindings either. I always pass dates as iso strings.

@elhigu When querying an Oracle database the parameter value becomes null. This completely breaks the Oracle compatibility for me. Is this an issue that can be resolved swiftly? This is a complete deal-breaker for my use case.

Doesn't seem like null to me... https://runkit.com/embed/nbtrv0kj944a

This is not a priority to fix, but pull requests are welcome as usual.

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

No branches or pull requests

3 participants