Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RANGE bounded window frame #561

Open
wants to merge 2 commits into
base: sprint-57
Choose a base branch
from
Open

Conversation

anusudarsan
Copy link

No description provided.

return new Range(-1, -1);
}

if (frameInfo.getType() == RANGE && emptyFrame(frameInfo, peerGroupStart, peerGroupEnd - 1)) {
Copy link
Author

Choose a reason for hiding this comment

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

the last argument must be endPosition - (peerGroupEnd - 1) here, I will fix that in the next patch where we will have bounded following.

Choose a reason for hiding this comment

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

Add assertion for frameEnd till you implement following.

@anusudarsan anusudarsan changed the title [WIP] Implement RANGE bounded PRECEDING window frame Implement RANGE bounded window frame Jun 9, 2017
@anusudarsan
Copy link
Author

@maciejgrzybek

@maciejgrzybek maciejgrzybek self-assigned this Jun 9, 2017
return new Range(-1, -1);
}

if (frameInfo.getType() == RANGE && emptyFrame(frameInfo, peerGroupStart, peerGroupEnd - 1)) {

Choose a reason for hiding this comment

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

Add assertion for frameEnd till you implement following.

.row(null, null, 1L)
.build());

assertWindowQueryWithNulls("sum(orderkey) OVER (PARTITION BY orderstatus ORDER BY orderkey " +

Choose a reason for hiding this comment

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

This test is virtually a duplicate of 0e78f1c#diff-f122e8a4be46fc9376777791451e4d94R599

private boolean emptyFrame(FrameInfo frameInfo, int rowPosition, int endPosition)
private int precedingEndRange(long endValue)
{
int peerGroupEndIndex = peerGroupEndIndices.indexOf(peerGroupEnd);

Choose a reason for hiding this comment

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

I'd use Map<Integer, Integer> (peer group -> position) instead of ArrayList<Integer>, to avoid slowness when there are many tiny groups in a large partition.
Imagine billions of rows with ~1 row / peer group.
Then indexOf will be O(billions). But if you use a Map, it's gonna be O(log(billions)).

while (currentValue < value) {
boolean peerFound = false;
followingPeerGroupEnd = followingPeerGroupStart + 1;
while ((followingPeerGroupEnd < partitionEnd) && pagesIndex.positionEqualsPosition(peerGroupHashStrategy, followingPeerGroupStart, followingPeerGroupEnd)) {

Choose a reason for hiding this comment

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

What if there are no peers, just each row is a distinct value within the partition?
You'd never set peerFound = true, IIUC so this loop would never end?
Can you explain this to me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants