orc icon indicating copy to clipboard operation
orc copied to clipboard

ORC-1683: Fix `instanceof` of BinaryStatisticsImpl merge method

Open cxzl25 opened this issue 1 year ago • 3 comments

What changes were proposed in this pull request?

This PR aims to fix instanceof of BinaryStatisticsImpl merge method.

Why are the changes needed?

In ORC-1542, we modified part of instanceof, but BinaryStatisticsImpl was not modified because the merge method was written differently.

However, the current code is instanceof BinaryColumnStatistics and then explicitly cast BinaryStatisticsImpl, so it should be replaced by the new instanceof writing method (Pattern Matching for instanceof).

if (other instanceof BinaryColumnStatistics) {
        BinaryStatisticsImpl bin = (BinaryStatisticsImpl) other;

How was this patch tested?

GA

Was this patch authored or co-authored using generative AI tooling?

No

cxzl25 avatar Apr 10 '24 04:04 cxzl25

In addition, this is not Fixing anything. We use Fix for bugs.

dongjoon-hyun avatar Apr 10 '24 06:04 dongjoon-hyun

Could you find more like the previous PR? We don't want to make many PRs like this online style change.

I have checked all related instanceof and only ORC-1683 and ORC-1685 are left.

In addition, this is not Fixing anything. We use Fix for bugs.

Since there could be a potential ClassCastException here, I've split it out to raise a separate PR.

cxzl25 avatar Apr 10 '24 06:04 cxzl25

Oh, I got what you mean.

dongjoon-hyun avatar Apr 10 '24 07:04 dongjoon-hyun

Sorry for being late. Please feel free to merge if you are confident, @cxzl25 .

dongjoon-hyun avatar Aug 14 '24 05:08 dongjoon-hyun