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

Permit force DATETIME timezone #92

Closed
ghost opened this issue Dec 2, 2019 · 7 comments
Closed

Permit force DATETIME timezone #92

ghost opened this issue Dec 2, 2019 · 7 comments

Comments

@ghost
Copy link

ghost commented Dec 2, 2019

I am using the ORM mikro-orm and it has support to use mariadb as connection driver. The author of the ORM has implemented a new feature to permit a DATETIME timezone forced. It work with all SQL drivers (mysql2, pg and sqlite3) but it has troubles with mariadb.

The author of mikro-orm indicates the next: mikro-orm/mikro-orm#181 (comment)

The mariadb driver is unfortunately buggy there, it throws an error when the timezone config option is provided (at runtime, the value passes their internal validation).

Would it be possible to solve it?

@B4nan
Copy link

B4nan commented Dec 2, 2019

Hello there, just to add a bit more details, here is the stacktrace after settings timezone: 'Z':

TypeError: opts.tz(...).clone(...).tz(...).format is not a function

    at PacketNodeEncoded.readDateTime (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/io/packet.js:345:12)
    at Query.readRowData (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/cmd/common-text-cmd.js:328:23)
    at Query.parseRowStd (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/cmd/common-text-cmd.js:256:39)
    at Query.readResultSetRow (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/cmd/resultset.js:375:22)
    at PacketInputStream.receivePacketBasic (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/io/packet-input-stream.js:104:9)
    at PacketInputStream.onData (/usr/local/var/www/b4nan/mikro-orm/node_modules/mariadb/lib/io/packet-input-stream.js:160:20)
    at Socket.emit (events.js:210:5)
    at Socket.EventEmitter.emit (domain.js:475:20)
    at addChunk (_stream_readable.js:309:12)
    at readableAddChunk (_stream_readable.js:290:11)
    at Socket.Readable.push (_stream_readable.js:224:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:182:23)

@CReimer
Copy link

CReimer commented Dec 3, 2019

I have the timezone: 'Z' problem, too. Just want to add a few observation I made to work around it and where it comes from.

Workaround:

  • Downgrading to 2.0.3 fixes this problem. It's somehow related to d24c8ee
  • It only seems to appear when the first parameter of .query() is an object (QueryOptions) instead of a string
  • It happens when any other timezone than the local timezone is selected

Cause:

@Hellek
Copy link

Hellek commented Jan 3, 2020

Same problem here ("mariadb": "2.1.4")

/node_modules/mariadb/lib/io/packet.js:345
          .format('YYYY-MM-DD HH:mm:ss.SSSSSS')
           ^
TypeError: opts.tz(...).clone(...).tz(...).format is not a function
    at PacketNodeEncoded.readDateTime (/node_modules/mariadb/lib/io/packet.js:345:12)
    at Query.readRowData (/node_modules/mariadb/lib/cmd/common-text-cmd.js:328:23)
    at Query.parseRowStd (/node_modules/mariadb/lib/cmd/common-text-cmd.js:256:39)
    at Query.readResultSetRow (/node_modules/mariadb/lib/cmd/resultset.js:375:22)
    at PacketInputStream.receivePacketBasic (/node_modules/mariadb/lib/io/packet-input-stream.js:104:9)    at PacketInputStream.onData (/node_modules/mariadb/lib/io/packet-input-stream.js:160:20)
    at Socket.emit (events.js:210:5)
    at addChunk (_stream_readable.js:309:12)
    at readableAddChunk (_stream_readable.js:290:11)
    at Socket.Readable.push (_stream_readable.js:224:10)

When trying to set timezone

const pool = require('mariadb').createPool({
	database: env.database.name,
	user: env.database.user,
	password: env.database.pass,
	timezone: 'Z',
	namedPlaceholders: true,
})

@Hellek
Copy link

Hellek commented Jan 3, 2020

  • It only seems to appear when the first parameter of .query() is an object (QueryOptions) instead of a string

Tnx, it looks like a temp solution

// has a problem
const res = await DB.query({
	sql: 'SELECT registered FROM users WHERE id=:id',
}, { id })

// has no problem
const res = await DB.query('SELECT registered FROM users WHERE id=:id', { id })

@mafischer
Copy link
Contributor

mafischer commented Feb 24, 2020

adding dateStrings: true to the connection config seems to be a viable workaround for my use case.

it looks like the problem stems from the value localTz being undefined here. I'm not sure why that value is undefined, I'm guessing the this context is getting messed up somewhere down the line.

@rusher this seems to be a fairly significant bug as it is causing the application to crash. Not sure if you've had time to look into it.

@rusher
Copy link
Collaborator

rusher commented Mar 10, 2020

reproduced. the @mafischer PR fix the problem.
This will be release in next version

@B4nan
Copy link

B4nan commented Mar 22, 2020

Trying this now with the latest version (2.3.1), the error is indeed gone, but looks like the timezone option does not have any effect.

Here is the test case, having set timezone: 'Z', alto tried to set different timezones, nodejs is running in CET (=UTC+1):

test('datetime is stored in correct timezone', async () => {
  const author = new Author2('n', 'e');
  author.born = new Date('2000-01-01T00:00:00Z');
  await orm.em.persistAndFlush(author);
  orm.em.clear();

  const res = await orm.em.getConnection().execute<{ born: string }[]>(`select date_format(born, '%Y-%m-%d %T.%f') as born from author2 where id = ${author.id}`);
  expect(res[0].born).toBe('2000-01-01 00:00:00.000000');
  const a = await orm.em.findOneOrFail(Author2, author.id);
  console.log('a.born', a.born);
  console.log('author.born', author.born);
  expect(+a.born!).toBe(+author.born); // <--- fails
});

console.log tests/EntityManager.mariadb.test.ts:70
    a.born 1999-12-31T23:00:00.000Z

console.log tests/EntityManager.mariadb.test.ts:71
    author.born 2000-01-01T00:00:00.000Z

Error: expect(received).toBe(expected) // Object.is equality

Expected: 946684800000
Received: 946681200000

As I noted above, I tried to set different timezones, yet the results are always the same.

Note that mariadb here is controlled via knex, so the issue might be also somewhere in the middle, but for mysql2 driver it works ok (and in MikroORM those drivers are pretty much identical, apart from the database connector used inside knex).

edit: one thing I should add is that I am testing the mariadb connector with vanilla mysql database (official docker image).

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

Successfully merging a pull request may close this issue.

5 participants