Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Perform type verification for comparison operators #49636

Open
costin opened this issue Nov 27, 2019 · 7 comments
Open

SQL: Perform type verification for comparison operators #49636

costin opened this issue Nov 27, 2019 · 7 comments
Labels
:Analytics/SQL SQL querying >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@costin
Copy link
Member

costin commented Nov 27, 2019

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.

@costin costin added >bug :Analytics/SQL SQL querying labels Nov 27, 2019
@elasticmachine
Copy link
Collaborator

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

@matriv
Copy link
Contributor

matriv commented Nov 27, 2019

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
Copy link
Contributor

bpintea commented Jan 27, 2020

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
Copy link
Contributor

bpintea commented Jan 28, 2020

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
Copy link
Contributor

matriv commented Jan 30, 2020

@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
Copy link
Contributor

matriv commented Mar 30, 2020

@elastic/es-ql

@rjernst rjernst added the Team:QL (Deprecated) Meta label for query languages team label May 4, 2020
@wchaparro wchaparro removed the Team:QL (Deprecated) Meta label for query languages team label Jan 17, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

No branches or pull requests

7 participants