wvanbergen/vertica

Invalid quoting of DateTime and Time values

Closed this issue · 4 comments

maxd commented

Look at this line. As you can see this code convert original value to string and cast to timestamp datatype.

I see the following problems:

  1. It is loose information about timezone.
  2. It is force Vertica to using system timezone. If I pass value in +0300 timezone, but system timezone was set to +0400 Vertica will write value in +0400 timezone to timestamptz column.

How about just convert date time values to ISO8601 (which is de facto standard for present date and time) and remove cast to timestamp datatype?

P.S. I am ready to provide PR if somebody review and approve it.

wvanbergen commented

This makes sense to me.

  • I am a bit afraid of breaking backwards compatibility though What happens when you provide a timestamptz value to a timestamp column? How does that compare to the current behavior?
  • We should probably add some documentation around timestamps and how they are handled. This can be very confusing.
maxd commented

What happens when you provide a timestamptz value to a timestamp column?

... If a time zone is specified in the input it is silently ignored. - from here

How does that compare to the current behavior?

It is equal to current behavior with timestamp column because quote silently ignored specified timezone.

I am a bit afraid of breaking backwards compatibility

Seems like this is doesn't break backward compatibility (as explained above) but fix bug with timestamptz columns.

wvanbergen commented

👍, feel free to open up a pull requests, with some tests that check the new behavior is working as expected.

wvanbergen commented

Potentially relevant: #35