EQL: Add infra for planning and query folding #52065
Conversation
Actual folding not yet in place
|
Pinging @elastic/es-search (:Search/EQL) |
x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plan/physical/FilterExec.java
Show resolved
Hide resolved
|
does any of this make sense to put in QL? a lot looks like boilerplate and good candidates for QL, but that could just be because the actual folding isn't implemented yet |
|
I was tempted to do that however the execution model between the SQL and EQL is likely to be different in the beginning. If it proves to remain similar, we can definitely move things in one place at least for some of nodes (like |
aleksmaus
left a comment
aleksmaus
left a comment
There was a problem hiding this comment.
few comments in few places maybe worth hardening against NPE?
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return executable.hashCode(); |
There was a problem hiding this comment.
this would NPE if executable is null
There was a problem hiding this comment.
The NPE cannot occur since:
- all children are non null as the plan is constructed.
- if by accident a null child is passed, the base class
Nodechecks and throws a proper exception.
There was a problem hiding this comment.
Thanks for the explanation!
| return failures.stream().map(f -> { | ||
| Location l = f.node().source().source(); | ||
| return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message(); | ||
| }).collect(Collectors.joining(StringUtils.NEW_LINE, "Found " + failures.size() + " problem(s)\n", StringUtils.EMPTY)); |
There was a problem hiding this comment.
I know it's code that already existed and you just moved it from one place to another, but I think we can do slightly better with the error message. Why not doing "Found " + failures.size() + " problem" + (failures.size() > 1 ? "s\n" : "\n")?
Actual folding not yet in place (TBD) (cherry picked from commit d52b96f)
5 participants
Actual folding not yet in place.