Skip to content

GH-50136: [C++][Gandiva] Enhance CHR to work with unicode - #50137

Merged
kou merged 8 commits into
apache:mainfrom
lriggs:gh-50136
Aug 13, 2026
Merged

GH-50136: [C++][Gandiva] Enhance CHR to work with unicode#50137
kou merged 8 commits into
apache:mainfrom
lriggs:gh-50136

Conversation

@lriggs

@lriggs lriggs commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

CHR(n) only worked for ASCII (0–127). Values ≥ 128 emitted a single raw byte
(invalid UTF‑8), causing "Error during planning". Goal: emit the proper
multi‑byte UTF‑8 encoding of the Unicode code point, consistent with
PostgreSQL/Snowflake.

What changes are included in this PR?

Arrow (C++ / Gandiva)

File Change
cpp/src/gandiva/precompiled/string_ops.cc chr_int64 rewritten to UTF‑8‑encode the code point (1–4 bytes) and error on invalid input (negative, > 0x10FFFF, surrogate range 0xD800–0xDFFF). chr_int32 now delegates to it.
cpp/src/gandiva/precompiled/string_ops_test.cc TestChrBigInt rewritten for UTF‑8 semantics: every byte‑length boundary (1/2/3/4‑byte, low+high), í/€/日/😀, and the three invalid‑input error cases.

Are these changes tested?

Yes, unit tests.

Are there any user-facing changes?

Yes, the CHR gandiva function now supports unicode characters.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #50136 has been automatically assigned in GitHub to PR creator.

@selvaganesang selvaganesang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jun 10, 2026
@lriggs lriggs changed the title GH-50136 GH-50136: [C++][Gandiva] Enhance CHR to work with unicode. Jun 11, 2026
@lriggs

lriggs commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@kou any chance this can be merged?

@kou
kou requested a lite review from Copilot August 12, 2026 05:28
@kou kou changed the title GH-50136: [C++][Gandiva] Enhance CHR to work with unicode. GH-50136: [C++][Gandiva] Enhance CHR to work with unicode Aug 12, 2026

Copilot AI 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.

Pull request overview

Updates Gandiva’s CHR(n) implementation to correctly emit UTF-8 for Unicode code points (instead of raw bytes), aligning behavior with common SQL engines and preventing invalid UTF-8 from causing planning failures.

Changes:

  • Rewrites chr_int64 to UTF-8 encode valid Unicode code points (1–4 bytes) and reject invalid inputs (negative, > 0x10FFFF, surrogate range).
  • Makes chr_int32 delegate to chr_int64 for consistent semantics.
  • Updates/extends unit and integration-style tests to validate UTF-8 boundaries, representative characters, and invalid-input error behavior.

Reviewed changes

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

File Description
cpp/src/gandiva/precompiled/string_ops.cc Implements UTF-8 encoding and validation for chr_int64; chr_int32 delegates.
cpp/src/gandiva/precompiled/string_ops_test.cc Expands CHR unit tests to cover UTF-8 boundaries, sample Unicode characters, and invalid code points.
cpp/src/gandiva/tests/projector_test.cc Updates projector-level CHR test for UTF-8 output and adds an invalid-input evaluation test.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cpp/src/gandiva/precompiled/string_ops.cc Outdated
Comment thread cpp/src/gandiva/precompiled/string_ops.cc Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 12, 2026
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 12, 2026

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@kou
kou merged commit 57139b0 into apache:main Aug 13, 2026
50 of 58 checks passed
@kou kou removed the awaiting change review Awaiting change review label Aug 13, 2026
@github-actions github-actions Bot added the awaiting merge Awaiting merge label Aug 13, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 57139b0.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 159 possible false positives for unstable benchmarks that are known to sometimes produce them.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants