ClickHouse/metabase-clickhouse-driver

Unnecessary CAST toDate for custom variables with time

ADovgalyuk opened this issue · 4 comments

While using variables with type "Field Filter", widget type "Date Filter" error occurred due to CAST to Date type.
In release notes for new driver i see "Removed superfluous CAST calls from generated queries that use Date* columns and/or intervals." which looks similar, but issue is still reproduced on last version.

Steps to reproduce

  1. New Sql Query
  2. Use variable with filter type "Date Filter"
  3. Choose one of options including smth related to Time, not only Date.
  4. Error is shown like (also presented on screenshot below):
Code: 53. DB::Exception: Cannot convert string 2023-10-03 18:05:00 to type Date: while executing 'FUNCTION greaterOrEquals(CAST(date_time, 'date') : 4, '2023-10-03 18:05:00' : 2) -> greaterOrEquals(CAST(date_time, 'date'), '2023-10-03 18:05:00') UInt8 : 5'. (TYPE_MISMATCH) (version 22.8.16.32 (official build)) , server ClickHouseNode [uri=http://192.168.109.237:8123/statistics, options={use_server_time_zone_for_dates=true,use_no_proxy=false,product_name=metabase/1.2.2}]@-1605831072

Expected behaviour

No errors, data on specified time interval is shown

Configuration

Environment

  • metabase-clickhouse-driver version: 1.2.2
  • metabase-clickhouse-driver configuration: no specific configuration
  • Metabase version: 0.47.2
  • OS:

ClickHouse server

  • ClickHouse Server version: 22.8
  • Any table with DateTime field.

query_preview
main_window

Thanks for the report.

In release notes for new driver i see "Removed superfluous CAST calls from generated queries that use Date* columns and/or intervals." which looks similar

Yes, this is correct, but for non-variable use cases. I am still to figure out where we can actually override the incorrect variables replacement.

I will look into it again early next week.

It looks like this is the same bug: metabase/metabase#21133
But, instead of losing performance like in that Postgres scenario, we cannot execute the query at all, as DateTime -> Date implicit upcast on the right side does not work.
I am investigating whether it will be feasible to introduce some hack around it, as it looks like a substantial amount of copy-paste from Metabase variables substitution sources is required, as almost everything is private methods in the related module.

After discussing my findings with the Metabase team, we agreed that this is blocked by Metabase currently.
We'd also like to avoid a massive copy-paste, especially from the project with a different license, and I will try my best to prepare a Metabase patch that makes that particular bit more driver-override-friendly, so we can resolve that unnecessary upcast in a more canonical way.
Stay tuned.

@ADovgalyuk, it is fixed in 1.3.0. You could try it with Metabase 0.48.0-RC1 now.