morningman opened a new pull request, #69568:
URL: https://github.com/apache/doris/pull/69568

   ### What problem does this PR solve?
   
   Issue Number: None
   
   Related PR: #69567
   
   Problem Summary:
   
   **In short.** A CASE WHEN whose result type is VARBINARY and that has two or 
more WHEN branches failed in BE with `argument_type VarBinary not supported`. 
BE now selects VARBINARY values the way it selects strings.
   
   **Background.** VARBINARY values reach queries through external tables 
(Iceberg, Paimon and Hive binary columns map to VARBINARY) and through 
`CAST(... AS VARBINARY)`. BE evaluates CASE WHEN in `VCaseExpr`: it computes, 
for every row, the index of the branch that the row takes, and then 
`VCaseExpr::_execute_update_result` builds the result column, dispatching on 
the result's primitive type to a typed copy loop. Nereids rewrites a CASE with 
a single WHEN and an ELSE into `if()`, whose generic path copies values through 
`IColumn::insert_from` and accepts any column type.
   
   **The problem, and what it cost.** The dispatch in `_execute_update_result` 
had no entry for `TYPE_VARBINARY`, so it fell through to 
`NOT_IMPLEMENTED_ERROR`. Every query with such a CASE failed, whatever the 
constant-folding mode (the CASE is evaluated by BE, not folded), for example
   
   ```sql
   SELECT CASE WHEN number % 3 = 0 THEN CAST(concat('a', number) AS VARBINARY)
               WHEN number % 3 = 1 THEN CAST('y' AS VARBINARY)
               ELSE X'FF' END
   FROM numbers("number" = "6");
   -- ERROR 1105 (HY000): errCode = 2, detailMessage = 
(127.0.0.1)[NOT_IMPLEMENTED_ERROR][E3]
   -- argument_type Nullable(VarBinary) not supported
   ```
   
   Only a CASE with one WHEN and an ELSE worked, because it is evaluated as 
`if()`. branch-4.2 has the same gap.
   
   **How this PR fixes it.** `TYPE_VARBINARY` now maps to `ColumnVarbinary`, 
and `ColumnVarbinary` takes the per-row copy (`update_result_normal`) that CASE 
already uses for strings, both for a nullable result (including CASE without 
ELSE, whose unmatched rows are NULL) and for a non-nullable one. FE needs no 
change.
   
   **Results.**
   
   | CASE WHEN with a VARBINARY result | Before | After |
   |---|---|---|
   | Two or more WHEN branches, with ELSE | fails: `argument_type VarBinary not 
supported` | returns the selected bytes |
   | Two or more WHEN branches, without ELSE | fails: `argument_type 
Nullable(VarBinary) not supported` | returns the selected bytes, NULL for 
unmatched rows |
   | One WHEN with ELSE (rewritten to `if()`) | works | works (unchanged) |
   
   - BE unit tests: `VCaseVarbinaryTest` adds two cases through 
`_execute_update_result`: a non-nullable result with ELSE (a column branch, a 
constant branch, NUL and non-UTF-8 bytes) and a nullable result without ELSE 
(NULL from a branch and from unmatched rows). Both fail without the change with 
the error above. `VCaseSelectionTest` still passes (50 cases in total).
   - Regression test: 
`query_p0/sql_functions/binary_functions/test_varbinary_case_when` runs CASE 
with and without ELSE over a table, over `numbers()` and on constants, in the 
three constant-folding modes (FE folding, folding by BE, no folding); the 
results match in all modes.
   
   ### Release note
   
   Fix: a CASE WHEN with two or more WHEN branches and a VARBINARY result 
failed with "argument_type VarBinary not supported".
   
   ### Check List (For Author)
   
   - Test <!-- At least one of them must be included. -->
       - [x] Regression test
       - [x] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
           - [ ] This is a refactor/code format and no logic has been changed.
           - [ ] Previous test can cover this change.
           - [ ] No code files have been changed.
           - [ ] Other reason <!-- Add your reason?  -->
   
   - Behavior changed:
       - [x] No.
       - [ ] Yes. <!-- Explain the behavior change -->
   
   - Does this need documentation?
       - [x] No.
       - [ ] Yes. <!-- Add document PR link here. eg: 
https://github.com/apache/doris-website/pull/1214 -->
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases
   - [ ] Confirm document
   - [ ] Add branch pick label <!-- Add branch pick label that this PR should 
merge into -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to