Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-574]implement columnar limit #575

Merged
merged 5 commits into from
Nov 26, 2021
Merged

Conversation

zhouyuan
Copy link
Collaborator

What changes were proposed in this pull request?

This patch implements columnar Limit
Signed-off-by: Yuan Zhou yuan.zhou@intel.com

How was this patch tested?

local unit tests

@github-actions
Copy link

#574

@zhouyuan zhouyuan force-pushed the wip_limit branch 2 times, most recently from 998ab09 to 8e06431 Compare November 22, 2021 02:23
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
val delta = iter.next()
rowCount += delta.numRows
if (rowCount > limit) {
val newSize = rowCount - limit
Copy link
Collaborator

@rui-mo rui-mo Nov 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newSize calculation logic can be changed like below codes:
if (rowCount < limit) {
val delta = iter.next()
val preRowCount = rowCount
rowCount += delta.numRows
if (rowCount > limit) {
delta.setNumRows(limit - preRowCount)
}
delta
} else {
throw new NoSuchElementException("End of ColumnarBatch iterator")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, thanks for the check

if (rowCount > limit) {
val newSize = limit - preRowCount
delta.setNumRows(newSize)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, delta should be returned

@zhouyuan zhouyuan force-pushed the wip_limit branch 2 times, most recently from dabe74e to 8d9d1cb Compare November 26, 2021 10:11
@rui-mo
Copy link
Collaborator

rui-mo commented Nov 26, 2021

@zhouyuan
Copy link
Collaborator Author

hi yuan, here should also be changed into ColumnarLocalLimitExec according to my test. https://github.com/oap-project/gazelle_plugin/pull/575/files#diff-6b153121ea621fb379a6c26579877f5d30fa85ae7749225b17a8d6ef5bcb7048L1113

right, I just added this change

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan zhouyuan merged commit dcdc459 into oap-project:master Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants