Support bi-directional graph traversal command graphlookup#5113
Support bi-directional graph traversal command graphlookup#5113qianheng-aws wants to merge 24 commits intofeature/graphlookupfrom
graphlookup#5113Conversation
Signed-off-by: Lantao Jin <ltjin@amazon.com>
…graphlookup # Conflicts: # core/src/main/java/org/opensearch/sql/analysis/Analyzer.java # core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java # core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java # ppl/src/main/antlr/OpenSearchPPLParser.g4
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
Change the target branch to |
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
LantaoJin
changed the title
graphlookup
| * | ||
| * <p>Returns: List - array of [row_fields..., depth?] | ||
| */ | ||
| public class GraphLookupBfsFunction extends ImplementorUDF { |
There was a problem hiding this comment.
Could the classes GraphLookupBfsFunction and GraphLookupFunction prune?
| } | ||
| } | ||
|
|
||
| if (++currentLevelDepth > graphLookup.maxDepth) break; |
There was a problem hiding this comment.
Seems it processes one additional level. For example maxDepth=2, it processes 3 levels total instead of stopping at level 2:
1st level done, then check (++0 > 2 ❎), continue;
2nd level done, then check (++1 > 2 ❎), continue;
3rd level done, then check (++ 2 > 2 ✅), stop
There was a problem hiding this comment.
That's by design. It starts from 0 hop which stands for direct connection. It means we can set maxDepth=0. You can refer to mongdb's definition about this param, it should be a non-negative integer
| if (graphLookup.depthField != null) { | ||
| Object[] rowWithDepth = new Object[rowArray.length + 1]; | ||
| System.arraycopy(rowArray, 0, rowWithDepth, 0, rowArray.length); | ||
| rowWithDepth[rowArray.length] = currentLevelDepth; |
There was a problem hiding this comment.
currentLevelDepth represents the depth of SOURCE nodes being queried, not the TARGET nodes being added to results. Target nodes should be at depth currentLevelDepth + 1?
Tests with depthField use default maxDepth=0, which only processes one iteration, so all results correctly show depth=0. Tests with maxDepth > 0 don't verify depth values.
| } | ||
|
|
||
| @Override | ||
| public Object current() { |
There was a problem hiding this comment.
Lacks ResourceMonitor health checks during BFS traversal. Need to add periodic health check monitor.isHealthy() in moveNext() and performBfs().
There was a problem hiding this comment.
The monitor is inner both scan for source table and lookup table. Since our all operators actually share the same jvm and monitor. It will be triggered in each iteration inner scan of source or lookup table.
|
|
||
| query = QueryBuilders.boolQuery().should(query).should(backQuery); | ||
| } | ||
| CalciteEnumerableIndexScan newScan = (CalciteEnumerableIndexScan) this.lookupScan.copy(); |
There was a problem hiding this comment.
The code creates new scan instance but never close it? can you debug to check if this instance be closed expectedly.
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
2 participants
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.