Add inputlookup as another form of search sources#5081
Add inputlookup as another form of search sources#5081Swiddis wants to merge 2 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdded an Changes
Sequence Diagram(s)(omitted — change is a small grammar addition that doesn't introduce multi-component control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ppl/src/main/antlr/OpenSearchPPLParser.g4`:
- Around line 623-632: The parser allows INPUTLOOKUP in the fromClause with the
dynamicSourceClause alternative, but visitDynamicSourceClause currently throws
UnsupportedOperationException causing a runtime crash for INPUTLOOKUP
dynamicSourceClause; implement visitDynamicSourceClause (or the corresponding
visitor method) to properly handle dynamicSourceClause when its parent is
INPUTLOOKUP by wiring the same dispatch/processing done for
tableOrSubqueryClause/tableFunction (or return a clear
UnsupportedOperationException only for truly unsupported subvariants), ensuring
the visitor inspects the parse node shape and delegates to the correct
downstream handler; refer to visitDynamicSourceClause, fromClause, INPUTLOOKUP,
and dynamicSourceClause to locate and fix the logic.
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
dai-chen
left a comment
There was a problem hiding this comment.
Could you link an issue to provide more context about the plan?
| | INPUTLOOKUP tableOrSubqueryClause | ||
| | SOURCE EQUAL tableFunction | ||
| | INDEX EQUAL tableFunction | ||
| | INPUTLOOKUP tableFunction |
There was a problem hiding this comment.
Any syntax or AST test to add? No other code changes required?
|
I am not sure if this change is actually needed. |
Description
Allows queries like
inputlookup big5 | head 1.Will rip this out again later once #5074 is actually in progress and replace it with a real inputlookup command, this is just some temp noise since we want to call plain indices lookups in the current version.
Related Issues
N/A
Check List
--signoffor-s.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.