QL: move query AST from SQL to QL #52069
Conversation
|
Pinging @elastic/es-search (:Search/SQL) |
| if (e.foldable()) { | ||
| set.add(DataTypeConverter.convert(e.fold(), dt)); | ||
| } else { | ||
| throw new QlIllegalArgumentException("Cannot determine value for {}", e); |
There was a problem hiding this comment.
I guess this would be the case of something like:
SELECT * FROM test WHERE test.a = test.b
- Isn't this caught earlier?
- Should we add a test for this exception?
There was a problem hiding this comment.
This is the intermediate query AST - in this case terms will be more like A in (1, 2).
The check there is a translation of the existing code - which is defensive - if one of the values cannot be folded and it hasn't been caught, bail out.
An alternative would be to simply add the folded values directly.
| q = new TermsQuery(in.source(), fa.exactAttribute().name(), in.list()); | ||
| FieldAttribute fa = (FieldAttribute) in.value(); | ||
| List<Expression> list = in.list(); | ||
| // TODO: this needs to be handled inside the optimizer |
There was a problem hiding this comment.
@matriv This case of eliminating nulls need to be moved from the query to the optimizer (maybe in a dedicated rule).
It seems to be inconsistent with the foldable method - currently In has a fairly scattered behavior when it comes to list that would be good to centralize.
There was a problem hiding this comment.
What was the issue? which tests were failing?
The In uses a HashSet to eliminate duplicates but if there are nulls in the list of values it keeps at list one, in order to implement the following behavior (adopted from Postgres):
postgres=# select 1 IN (2, 3, null);
?column?
----------
(1 row)
postgres=# select 1 IN (1, 2, 3, null);
?column?
----------
t
(1 row)
postgres=# select 1 IN (2, 3);
?column?
----------
f
(1 row)
So if there is no match in the list, and there a null in the list, we should return null instead of false.
There was a problem hiding this comment.
Eliminating the null from the list of values before doing the translation. This logic should be externalized from the query translation which should only worry about that not do any clean-up.
(cherry picked from commit 5936896)
No description provided.