[BugFix] Never raise a strict cast overflow error from a null row's data

[trueeyu]

Why I'm doing:

A strict-mode numeric narrowing cast (the AllowThrowException path of UNARY_FN_CAST_VALID,
e.g. cast(x AS INT) when an overflow must error instead of becoming NULL) evaluated the overflow
check on every row of the input, including null rows. The data stored in a null row's slot
is undefined: when a producer leaves garbage there, the check sees an out-of-range value and throws
a spurious exception, failing a query that should succeed — the row is NULL and its value is
irrelevant.

What I'm doing:

• Add NullAwareInputCheckUnaryFunction (be/src/exprs/unary_function.h). For a nullable input it
  converts all rows but only treats a non-null row's overflow as an error; a null row's data
  can never be the cause of an exception. The hot loop stays branchless and vectorizable (convert
  every row, detect overflow with the non-throwing check AND-ed with the not-null mask and
  OR-accumulated); the throwing check runs only on a cold path entered when a genuine non-null row
  overflows.
• Wire the strict (AllowThrowException) branch of UNARY_FN_CAST_VALID to use it, passing the
  non-throwing NumberCheck for hot-loop detection and NumberCheckWithThrowException for the cold
  error path.

Bonus: the new implementation is ~1.4-1.6x faster than the old one across nullable/non-nullable
shapes — it avoids materializing and scanning a separate null-mask column and keeps the loop
vectorizable (no per-row branch, no throw in the hot path). Benchmark numbers are in the bench file.

What type of PR is this:

• BugFix
• Feature
• Enhancement
• Refactor
• UT
• Doc
• Tool

Does this PR entail a change in behavior?

• Yes, this PR will result in a change in behavior.
• No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

• Interface/UI changes: syntax, type conversion, expression evaluation, display information
• Parameter changes: default values, similar parameters but with different default values
• Policy changes: use new policy to replace old one, functionality automatically enabled
• Feature removed
• Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

• I have added test cases for my bug fix or my new feature
• This pr needs user documentation (for new or modified features or behaviors)
  • I have added documentation for my new feature or new function
  • This pr needs auto generate documentation
• This is a backport pr

Bugfix cherry-pick branch check:

• I have checked the version labels which the pr will be auto-backported to the target branch
  • 4.1
  • 4.0
  • 3.5

[Copilot]

Pull request overview

Fixes a correctness bug in the native Parquet dictionary decoder where NULL rows could leave their backing data slots uninitialized, which can trigger false range-check failures during narrowing casts (e.g., under sql_mode='ALLOW_THROW_EXCEPTION'). The change ensures deterministic zero-filling for the null-heavy decode paths and adds both SQL and BE-unit regressions to guard against reintroduction.

Changes:

• Zero-initialize newly grown buffers in dict decode-with-nulls paths by switching from resize_uninitialized() to resize().
• Add a deterministic BE unit test that poisons the destination buffer and verifies NULL slots are zero-filled (covers sparse and filter-forwarding branches).
• Add a Hive SQL regression reproducer for the original cast failure scenario.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
be/src/formats/parquet/encoding_dict.h	Zero-fills nullable dict decode output slots to prevent uninitialized reads during casts; adds a test hook to control dict-size threshold.
be/test/formats/parquet/dict_encoding_test.cpp	Refactors existing tests slightly and adds a regression test validating NULL-slot zero-fill behavior.
test/sql/test_hive/T/test_hive_parquet_dict_null_cast	New SQL test that exercises dict-encoded, mostly-NULL Parquet data narrowed to INT under throw-checking mode.
test/sql/test_hive/R/test_hive_parquet_dict_null_cast	Expected results for the new SQL test.

Comments suppressed due to low confidence (1)

be/test/formats/parquet/dict_encoding_test.cpp:82

