AlexR2D2 / metabase_duckdb_driver

Metabase DuckDB Driver shipped as 3rd party plugin
Apache License 2.0
75 stars 25 forks source link

Cannot filter date values with 0.10.0 update #24

Closed nfire11 closed 7 months ago

nfire11 commented 8 months ago

I updated the driver to 0.10.0, but met some error in filtering datetime values. I have checked sql script auto-generated by metabase when applying a date filter :

where datetime >= timestamp with time zone '2023-01-01 00:00:00.000Z'

It works fine in 0.9.1 but would return an error which asks for a type conversion in 0.10.0 If mannually change the type to timestamp without timezone, it works. where datetime >= timestamp without time zone '2023-01-01 00:00:00.000Z'

Any idea about fixing that?

AlexR2D2 commented 8 months ago

Hmm... i tried it in the Met abase 1.48.7 with 0.10.0, it seems that both variants work

Screenshot 2024-03-05 at 00 09 45
nfire11 commented 8 months ago

Hi Alex! Nice work for developing the driver. It will happen when you apply a datetime filter. image

Which requires an explicit type conversion for datetime comparisions (timestamp without time zone VS timestamp with time zone) This might be a feature in duckdb 0.10? I complied the driver with duckdb 0.9.2 works..

script generated when applying a filter:


  (
    "public"."gset"."ndt_datetime" >= timestamp with time zone '2024-01-01 00:00:00.000+08:00'
  )

   AND (
    "public"."gset"."ndt_datetime" < timestamp with time zone '2024-03-06 00:00:00.000+08:00'
  ) ```
K377U commented 8 months ago

Im seeing this timezone cast problem with timestamps and dates, probably related to this: https://duckdb.org/2024/02/13/announcing-duckdb-0100.html#breaking-sql-changes

Binder Error: Cannot compare values of type DATE and type TIMESTAMP WITH TIME ZONE - an explicit cast is required

Binder Error: Cannot compare values of type TIMESTAMP and type TIMESTAMP WITH TIME ZONE - an explicit cast is required

K377U commented 8 months ago

I did some testing and this can be fixed by setting SET old_implicit_casting = true;. Would this be possible at driver level? If so would it be possible to add some other configurations too from environment variables?

nfire11 commented 8 months ago

I did some testing and this can be fixed by setting SET old_implicit_casting = true;. Would this be possible at driver level? If so would it be possible to add some other configurations too from environment variables?

Thanks, Roope. I complied the 0.10.0 driver with the settings. It works! You can put extra configuration variables in the plugin source code. Maybe it would be helpful to add an optional field in the plugin to set connection properties?

AlexR2D2 commented 7 months ago

Hi, as lib maintainer I can't make decisions for the developer do you need the old_implicit_casting as true or false, so i added the settings on db settings page