MotherDuck-Open-Source / metabase_duckdb_driver

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

Relative dates TYPE CAST error #2

Open naxels opened 7 months ago

naxels commented 7 months ago

Hi team,

I'm excited to try the DuckDb driver for Metabase, however quickly ran into issues with the driver that upon double check doesn't occur when the DuckDb database is converted to Sqlite and the same is tried.

Reproduce sample:

CREATE TABLE transactions (
    bookdate DATE
);

INSERT INTO transactions VALUES ('2024-01-01');
INSERT INTO transactions VALUES ('2024-02-01');
INSERT INTO transactions VALUES ('2024-03-01');
INSERT INTO transactions VALUES ('2024-04-01');

which I've saved as sample.duckdb

Now in Metabase, load the sample database.

Then, exit out of Admin and click New -> Question and select Sample, transactions table.

Filter now the bookdate on Relative Dates -> Current -> Year (any other than Day)

Next you will be presented with an error: Binder Error: Cannot compare values of type DATE and type TIMESTAMP WITH TIME ZONE - an explicit cast is required

The generated SQL is:

SELECT
  "main"."transactions"."bookdate" AS "bookdate"
FROM
  "main"."transactions"
WHERE
  (
    "main"."transactions"."bookdate" >= DATE_TRUNC('year', NOW())
  )

   AND (
    "main"."transactions"."bookdate" < DATE_TRUNC(
      'year',
      (CAST(NOW() AS timestamp) + (INTERVAL '1' year))
    )
  )
LIMIT
  1048575

What I expected was like Sqlite to have the data presented.

For complete report, creating the same database in Sqlite, creates this query in Metabase:

SELECT
  "main"."transactions"."bookdate" AS "bookdate"
FROM
  "main"."transactions"
WHERE
  (
    "main"."transactions"."bookdate" >= DATE(DATETIME('now'), 'start of year')
  )

   AND (
    "main"."transactions"."bookdate" < DATE(
      DATETIME(DATETIME('now'), '+1 years'),
      'start of year'
    )
  )
LIMIT
  1048575

Thank you for looking into this!

hrl20 commented 7 months ago

Thanks for reporting this! this seems similar to https://github.com/AlexR2D2/metabase_duckdb_driver/issues/24 could you try enabling the old_implicit_casting option?

naxels commented 7 months ago

Thank you, that seems to work :) !

For future reference, here is the documentation from DuckDB: https://duckdb.org/docs/configuration/overview.html

However, I find it strange that this setting isn't on by default and/or there is more explanation as to what this setting does.

In my opinion, the date filters are an essential feature of Metabase.

hrl20 commented 6 months ago

Thanks for the input @naxels. in the latest version i've added explicit casting for fields that are used in date/times filters, such that old_implicit_casting option is not required for this feature to work.

hrl20 commented 6 months ago

had a false start on this, so i've just enabled old_implicit_casting by default. will continue to look into making datetime filtering more robust.