elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.9k stars 24.73k forks source link

SQL: Perform type verification for comparison operators #49636

Open costin opened 4 years ago

costin commented 4 years ago

Currently BinaryOperators only check whether both types are resolved yet it doesn't check whether the types are actually compatible. This means 123 > 'foo' is allowed despite being a numeric vs string comparison which is incorrect.

elasticmachine commented 4 years ago

Pinging @elastic/es-search (:Search/SQL)

matriv commented 4 years ago

Maybe as a bonus check if the Postgres behaviour could be followed:

postgres=# select 123 > 'foo';
ERROR:  invalid input syntax for integer: "foo"
LINE 1: select 123 > 'foo';
                     ^
postgres=# select 123 > '222';
 ?column?
----------
 f
(1 row)

postgres=# select 123 > '111';
 ?column?
----------
 t
(1 row)
bpintea commented 4 years ago

postgres=# select 123 > '222';

@matriv Does that work in where clauses too, like select ... where int_column > '222'? On the flip side, ES accepts integers in range queries on text/keyword fields (i.e. select ... from test_emp where first_name > 1), but not sure if that makes sense for SQL?

bpintea commented 4 years ago

We probably still want to support queries like these: SELECT ... FROM ... WHERE date > '1969-05-13', so if the literal can be converted to the other operand's type, the comparison should be allowed (as suggested above).

But the following tests, currently asserting success, would need to be changed/dropped:

matriv commented 4 years ago

@matriv Does that work in where clauses too, like select ... where int_column > '222'? On the flip side, ES accepts integers in range queries on text/keyword fields (i.e. select ... from test_emp where first_name > 1), but not sure if that makes sense for SQL?

postgres=# create table t(a int);
CREATE TABLE
postgres=# insert into t values(1),(2),(3),(4),(111),(222);
INSERT 0 6
postgres=# select * from t where a > '111';
  a
-----
 222
(1 row)

postgres=# select * from t where a > '3';
  a
-----
   4
 111
 222
(3 rows)
matriv commented 4 years ago

@elastic/es-ql

elasticsearchmachine commented 9 months ago

Pinging @elastic/es-analytical-engine (Team:Analytics)