ClickHouse / metabase-clickhouse-driver

ClickHouse database driver for the Metabase business intelligence front-end
Apache License 2.0
461 stars 84 forks source link

issue https://github.com/ClickHouse/metabase-clickhouse-driver/issues/224 happened again. #248

Closed bladexxx closed 2 weeks ago

bladexxx commented 2 weeks ago

Describe the bug

issue https://github.com/ClickHouse/metabase-clickhouse-driver/issues/224 happened again. image

Steps to reproduce

Expected behaviour

Error log

Configuration

Environment

ClickHouse server

slvrtrn commented 2 weeks ago

Steps to reproduce

This is important. The CI tests with an older version of ClickHouse (23.3) pass, which means that the correct function (startsWith instead of startsWithUTF8) is picked there.

Regardless, the current LTS version of ClickHouse is 23.8; please consider upgrading your ClickHouse deployment, as it is EOL already.

bladexxx commented 2 weeks ago

@slvrtrn thanks for your quick reply. is there any workaround way for this issue If we don't have any plans to upgrade at the moment? 22.8.21.38 is LTS version too.

bladexxx commented 2 weeks ago

@slvrtrn I am confused, seems below compatibility fix was included since 1.3.4. so has it been removed from version 1.5.1/1.5.0 ?

image

slvrtrn commented 2 weeks ago

@bladexxx, I did not remove anything. I even have a CI workflow to test the driver with 23.3 - https://github.com/ClickHouse/metabase-clickhouse-driver/blob/master/.github/workflows/check.yml#L160-L165, which also runs the string filters tests that use either startsWith/startsWithUTF8, depending on the ClickHouse version.

slvrtrn commented 2 weeks ago

I just tested it with both 22.8 and 23.3. The generated queries were correct (lowerUTF8 is from <2022, but startsWithUTF8 was not used; startsWith was picked instead, as the query processor correctly failed the version check).

-- Metabase:: userID: 1 queryType: MBQL queryHash: 2f9ad0d6591210b7445381c1738d340759576fd276f167b61f87b2b26316e458
SELECT `default`.`foo`.`id` AS `id`, `default`.`foo`.`s` AS `s` FROM `default`.`foo` 
WHERE `startsWith`(`lowerUTF8`(`default`.`foo`.`s`), `lowerUTF8`('Ш')) LIMIT 2000                                                                                                                                                                                         

The CI is green, too. The relevant part of the code is https://github.com/ClickHouse/metabase-clickhouse-driver/blob/1.5.0/src/metabase/driver/clickhouse_qp.clj#L288-L293, which seems to be working.

Regarding 22.8: likely, you won't be able to use 1.50.0+ with it cause the driver enforces allow_experimental_analyzer=0 since that release, which was not yet an option as of 22.8, so that puts 23.3 as a minimal best effort supported version, while officially it is 23.8+, as 23.8 is the current LTS.

I will update the README regarding that.

22.8.21.38 is LTS version too.

It was, but its support has ended almost one year ago. 23.3 support ended a few months ago. The current LTS is 23.8. See the official docs.

lts are released twice a year and are supported for a year after their initial release.


As much as I'd like to support outdated ClickHouse versions, both Metabase and ClickHouse evolve rather quickly; we simply cannot take all the edge cases that might occur with older versions into account, as this becomes increasingly difficult and time consuming to fix and test.