metabase-clickhouse-driver icon indicating copy to clipboard operation
metabase-clickhouse-driver copied to clipboard

Clickhouse timezone query is not effective

Open fangpeng-lingoace opened this issue 2 years ago • 24 comments

metabase version: 0.41.2 metabase clickhouse driver version: 0.8.0

image

metabase REPORT TIMEZONE set Asia/Shanghai

image

using clickhouse CLI query is normal

image

fangpeng-lingoace avatar Dec 08 '21 10:12 fangpeng-lingoace

This looks like a bug, but I am not sure. The timestamp is the same, and Metabase uses a single timezone to format the timestamps. I am going to investigate further.

See Timezones troubleshooting guide in the Metabase documentation.

Both now() and now('Asia/Shanghai') produce a type/DateTime in Metabase.

enqueue avatar Dec 08 '21 12:12 enqueue

Sorry to ping you via github directly @flamber ! I hope you'll have a minute or so to help me. The user selects the same datetime instants formatted for different timezones. The driver code reports these columns as type/DateTime. Do you know if the current behavior, i.e. the normalization of both values to a report or JVM timezone, is expected or not from a Metabase point of view? Have you encountered this question before? Is there a "canonical" way for a driver developer to handle this? Any help is appreciated.

enqueue avatar Dec 08 '21 12:12 enqueue

@enqueue No worries, I'll try to provide as much insight as I can.

Which of the official drivers does your driver matches most? Then I don't see something like this, but I don't know what timezone support you have: https://github.com/metabase/metabase/blob/master/modules/drivers/redshift/src/metabase/driver/redshift.clj#L124-L126

Timezones are generally some of the most difficult parts - and each database behaves different, and it even depends on column types.

So when I'm debugging, then I usually get all timezone information, since it's otherwise just guessing.

  1. "Diagnostic Info" from Admin > Troubleshooting - contains both JVM timezone and Metabase Reporting Timezone
  2. The database connection timezone - this is normally just the timezone of the database, but some databases allow adjusting via the Metabase > Admin > Databases > (db) > Additional Connection String On Postgres or MySQL I would do a select-query in Metabase to get the session variables
  3. Then depending on the specific problem, I would also request the actual column types, since certain column types does not support timezones and would not display adjusted values (e.g. Postgres timestamp vs timestamptz)
  4. And then I would do something like select current_timestamp, current_timestamp::text, so I can see if it is the frontend or backend, which is causing problems.

flamber avatar Dec 10 '21 11:12 flamber

Thanks for your help @flamber :heart:

Timezones are generally some of the most difficult parts

Life would be boring without timezones or leap secods :smile:

The driver is JDBC based. While ClickHouse still has some room for improvement when it comes to SQL compliance, I am using Postgresql as a benchmark (see below). Its compliance is top notch, and the Metabase driver is officially supported and probably among the more popular ones. I am going take a look at the Redshift driver you pointed at, as well.

  1. The screenshort showed report time zone to be "Asia/Shanghai". But it's a very good hint to inquire about the other details. @fangpeng-lingoace could you please find out the JVM timezone and the timezone of your ClickHouse server (e.g. by running SELECT timezone()).
  2. We don't have any session timezones. I was able to reproduce the behavior without any connection settings.
  3. The types returned by the database are both based on DateTime:
SELECT
    toTypeName(NOW()),
    toTypeName(NOW('Asia/Shanghai'))

Query id: eeffe675-bb2d-4a25-a328-73f175589dbd

┌─toTypeName(now())─┬─toTypeName(now('Asia/Shanghai'))─┐
│ DateTime          │ DateTime('Asia/Shanghai')        │
└───────────────────┴──────────────────────────────────┘

  1. When asking the database to return Strings, we do not have an issue.

Comparing with Postgresql database in Metabase, we clearly see a difference. This is what it looks like using Postgresql database:

SELECT
  NOW() AT TIME ZONE 'Asia/Shanghai' AS ts_shanghai,
  pg_typeof(NOW() AT TIME ZONE 'Asia/Shanghai') AS type_ts_shanghai,
  NOW() AS ts_default,
  pg_typeof(NOW()) AS type_ts_default;

postgres_timestamp_simple

With some explicit casting:

