Skip to content

[FLINK-33725][core] Return false from MathUtils.isPowerOf2 for non-positive values#28557

Open
vasiliy-mikhailov wants to merge 1 commit into
apache:masterfrom
vasiliy-mikhailov:flink-33725-ispowerof2
Open

[FLINK-33725][core] Return false from MathUtils.isPowerOf2 for non-positive values#28557
vasiliy-mikhailov wants to merge 1 commit into
apache:masterfrom
vasiliy-mikhailov:flink-33725-ispowerof2

Conversation

@vasiliy-mikhailov

@vasiliy-mikhailov vasiliy-mikhailov commented Jun 27, 2026

Copy link
Copy Markdown

What is the purpose of the change

MathUtils.isPowerOf2(long) returns true for 0 and for Long.MIN_VALUE,
neither of which is a power of two. 0 & (0 - 1) == 0 and
Long.MIN_VALUE & (Long.MIN_VALUE - 1) == 0, so the existing bit trick alone
misclassifies non-positive inputs. This fixes
FLINK-33725.

Brief change log

  • Guard isPowerOf2 with value > 0 so non-positive values return false.
  • Extend MathUtilTest#testPowerOfTwo with the 0, -1 and Long.MIN_VALUE cases.

Verifying this change

Added assertions to MathUtilTest#testPowerOfTwo for 0, -1 and Long.MIN_VALUE. Running just that test on the current master (before the one-line change) fails, and passes after it:

Before the fix (on master):

$ mvn test -Dtest=MathUtilTest#testPowerOfTwo -pl flink-core
[INFO] Running org.apache.flink.util.MathUtilTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 <<< FAILURE! -- in org.apache.flink.util.MathUtilTest
[ERROR] org.apache.flink.util.MathUtilTest.testPowerOfTwo <<< FAILURE!
Expecting value to be false but was true
	at org.apache.flink.util.MathUtilTest.testPowerOfTwo(MathUtilTest.java:126)
[INFO] BUILD FAILURE

After the fix:

$ mvn test -Dtest=MathUtilTest#testPowerOfTwo -pl flink-core
[INFO] Running org.apache.flink.util.MathUtilTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 -- in org.apache.flink.util.MathUtilTest
[INFO] BUILD SUCCESS

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Change to the configuration of TaskManager, etc.: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable
Was generative AI tooling used to co-author this PR?
  • Yes (Claude Code, Claude Opus 4.8)

Generated-by: Claude Code (Claude Opus 4.8)

@flinkbot

flinkbot commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@spuru9 spuru9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look good, Can you update the PR description as per guidlines.

@vasiliy-mikhailov

Copy link
Copy Markdown
Author

Thanks for taking a look. I've updated the description to follow the PR template (filled in the full "potentially affect" checklist and the verifying section).

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jun 27, 2026
@spuru9

spuru9 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for taking a look. I've updated the description to follow the PR template (filled in the full "potentially affect" checklist and the verifying section).

Can you add the AI declaration too?

@vasiliy-mikhailov

Copy link
Copy Markdown
Author

Added an AI assistance disclosure to the description. For transparency: an AI pipeline was used to scan a large amount of code, surface suspected bugs, reproduce a subset with failing unit tests and generate fixes, and prepare PRs; each one — including this — was reviewed and verified by a human (fail-before / pass-after test run plus code review) before opening.

@spuru9

spuru9 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Can you refer #28463 (comment)

@vasiliy-mikhailov

Copy link
Copy Markdown
Author

Thanks @spuru9 — good call, I've switched to the standard template field from #28463: checked "Was generative AI tooling used to co-author this PR?" and added the Generated-by: trailer, replacing my earlier free-form note.

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

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants