SQLAlchemy DQL: Use CrateDB's native ILIKE operator

[amotl]

About

At crate/sqlalchemy-cratedb#98, @hlcianfagna suggested to use CrateDB's native ILIKE operator instead of using SA's generic implementation lower() LIKE lower().

Details

This patch brings in the corresponding visitor methods from the PGDialect to the CrateDialect implementation.

[amotl]

Does CrateDB support ESCAPE in this context? Do you suggest to remove this code, if not?

[hlcianfagna]

No,

UnsupportedFeatureException[ESCAPE is not supported.]

Maybe we should raise a feature request?

How were % and _ handled previously in the lower() like lower() implementation?

[matriv]

Escape character for _ and % in CrateDB is \, so \%' or _`:

cr> create table t(s string);
CREATE OK, 1 row affected  (0.385 sec)
cr> insert into t values('f%oo');
INSERT OK, 1 row affected  (0.062 sec)
cr> refresh table t;
REFRESH OK, 1 row affected  (0.005 sec)
cr> select * from t where s ilike 'f\%O%';
+------+
| s    |
+------+
| f%oo |
+------+
SELECT 1 row in set (0.003 sec)

I don't get what's this ESCAPE here, can you please explain?

[amotl]

I was hoping that you can explain, and/or follow up with proper guidance. ;]

[amotl]

Shall we just remove the corresponding code, in order to be able to bring in the patch, and create an issue at crate/crate to track concerns of the ESCAPE operator/keyword? I guess it can also be used on statements/clauses other than this specific spot?

[matriv]

Yes please, it seems that postgres supports defining a custom escape char seq:
https://www.postgresql.org/docs/15/functions-matching.html
https://stackoverflow.com/questions/196626/escaping-in-a-ilike-query
which is not the case for CrateDB.

[amotl]

Thank you. I've just removed corresponding code. Please let me know if there should be an issue on the crate/crate tracker, in order to track any missing compatibility features.

[hlcianfagna]

See #564 (comment) about the handling of % and _ in the matching string

[matriv]

Thx!

[hlcianfagna]

I confirmed that ilike accepts an escape parameter and if that is used today it renders as the SQL ESCAPE clause which results on an UnsupportedFeatureException being returned from CrateDB, if the change goes in as is there would be no error but we would be risking incorrect results, maybe we should raise some exception if the escape parameter is used?

[matriv]

I confirmed that ilike accepts an escape parameter and if that is used today it renders as the SQL ESCAPE clause which results on an UnsupportedFeatureException being returned from CrateDB, if the change goes in as is there would be no error but we would be risking incorrect results, maybe we should raise some exception if the escape parameter is used?

Very true, thx! I agree to throw an exception if escape is used.

[amotl]

@hlcianfagna: Well spotted, thank you very much. I agree to catch the escape argument, but not ignore it, and raise a synthetic exception instead, mimicking the one which is currently raised by CrateDB when actually sending the ESCAPE keyword.

[amotl]

Can we find out since which version CrateDB supports the ILIKE operator, and if it would be advisable that we need to fall back to the generic SQLAlchemy implementation, when operating on a lower version of CrateDB, like, not breaking support for CrateDB 3.x?

[hlcianfagna]

Can we find out since which version CrateDB supports the ILIKE operator, and if it would be advisable that we need to fall back to the generic SQLAlchemy implementation, when operating on a lower version of CrateDB, like, not breaking support for CrateDB 3.x?

Support for ILIKE was added in 4.1.0

[amotl]

Support for ILIKE was added in 4.1.0.

Thank you @hlcianfagna. I've just added b93007d, in order to implement corresponding code branching based on the CrateDB version in use.

May I ask for another review from you and @matriv?