Skip to content

Fix long overflow for OperatorStats#22230

Merged
kewang1024 merged 1 commit intoprestodb:masterfrom
kewang1024:Fix-Operator-Stats
Mar 20, 2024
Merged

Fix long overflow for OperatorStats#22230
kewang1024 merged 1 commit intoprestodb:masterfrom
kewang1024:Fix-Operator-Stats

Conversation

@kewang1024
Copy link
Collaborator

@kewang1024 kewang1024 commented Mar 18, 2024

In OperatorStats, summing long types leads to overflow. Use double type to do calculation.
If final result is larger than Long.MAX_VALUE, return Long.MAX_VALUE.

== NO RELEASE NOTE ==

@kewang1024 kewang1024 requested a review from a team as a code owner March 18, 2024 01:20
@kewang1024 kewang1024 requested a review from presto-oss March 18, 2024 01:20
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

Can you add a test case for this in TestOperatorStats?

joinProbeKeyCount);
}

public static long convertDoubleToLong(double dataSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could remove this and just use a simple cast. See https://docs.oracle.com/javase/specs/jls/se8/html/jls-5.html#jls-5.1.3

@kewang1024 kewang1024 requested a review from tdcmeehan March 18, 2024 17:57
@kewang1024
Copy link
Collaborator Author

Thanks @tdcmeehan for your good suggestions, just updated accordingly

@kewang1024 kewang1024 force-pushed the Fix-Operator-Stats branch 2 times, most recently from d5bf970 to 4a5e381 Compare March 18, 2024 19:06
Summing long types leads to overflow and query failure, use double to do
calculation. If final result is larger than Long.MAX_VALUE,
return Long.MAX_VALUE.
@kewang1024 kewang1024 merged commit e17757c into prestodb:master Mar 20, 2024
@natashasehgal natashasehgal mentioned this pull request Apr 1, 2025
7 tasks
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