Skip to content

expression: fix IS_IPV4_COMPAT for all-zero address#68438

Open
hawkingrei wants to merge 1 commit into
pingcap:masterfrom
hawkingrei:issue-59458-is-ipv4-compat-zero
Open

expression: fix IS_IPV4_COMPAT for all-zero address#68438
hawkingrei wants to merge 1 commit into
pingcap:masterfrom
hawkingrei:issue-59458-is-ipv4-compat-zero

Conversation

@hawkingrei
Copy link
Copy Markdown
Member

@hawkingrei hawkingrei commented May 17, 2026

What problem does this PR solve?

Issue Number: close #59458

Problem Summary:

IS_IPV4_COMPAT(X'00000000000000000000000000000000') returned 1, while MySQL-compatible behavior expects 0 for the all-zero IPv6 unspecified address.

What changed and how does it work?

This PR adds a shared IS_IPV4_COMPAT binary classifier for both scalar and vectorized evaluation.

The function still requires a 16-byte IPv6 value with a zero 12-byte prefix, but it now rejects the all-zero address by requiring at least one non-zero byte in the embedded IPv4 suffix.

Check List

Tests

  • Unit test
    • ./tools/check/failpoint-go-test.sh pkg/expression -run TestIsIPv4Compat -count=1
    • ./tools/check/failpoint-go-test.sh pkg/expression -run TestVectorizedBuiltinMiscellaneousFunc -count=1 -args builtinIsIPv4CompatSig
  • Integration test
    • ./tools/check/failpoint-go-test.sh pkg/expression/integration_test -run TestVectorMiscFunctions -count=1
    • git diff --check -- pkg/expression/builtin_miscellaneous.go pkg/expression/builtin_miscellaneous_vec.go pkg/expression/builtin_miscellaneous_test.go pkg/expression/builtin_miscellaneous_vec_test.go pkg/expression/integration_test/integration_test.go
    • make lint
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Fix `IS_IPV4_COMPAT` to return `0` for the all-zero IPv6 address.

Summary by CodeRabbit

  • Bug Fixes
    • Improved IS_IPV4_COMPAT function behavior. The all-zero IPv6 unspecified address is now correctly classified as not IPv4-compatible, returning 0 instead of the previous incorrect result. This fixes edge case handling in IPv6 address validation.

Review Change Stack

@ti-chi-bot ti-chi-bot Bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 17, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented May 17, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign windtalker for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 17, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5b9b2f12-ac1b-4d96-901c-e7608bab23ef

📥 Commits

Reviewing files that changed from the base of the PR and between 6a6eefe and 90b6864.

📒 Files selected for processing (5)
  • pkg/expression/builtin_miscellaneous.go
  • pkg/expression/builtin_miscellaneous_test.go
  • pkg/expression/builtin_miscellaneous_vec.go
  • pkg/expression/builtin_miscellaneous_vec_test.go
  • pkg/expression/integration_test/integration_test.go

📝 Walkthrough

Walkthrough

The IS_IPV4_COMPAT function is corrected to return 0 for the all-zero IPv6 address (::) instead of 1. A new helper function isIPv4CompatBinary consolidates the compatibility check logic for both scalar and vectorized evaluation, and comprehensive test coverage validates the fix across all evaluation paths.

Changes

IPv4-compat all-zero address fix

Layer / File(s) Summary
IPv4-compat helper and scalar evaluation
pkg/expression/builtin_miscellaneous.go
New isIPv4CompatBinary helper checks IPv6 byte length, verifies the first 12 bytes are zero, and treats the all-zero address as not IPv4-compatible. builtinIsIPv4CompatSig.evalInt replaces inline prefix logic with calls to the helper.
Vectorized evaluation update
pkg/expression/builtin_miscellaneous_vec.go
builtinIsIPv4CompatSig.vecEvalInt removes local prefixCompat setup and manual prefix/length checks, delegating to isIPv4CompatBinary in the per-row loop condition.
Test coverage for all-zero address case
pkg/expression/builtin_miscellaneous_test.go, pkg/expression/builtin_miscellaneous_vec_test.go, pkg/expression/integration_test/integration_test.go
Unit, vectorized, and integration tests all verify that the all-zero IPv6 address input returns 0, covering scalar evaluation, vectorized batches, and end-to-end queries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A zero address dared to deceive,
But our helper now makes it quite clear:
IPv6's all-zeros shall not make us believe—
False it returns, the fix drawing near! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing IS_IPV4_COMPAT function behavior for the all-zero address case.
Description check ✅ Passed The description includes the required issue reference (#59458), problem summary, implementation explanation, comprehensive test coverage (unit and integration), release notes, and documentation flags.
Linked Issues check ✅ Passed The PR directly addresses issue #59458 by implementing the exact fix required: IS_IPV4_COMPAT now returns 0 for the all-zero IPv6 address instead of 1, matching MySQL behavior.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the IS_IPV4_COMPAT function across scalar, vectorized, and test implementations, with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hawkingrei hawkingrei added the AI-Correction Bugfix by AI label May 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.4894%. Comparing base (6a6eefe) to head (90b6864).

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #68438        +/-   ##
================================================
- Coverage   77.2762%   76.4894%   -0.7869%     
================================================
  Files          2010       1992        -18     
  Lines        555477     557618      +2141     
================================================
- Hits         429252     426519      -2733     
- Misses       125305     131054      +5749     
+ Partials        920         45       -875     
Flag Coverage Δ
integration 41.5432% <73.3333%> (+1.7492%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 60.4888% <ø> (ø)
parser ∅ <ø> (∅)
br 49.9725% <ø> (-13.0354%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hawkingrei
Copy link
Copy Markdown
Member Author

/retest-required

1 similar comment
@hawkingrei
Copy link
Copy Markdown
Member Author

/retest-required

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

Labels

AI-Correction Bugfix by AI release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IS_IPV4_COMPAT gives wrong result for all zero value

1 participant