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

[ES|QL] where null throws VerificationException when it is not under an aggregation #116351

Closed
fang-xing-esql opened this issue Nov 6, 2024 · 8 comments · Fixed by #118324
Closed
Assignees
Labels
:Analytics/ES|QL AKA ESQL Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@fang-xing-esql
Copy link
Member

The behavior of null filter is different when it is under or not under an aggregation.

Null filter is allowed under aggregation

from employees
| stats max = max(salary), max_a = max(salary) where null,
        min = min(salary), min_a = min(salary) where to_string(null) == "abc"  
;

max:integer |max_a:integer|min:integer | min_a:integer
74999       |null         |25324       | null        
;

However if a null filter is not under an aggregation, VerificationException is thrown

{"query": "from sample_data | where null"}
{
  "error" : {
    "type" : "verification_exception",
    "reason" : "Found 1 problem\nline 1:26: Condition expression needs to be boolean, found [NULL]"
  },
  "status" : 400
}

Both postgresql and mysql allow where null, it returns empty resultset and doesn't error out. We may want to have where null behave the same when it is not under an aggregation.

@fang-xing-esql fang-xing-esql self-assigned this Nov 6, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Nov 6, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@kanoshiou
Copy link
Contributor

Hi @fang-xing-esql 👋

May I take on this issue? I would be very grateful for the opportunity.

@astefan
Copy link
Contributor

astefan commented Dec 16, 2024

What is the actual desired outcome of where null or where concat(multi_value_field, "foobar") (note that many other operations result in a null, not only the null constant).

@bpintea
Copy link
Contributor

bpintea commented Dec 17, 2024

What is the actual desired outcome of where null

I would assume a LocalRelation with an empty supplier.

@astefan
Copy link
Contributor

astefan commented Dec 18, 2024

I think the discussion here is more subtle.
row x = null
row x = null::integer
row x = null::boolean

All these three queries return the value null, but the type of that "null" is null, integer, boolean.

row x = null | where x
row x = null::integer | where x
row x = null::boolean | where x

The first two queries return an exception, the last one succeeds and returns no rows and all columns.

My previous question hinted at the difference between null the datatype and null the value.

@astefan
Copy link
Contributor

astefan commented Dec 18, 2024

The example provided in the description hints at the value null with where to_string(null) == "abc". In this condition the type of the null is keyword and the result is boolean. In where null the type of the result is null. The provided PR for this issue changes the accepted type of the filter, but the filter in the aggregation seems to look at the value null, not its type (or both, I am not sure)?

@bpintea @fang-xing-esql

@bpintea
Copy link
Contributor

bpintea commented Dec 18, 2024

I think there's an inconsistency in the way we check the filters: the agg ones we allow a NULL type (we don't allow other NULL values if of different type than BOOLEAN); I think the PR replicates that.

The command one only accepts BOOLEAN types (of whatever value). I think we should harmonise these. The way the proposed PR does seems OK if we accept the NULL type agg filters. Otherwise we should only accept BOOLEAN types (which seems more correct at first sight).

@astefan
Copy link
Contributor

astefan commented Dec 18, 2024

I was having the wrong assumption. I've tested some queries and aggs do the right thing imo.

where to_string(null) == "abc"  

this results in null boolean type. Because to_string(null) is null type keyword, then null keyword == "abc" results in null boolean. Aggs allow null the null type and null the boolean type. And the PR seems to do the right thing, but it needs more tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants