Fix error when pushing down script filter with struct field #3693
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Heng Qian <[email protected]>
qianheng-aws
requested review from
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
mengweieric,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto,
GumpacG,
ykmr1224,
LantaoJin and
noCharger
as code owners
Signed-off-by: Heng Qian <[email protected]>
LantaoJin
previously approved these changes
May 29, 2025
penghuo
reviewed
May 30, 2025
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Cannot pushdown the filter condition: {}", queryCondition, e); | ||
| } else { | ||
| LOG.info("Cannot pushdown the filter condition: {}, ", queryCondition); |
There was a problem hiding this comment.
sanitize queryCondition before logging.
There was a problem hiding this comment.
Do you mean anonymizing the queryCondition by using PPLQueryDataAnonymizer?
I think, after anonymizing, we cannot get any useful information from the log. If it's not allowed to show that information in log, maybe we should remove queryCondition here directly.
There was a problem hiding this comment.
Or maybe only keep debug level log and keep queryCondition in debug log. Does it make sense? What's your opinion? @penghuo
There was a problem hiding this comment.
queryCondtion include user meta data, we should not log it. remove it
Signed-off-by: Heng Qian <[email protected]>
…tField # Conflicts: # integ-test/src/test/resources/expectedOutput/ppl/explain_patterns_agg_push.json
penghuo
previously approved these changes
Jun 11, 2025
Signed-off-by: Heng Qian <[email protected]>
LantaoJin
approved these changes
Jun 11, 2025
penghuo
approved these changes
Jun 11, 2025
penghuo
pushed a commit
that referenced
this pull request
Jun 16, 2025
* Fix error when pushing down script filter with struct field Signed-off-by: Heng Qian <[email protected]> * Fix IT Signed-off-by: Heng Qian <[email protected]> * Address comments Signed-off-by: Heng Qian <[email protected]> * Fix IT Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> Signed-off-by: xinyual <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fix error when pushing down script filter with struct field. This bug only happens in v2 engine, since calcite engine doesn't support script pushing down currently.
It's caused by that the
FilterScript::getDoconly provides the capability of looking up leaf node, thus it will throw exceptionNo field found for [xxx] in mappingwhen using fields of struct type. We could address this issue by preventing push down script filter with fields of struct type.https://github.com/opensearch-project/OpenSearch/blob/9bca961b799625438d024491320323b310dc531f/server/src/main/java/org/opensearch/script/FilterScript.java#L73
Related Issues
Resolves #3312
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.