topN push down for lookup #3499
Conversation
Sophie-Xie
requested review from
Shylock-Hg,
cangfengzhs,
yixinglu and
critical27
| std::vector<T> v_; | ||
| }; | ||
|
|
||
| class IndexTopNNode : public IndexLimitNode { |
There was a problem hiding this comment.
It may be better to derive from IndexNode than IndexLimitNode
src/storage/exec/IndexTopNNode.h
Outdated
|
|
||
| void setComparator(std::function<bool(T&, T&)> comparator) { comparator_ = comparator; } | ||
|
|
||
| void push(T data) { |
There was a problem hiding this comment.
Maybe the performance will be better if you use rvalue
There was a problem hiding this comment.
yes, I have fixed it
|
|
||
| for (auto factor : factors) { | ||
| auto colName = projColNames[factor.first]; | ||
| auto found = namesMap.find(colName); |
There was a problem hiding this comment.
So you treat the columns in Project and IndexScan with same name as the same column?
There was a problem hiding this comment.
yes, is there any problem?
There was a problem hiding this comment.
It's not always correct. See the plan:
-----+------------------+--------------+----------------+--------------------------------------
| 2 | Project | 6 | | outputVar: [ |
| | | | | { |
| | | | | "colNames": [ |
| | | | | "name" |
| | | | | ], |
| | | | | "type": "DATASET", |
| | | | | "name": "__Project_2" |
| | | | | } |
| | | | | ] |
| | | | | inputVar: __TagIndexFullScan_1 |
| | | | | columns: [ |
| | | | | "player.name AS name" |
| | | | | ] |
-----+------------------+--------------+----------------+--------------------------------------
| 6 | TagIndexFullScan | 0 | | outputVar: [ |
| | | | | { |
| | | | | "colNames": [ |
| | | | | "_vid", |
| | | | | "player.name" |
| | | | | ], |
| | | | | "name": "__TagIndexFullScan_1", |
| | | | | "type": "DATASET" |
| | | | | } |
| | | | | ] |
| | | | | inputVar: |
| | | | | space: 9 |
| | | | | dedup: false |
| | | | | limit: 9223372036854775807 |
| | | | | filter: |
| | | | | orderBy: [] |
| | | | | schemaId: 10 |
| | | | | isEdge: false |
| | | | | returnCols: [ |
| | | | | "_vid", |
| | | | | "name" |
| | | | | ] |
| | | | | indexCtx: [ |
| | | | | { |
| | | | | "columnHints": [], |
| | | | | "filter": "", |
| | | | | "index_id": 16 |
| | | | | } |
The columns of Project is set by user in YIELD clause , so user could set to Anything he like. So the better way is to check expression of column in Project and ReturnCols in IndexScan.
There was a problem hiding this comment.
我理解你说的是lookup on player yield player.name as n | order by $-.n | limit 2这种情况吧
上边我对你的第一个问题理解有点问题，这里的转换逻辑用的就是yieldClause中的expr的prop作为下推的排序字段
MMyheart
force-pushed
the
feature/optimize_topn_push_down_for_lookup
branch
3 times, most recently
from
9708695 to
0039a5a
Compare
MMyheart
force-pushed
the
feature/optimize_topn_push_down_for_lookup
branch
from
5fb4ab5 to
43d60a0
Compare
| """ | ||
| Then the result should be, in any order: | ||
| | name | | ||
| | /[a-zA-Z ']+/ | |
There was a problem hiding this comment.
Why not compare real name? The result is fixed when specify order.
There was a problem hiding this comment.
我原本只是想测试topN算子的下推情况，因为结果的顺序性在 IndexScanTopNTest 里面已经保证了
我改成比较真实的id跟名字吧
|
加两个降序sort的测试吧 |
DONE |
|
Good job. |
foesa-yang
removed
the
doc affected
What type of PR is this?
What does this PR do?
Which issue(s)/PR(s) this PR relates to?
Special notes for your reviewer, ex. impact of this fix, etc:
Additional context/ Design document:
Checklist:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: