Skip to content

fix: Remove !=0 check from supports_collect_by_thresholds#20730

Open
jonathanc-n wants to merge 5 commits intoapache:mainfrom
jonathanc-n:remove-threshold-todo
Open

fix: Remove !=0 check from supports_collect_by_thresholds#20730
jonathanc-n wants to merge 5 commits intoapache:mainfrom
jonathanc-n:remove-threshold-todo

Conversation

@jonathanc-n
Copy link
Contributor

Which issue does this PR close?

  • Closes #.

Rationale for this change

Small change to remove the TODO + != 0 check. Since the commit was made two years ago, statistics have been changed to use Precision. Exact(0) and inexact(0) are now valid stats while absent will evaluate to false

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the optimizer Optimizer rules label Mar 5, 2026
@jonathanc-n
Copy link
Contributor Author

cc @gabotechs

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Mar 5, 2026
Co-authored-by: Daniël Heres <danielheres@gmail.com>
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @jonathanc-n

// them as unknown, `Absent` will return None (this is in regards to why
// `!=0` is not checked)
if let Some(byte_size) = stats.total_byte_size.get_value() {
*byte_size != 0 && *byte_size < threshold_byte_size
Copy link
Contributor

Choose a reason for hiding this comment

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

when would byte size ever be zero ? This seems like if the size was zero something is wrong 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Byte size can be zero if working on empty table/partition or if filter exec filters down to zero rows.

This PR just removes this check as it is not longer a problem due to the addition of Precision (this check was added 2 years ago). This sanity check was previously here because getting statistics with limit would unwrap to zero (which would now be Absent)

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @jonathanc-n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants