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

ORC-1146: Float category missing check if the statistic sum is a finite value #1077

Closed
wants to merge 2 commits into from

Conversation

guiyanakuang
Copy link
Member

@guiyanakuang guiyanakuang commented Apr 4, 2022

What changes were proposed in this pull request?

This pr is aimed at checking whether the float category statistic sum has a finite value.

Why are the changes needed?

When the orc float category is written with NaN, pushing down is not supported.

How was this patch tested?

Added unit test.

@github-actions github-actions bot added the JAVA label Apr 4, 2022
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @guiyanakuang .

cc @williamhyun and @pgaref

@dongjoon-hyun dongjoon-hyun modified the milestones: 1.7.4, 1.8.0 Apr 4, 2022
dongjoon-hyun pushed a commit that referenced this pull request Apr 4, 2022
…te value

### What changes were proposed in this pull request?

This pr is aimed at checking whether the float category statistic sum has a finite value.

### Why are the changes needed?

When the orc float category is written with NaN, pushing down is not supported.

### How was this patch tested?

Added unit test.

Closes #1077

Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 93a3505)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun dongjoon-hyun modified the milestones: 1.8.0, 1.7.4 Apr 4, 2022
@dongjoon-hyun
Copy link
Member

I deleted my previous comment about the branch-1.7 test result.

At a clean build, I verified this patch in branch-1.7 and backported. Thank you again, @guiyanakuang .

Could you re-run your integration tests, @williamhyun ?

@dongjoon-hyun
Copy link
Member

BTW, @guiyanakuang . Could you check branch-1.6 too when you have a time? We backported the following in branch-1.6. I guess we might have the same issue there.

@guiyanakuang
Copy link
Member Author

guiyanakuang commented Apr 5, 2022

Thanks @dongjoon-hyun . I checked the branch-1.6 and confirmed it has the same problem. This pr can just cherry pick and fix it without any conflicts.
Later on I will also commit an improvement to determine the presence of NaN writes, which I think can also be ported backwards to 1.6. 😄

@dongjoon-hyun
Copy link
Member

Thank you, @guiyanakuang .

cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
…te value

### What changes were proposed in this pull request?

This pr is aimed at checking whether the float category statistic sum has a finite value.

### Why are the changes needed?

When the orc float category is written with NaN, pushing down is not supported.

### How was this patch tested?

Added unit test.

Closes apache#1077

Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants