GH-50136: [C++][Gandiva] Enhance CHR to work with unicode - #50137
Conversation
|
|
|
@kou any chance this can be merged? |
There was a problem hiding this comment.
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_int64to UTF-8 encode valid Unicode code points (1–4 bytes) and reject invalid inputs (negative, > 0x10FFFF, surrogate range). - Makes
chr_int32delegate tochr_int64for 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.
|
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. |
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)
cpp/src/gandiva/precompiled/string_ops.ccchr_int64rewritten to UTF‑8‑encode the code point (1–4 bytes) and error on invalid input (negative, > 0x10FFFF, surrogate range 0xD800–0xDFFF).chr_int32now delegates to it.cpp/src/gandiva/precompiled/string_ops_test.ccTestChrBigIntrewritten 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.