SELECT
  (NOW() AT TIME ZONE 'Asia/Shanghai')::timestamptz AS ts_shanghai,
  pg_typeof((NOW() AT TIME ZONE 'Asia/Shanghai')::timestamptz) AS type_ts_shanghai,
  (NOW())::timestamptz AS ts_default,
  pg_typeof((NOW())::timestamptz) AS type_ts_default;

postgres_timestamp_casted

These observations all relate to the native, SQL-based interface in Metabase. I think that the behavior is different when using the regular query builder.

enqueue avatar Dec 11 '21 09:12 enqueue

@enqueue I'm glad that you think of timezones+leaps as a challenge. For me it's closer to a headache, since I usually need to come up with solutions, which can be quite complicated.

It looked like you were close to Postgres, which is why I referenced Redshift, since that's a dependent of that too. Generally there's a lot I learn from looking at other/similar drivers. I know very little Clojure, I have other skills :robot: ;-)

The explicit casting seems soooo strange. Not quite sure what's going on there. When I do something like this on a Postgres at UTC or Redshift at UTC with Metabase JVM UTC + Report Timezone at PDT and browser at CEST

select
   current_timestamp as t1
  ,current_timestamp::text as t2
  ,current_timestamp AT TIME ZONE 'Asia/Shanghai' as t3
  ,(current_timestamp AT TIME ZONE 'Asia/Shanghai')::text as t4
  ,(current_timestamp AT TIME ZONE 'Asia/Shanghai')::timestamptz as t5
  ,(current_timestamp AT TIME ZONE 'Asia/Shanghai')::timestamptz::text as t6

image

And this is because the session timezone is being defined, so I would get equal results via GUI.

But if I run this against Snowflake at UTC with Metabase JVM CEST + Report Timezone at PDT and browser at CEST

select
   current_timestamp as t1
  ,current_timestamp::text as t2
  ,CONVERT_TIMEZONE('Asia/Shanghai', current_timestamp) as t3
  ,CONVERT_TIMEZONE('Asia/Shanghai', current_timestamp)::text as t4
  ,CONVERT_TIMEZONE('Asia/Shanghai', current_timestamp)::timestamptz as t5
  ,CONVERT_TIMEZONE('Asia/Shanghai', current_timestamp)::timestamptz::text as t6

image

That seems similar to your explicit casting. But there are some known timezone issues with Snowflake, specially when using SQL vs using the GUI.

flamber avatar Dec 11 '21 10:12 flamber

It has to do with the reporting or JVM timezone. The Postgresql driver sets a session timezone. This leads to results being reported in that timezone.

enqueue=> SET SESSION TIMEZONE to 'Europe/Berlin';
SET
enqueue=> SELECT
  (NOW() AT TIME ZONE 'Asia/Shanghai')::timestamptz AS ts_shanghai,
  (NOW())::timestamptz AS ts_default;
          ts_shanghai          |          ts_default           
-------------------------------+-------------------------------
 2021-12-11 22:27:01.414843+01 | 2021-12-11 15:27:01.414843+01
(1 row)

enqueue avatar Dec 11 '21 14:12 enqueue

Try adding this to your driver - would likely require a lot of testing, probably by a lot of people in various timezones, so you can catch any negative consequences.

(defmethod sql-jdbc.execute/set-timezone-sql :clickhouse
  [_]
  "SET SESSION TIMEZONE to %s;")

But with "Diagnostic Info" from OP, then I cannot tell you what the problem is. I have spend so many hours debugging timezone issues, but if I don't get the needed information, then I just close issues, since it's impossible to reproduce otherwise (and I don't want our developers to spend hours on trying to debug themselves without success too)

flamber avatar Dec 11 '21 16:12 flamber

Thank for your reply. The JVM timezone and the timezone of my ClickHouse server all are UTC. @enqueue

SELECT timezone()

Query id: 9f4a708b-9868-4ad8-a92f-659dd19f0d88

┌─timezone()─┐
│ UTC        │
└────────────┘

fangpeng-lingoace avatar Dec 13 '21 02:12 fangpeng-lingoace

@fangpeng-lingoace It would be very helpful if you always include "Diagnostic Info" from Admin > Troubleshooting. Not that I don't believe you, but I have just seen way too many that thought they used something different than they actually did.

flamber avatar Dec 13 '21 10:12 flamber

I found no clues in the Metabase log, I got the Metabase Diagnostic Info:

{
  "browser-info": {
    "language": "zh",
    "platform": "MacIntel",
    "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.54 Safari/537.36",
    "vendor": "Google Inc."
  },
  "system-info": {
    "file.encoding": "UTF-8",
    "java.runtime.name": "Java(TM) SE Runtime Environment",
    "java.runtime.version": "13.0.1+9",
    "java.vendor": "Oracle Corporation",
    "java.vendor.url": "https://java.oracle.com/",
    "java.version": "13.0.1",
    "java.vm.name": "Java HotSpot(TM) 64-Bit Server VM",
    "java.vm.version": "13.0.1+9",
    "os.name": "Linux",
    "os.version": "3.10.0-1160.41.1.el7.x86_64",
    "user.language": "en",
    "user.timezone": "UTC"
  },
  "metabase-info": {
    "databases": [
      "clickhouse"
    ],
    "hosting-env": "unknown",
    "application-database": "h2",
    "application-database-details": {
      "database": {
        "name": "H2",
        "version": "1.4.197 (2018-03-18)"
      },
      "jdbc-driver": {
        "name": "H2 JDBC Driver",
        "version": "1.4.197 (2018-03-18)"
      }
    },
    "run-mode": "prod",
    "version": {
      "date": "2021-11-09",
      "tag": "v0.41.2",
      "branch": "release-x.41.x",
      "hash": "ad599fd"
    },
    "settings": {
      "report-timezone": "Asia/Shanghai"
    }
  }
}

I opened the chrome debugger, initiated a query request, and found clues. I don't know if it is useful.

url:  /api/dataset

request payload:  
{"database":2,"native":{"query":"select now('Asia/Shanghai') as n;","template-tags":{}},"type":"native","parameters":[]}

response:
{"data":{"rows":[
["2021-12-13T10:28:07Z"]
],
"cols":[{"display_name":"n","source":"native","field_ref":["field","n",{"base-type":"type/DateTime"}],"name":"n","base_type":"type/DateTime","effective_type":"type/DateTime"}],"native_form":{"query":"select now('Asia/Shanghai') as n;","params":null},"results_timezone":"UTC","requested_timezone":"Asia/Shanghai","results_metadata":{"checksum":"2sq1wcvzvW6ZYLthXvj/7A==","columns":[{"name":"n","display_name":"n","base_type":"type/DateTime","effective_type":"type/DateTime","field_ref":["field","n",{"base-type":"type/DateTime"}],"semantic_type":null,"fingerprint":{"global":{"distinct-count":1,"nil%":0.0}}}]},"insights":null},
"database_id":2,"started_at":"2021-12-13T10:28:07.074068Z","json_query":{"database":2,"native":{"query":"select now('Asia/Shanghai') as n;","template-tags":{}},"type":"native","middleware":{"js-int-to-string?":true,"add-default-userland-constraints?":true}},"average_execution_time":null,"status":"completed","context":"ad-hoc","row_count":1,"running_time":12}

fangpeng-lingoace avatar Dec 13 '21 10:12 fangpeng-lingoace

@fangpeng-lingoace You'll definitely want to upgrade as soon as possible: https://github.com/metabase/metabase/security/advisories/GHSA-vmm4-cwrm-38rj And "upgrade" to Java 11: https://www.metabase.com/docs/latest/operations-guide/java-versions.html And migrate away from H2 if you're using Metabase in production: https://www.metabase.com/docs/latest/operations-guide/migrating-from-h2.html

flamber avatar Dec 13 '21 10:12 flamber

Thank you for your reminder, I will try to upgrade and deal with security issues. @flamber

fangpeng-lingoace avatar Dec 13 '21 10:12 fangpeng-lingoace

Hi @enqueue , I have the same problem. And this is the info:

{
  "browser-info": {
    "language": "zh-CN",
    "platform": "MacIntel",
    "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.110 Safari/537.36",
    "vendor": "Google Inc."
  },
  "system-info": {
    "file.encoding": "UTF-8",
    "java.runtime.name": "OpenJDK Runtime Environment",
    "java.runtime.version": "1.8.0_312-b07",
    "java.vendor": "Red Hat, Inc.",
    "java.vendor.url": "https://www.redhat.com/",
    "java.version": "1.8.0_312",
    "java.vm.name": "OpenJDK 64-Bit Server VM",
    "java.vm.version": "25.312-b07",
    "os.name": "Linux",
    "os.version": "3.10.0-1160.15.2.el7.x86_64",
    "user.language": "en",
    "user.timezone": "Asia/Shanghai"
  },
  "metabase-info": {
    "databases": [
      "h2",
      "postgres",
      "clickhouse"
    ],
    "hosting-env": "unknown",
    "application-database": "h2",
    "application-database-details": {
      "database": {
        "name": "H2",
        "version": "1.4.197 (2018-03-18)"
      },
      "jdbc-driver": {
        "name": "H2 JDBC Driver",
        "version": "1.4.197 (2018-03-18)"
      }
    },
    "run-mode": "prod",
    "version": {
      "tag": "v0.41.5",
      "date": "2021-12-16",
      "branch": "release-x.41.x",
      "hash": "fbfffc6"
    },
    "settings": {
      "report-timezone": "Asia/Shanghai"
    }
  }
}

select now() result is UTC

Folgerjun avatar Jan 04 '22 08:01 Folgerjun

Hi, @enqueue sorry, @ you again. How should i solve it? BUG? Or need other setting?

metabase version: v0.41.5 metabase-clickhouse-driver version: v0.8.1

select timezone() is Asia/Shanghai but select now() still is UTC time.

image image

Folgerjun avatar Jan 05 '22 08:01 Folgerjun

Seems like this might have been fixed in the upstream JDBC driver 0.3.2 https://github.com/ClickHouse/clickhouse-jdbc/releases/tag/v0.3.2 https://github.com/ClickHouse/clickhouse-jdbc/issues/680

flamber avatar Jan 05 '22 08:01 flamber

@flamber @enqueue I changed driver version to v0.7.5, and select now() no probleam... 😂

Folgerjun avatar Jan 05 '22 10:01 Folgerjun

The same situation here. I was on meta 0.39.4 and tried upgrade to 40.2: If I use 0.7.5 driver - an issue with enum If i use new driver - this issue with timezone.

So there is no clear way I see to use more fresh version.

ADovgalyuk avatar Feb 28 '22 15:02 ADovgalyuk

We having a same issue. Thanks for the clue. We have installed v0.7.5 and select now() works as expected. We will stay on that version because we don't use enum. And it looks like there are no other side effects with v0.7.5 and the latest Metabase.

m4ex avatar Apr 07 '22 10:04 m4ex

when will release a new version to fix this ?

jovi-Li avatar Apr 09 '22 06:04 jovi-Li

Seems like this might have been fixed in the upstream JDBC driver 0.3.2 https://github.com/ClickHouse/clickhouse-jdbc/releases/tag/v0.3.2 ClickHouse/clickhouse-jdbc#680

I can confirm that building this plugin with 0.3.2 jdbc driver fixed the issue.

Steps to build by following the github workflow in this repo.

  • Clone metabase repo.
  • Copy contents of this repo into metabase/modules/drivers/clickhouse.
  • Change metabase/modules/drivers/clickhouse/deps.edn to use 0.3.2
  • Change metabase/modules/drivers/clickhouse/resources/metabase-plugin.yaml and replace ru.yandex.clickhouse.ClickHouseDriver -> com.clickhouse.jdbc.ClickHouseDriver under register-jdbc-driver step.
  • Add metabase/clickhouse {:local/root "clickhouse"} to metabase/modules/drivers/deps.edn
  • bin/build-driver.sh clickhouse from metabase folder. That should give us a jar file at metabase/resources/modules/clickhouse.metabase-driver.jar

joeirimpan avatar Apr 18 '22 10:04 joeirimpan

Any plan to fix this issue?

FesonX avatar May 19 '22 06:05 FesonX

@joeirimpan Thx, it works

FesonX avatar May 27 '22 10:05 FesonX

Thanks @joeirimpan for the workaround. I started trying to integrate the new JDBC driver a couple of weeks ago, but progress stalled. There are some incompatible changes, so many tests became red. However, if you are not affected, you are of course free to build your own.

enqueue avatar May 30 '22 05:05 enqueue

@enqueue any updates to fix this?

yinheli avatar Jul 07 '22 03:07 yinheli

@flamber @enqueue I changed the driver version to v0.7.5 and select now() has no problem... +1

CharesLee avatar Oct 19 '22 09:10 CharesLee

Should be fixed in 0.9.0

slvrtrn avatar Dec 07 '22 11:12 slvrtrn