Query refactoring: validate inner queries whenever supported#11889
Conversation
c389ff4 to
7d2a0e9
Compare
There was a problem hiding this comment.
I see how this gives context to the error message, wondering if there is an easy way to be more specific in case there is more than query with the same name in the complete query (like a match in both must and should clause of BooleanQueryBuilder). I guess that's not possible right now, but would be great if inner queries knew about their parents, then we could output the full path here.
There was a problem hiding this comment.
good point, this makes sense. I just checked what we currently do given that validation before refactoring happens while parsing. No context is given for messages that we can move to validate, which are not json specific. That means this is not a regression caused by the refactoring, rather an improvement that we could work on in the future ;)
|
I went through the changes quickly, it is great that we can validate recursively that way. LGTM besides the small remark which at the moment doesn't seem solvable, unless you have an idea. Tracking the path in the query tree as state in some collector object we pass in with the topmost query might be possible, but maybe overkill at this point. |
I would leave keeping track of the query tree etc. for later, it is a nice to have though, opened #11939 for that. |
b62c466 to
1861095
Compare
Added validation for all inner queries that any already refactored query may hold. Added also tests around this. At the end of the refactoring validate will be called by SearchRequest#validate during TransportSearchAction execution, which will call validate against the top level query builder that will need to go and validate its data plus all of its inner queries, and so on. Closes elastic#11889
1861095 to
10f8016
Compare
Added validation for all inner queries that any already refactored query may hold. Added also tests around this. At the end of the refactoring validate will be called by SearchRequest#validate during TransportSearchAction execution, which will call validate against the top level query builder that will need to go and validate its data plus all of its inner queries, and so on. Closes elastic#11889
Added validation for all inner queries that any already refactored query may hold. Added also tests around this. At the end of the refactoring validate will be called by SearchRequest#validate during TransportSearchAction execution, which will call validate against the top level query builder that will need to go and validate its data plus all of its inner queries, and so on.