• The RleEncoder does not flush pending state on scope exit (it has no destructor calling Flush), so the last run may not be written to fs. This can make the test’s encoded stream incomplete and potentially hide regressions if the decoded count changes. Call encoder.Flush() after the loop to ensure the backing buffer contains all encoded values.

        RleEncoder<DICT_CXX_TYPE> encoder(&fs, 32);
        for (size_t i = 0; i < 4096; ++i) {
            encoder.Put(i % 9 + 1, 10);
        }
    }

[CelerData-Reviewer]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afc1ce85e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[stdpain]

I suggest a better fix: Call NullableColumn::fill_null_with_default before importing the side and handling the cast. This change will not solve the root cause, because OLAP scans or columns calculated from an expression will also exhibit the same behavior.

[trueeyu]

I suggest a better fix: Call NullableColumn::fill_null_with_default before importing the side and handling the cast. This change will not solve the root cause, because OLAP scans or columns calculated from an expression will also exhibit the same behavior.

I split the changes into two PRs. This one fixes the direct root cause. The next PR will address the cast issue after passing performance tests.

[dirtysalt]

LGTM! 🚀

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

[github-actions]

@Mergifyio backport branch-4.0

[github-actions]

@Mergifyio backport branch-3.5

[mergify]

backport branch-4.0

✅ Backports have been created

Details

• #74957 [BugFix] Never raise a strict cast overflow error from a null row's data (backport #74903) has been created for branch branch-4.0 but encountered conflicts

Cherry-pick of 5847dd7 has failed:

On branch mergify/bp/branch-4.0/pr-74903
Your branch is up to date with 'origin/branch-4.0'.

You are currently cherry-picking commit 5847dd7825.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   be/src/bench/cast_overflow_check_bench.cpp
	modified:   be/src/exprs/cast_expr.cpp
	modified:   be/src/exprs/unary_function.h
	modified:   be/test/CMakeLists.txt
	new file:   be/test/exprs/unary_function_test.cpp
	new file:   test/sql/test_hive/R/test_hive_parquet_dict_null_cast
	new file:   test/sql/test_hive/T/test_hive_parquet_dict_null_cast

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/src/bench/CMakeLists.txt

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[github-actions]

@Mergifyio backport branch-4.1

[mergify]

backport branch-3.5

✅ Backports have been created

Details

• #74958 [BugFix] Never raise a strict cast overflow error from a null row's data (backport #74903) has been created for branch branch-3.5 but encountered conflicts

Cherry-pick of 5847dd7 has failed:

On branch mergify/bp/branch-3.5/pr-74903
Your branch is up to date with 'origin/branch-3.5'.

You are currently cherry-picking commit 5847dd7825.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   be/src/bench/cast_overflow_check_bench.cpp
	modified:   be/src/exprs/cast_expr.cpp
	modified:   be/src/exprs/unary_function.h
	modified:   be/test/CMakeLists.txt
	new file:   be/test/exprs/unary_function_test.cpp
	new file:   test/sql/test_hive/R/test_hive_parquet_dict_null_cast
	new file:   test/sql/test_hive/T/test_hive_parquet_dict_null_cast

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/src/bench/CMakeLists.txt

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[mergify]

backport branch-4.1

✅ Backports have been created

Details

• #74959 [BugFix] Never raise a strict cast overflow error from a null row's data (backport #74903) has been created for branch branch-4.1 but encountered conflicts

Cherry-pick of 5847dd7 has failed:

On branch mergify/bp/branch-4.1/pr-74903
Your branch is up to date with 'origin/branch-4.1'.

You are currently cherry-picking commit 5847dd7825.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   be/src/bench/cast_overflow_check_bench.cpp
	modified:   be/src/exprs/cast_expr.cpp
	modified:   be/src/exprs/unary_function.h
	modified:   be/test/CMakeLists.txt
	new file:   be/test/exprs/unary_function_test.cpp
	new file:   test/sql/test_hive/R/test_hive_parquet_dict_null_cast
	new file:   test/sql/test_hive/T/test_hive_parquet_dict_null_cast

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/src/bench/CMakeLists.txt

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally