[FR] Support ES|QL sub queries - #6665
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes ES|QL remote validation for queries that use FROM (subquery) sources (ES|QL subqueries), by extracting and handling all FROM clauses (including nested ones) instead of assuming a single flat FROM idx-a, idx-b ... source list.
Changes:
- Added ES|QL query source scanning utilities that ignore comments/literals, group
FROMclauses by the indices they read, and support targeted source replacement. - Updated ES|QL remote validation to prepare/create test indices per
FROM-source group and rewrite the query accordingly (plus deduplicated cleanup index list). - Hardened index-pattern matching in schema filtering by escaping patterns (preventing regex compilation failures) and added unit tests for the new parsing behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_esql.py |
Adds unit tests covering ES |
pyproject.toml |
Bumps project version to 2.1.3. |
detection_rules/schemas/definitions.py |
Replaces the old single-FROM regex with reusable ES |
detection_rules/rule_validators.py |
Updates ES |
detection_rules/index_mappings.py |
Escapes index patterns before compiling regex and ensures remote index names remain unique across multiple groups. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
initial thoughts from PR description - may be immature without checking, please ignore if so:
That was my concern. I assume our approach to handle this is to treat the sibling or parent query nodes as "walkable" like AST objects and ensure the field is valid. Even then, these are runtime checks would have to use the original query while unit tests can walk the nodes.
Should be |
|
consideration: if subqueries found, ensure rule is min-stack 9.5+ |
terrancedejesus
left a comment
There was a problem hiding this comment.
peer review with @eric-forte-elastic. couple of nits and comments/thoughts about min-stack validation if sub queries. valid question if we plan to control/own parser - is it worth the squeeze here or handle it on sprint for python parser. approving not to block.
w0rk3r
left a comment
There was a problem hiding this comment.
LGTM, needed to unblock some new work
|
To unblock, I think this is fine to go in as the last patch for ESQL remote validation support. In the next couple weeks we will remove all of this logic. If this is a ~2 week stopgap, then @eric-forte-elastic we should just be clear in the PR summary that we may drop support in the upcoming cycle. |
Mikaayenson
left a comment
There was a problem hiding this comment.
lgtm as a lowrisk stopgap.
Summary already reflects that this is a stop gap, updating to reflect the ~2 week timetable 👍 |
Pull Request
Issue link(s):
Related: #6491, #6633
Summary - What I changed
ES|QL remote validation could not handle a query whose sources are subqueries (
FROM (FROM a-* | ...), (FROM b-* | ...), generally available in 9.5.0). Index extraction assumed the flatFROM idx-a, idx-b [METADATA ...]form, so for a subquery it extracted the literal text(\n FROM logs-endpoint.events.file-*as the "index", whichget_filtered_index_schemathen fed tore.compile. The leading(caused the validation to abort withre.PatternErroras this broke the pattern.Primary change in the code is to scan every
FROMclause in a query rather than only the first, and to group those clauses by the sources they read. This addresses the error as it allows multipleFROMclauses. However in a similar nature to #6491, each clause is then prepared and executed against only its own indices, so a subquery cannot validate a field that exists solely in a sibling subquery's index. For example, thefiledatastream in the endpoint package declares 210 fields andprocessdeclares 428, with 309 fields inprocessthatfiledoes not have. While this would have passed validation before, that is more due to the subquery validation itself failing rather than a logical error in how the subqueries would have been handled.Important
We should generally not be using regex to do this as it is by nature fragile. This is a stopgap until we can switch to a more fully featured parsing strategy e.g. #6207 or #6500 (ETA is to potentially drop support in ~2 weeks ref)
Note
event.datasetvalues are still extracted from the whole query and passed to every group rather than being narrowed per subquery. This follows the reasoning in #6491, that these values are extracted by regex and may sit insideORbranches, so they cannot be trusted to narrow scope. Passing the full set can only widen a group's mappings, andget_filtered_index_schemafilters the integration streams by that group's indices regardless.How To Test
Before (main), against the two
FROM (subquery)rules from #6633After (this branch)

Checklist
bug,enhancement,schema,maintenance,Rule: New,Rule: Deprecation,Rule: Tuning,Hunt: New, orHunt: Tuningso guidelines can be generatedmeta:rapid-mergelabel if planning to merge within 24 hoursContributor checklist