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

   ### Transparency
   
   I had this issue and used Claude (Opus 5) to analyse the report, 
reproduction scenario I had, and cause, and locate if it was already fixed. It 
found the below issue which on paper looks fine. I had it create a test and 
ensure it failed, then proceeded to have it resolve the failure.
   
   I'm no where near an expert at this level but on the surface this all looks 
a genuine issue. I haven't been able to test this fully and have relied solely 
on the tests that were written.
   
   What follows below is the AI generated summary, and all code changes are AI 
generated. I've vetted and checked to the best of my ability. My apologies if I 
made any mistake.
   
   For reference, here is the full analysis AI generated: 
https://gist.github.com/driskell/c37d730bff4e1ecf740e0807184eac4f
   Please note - the "secondary defect" this analysis noted was later found by 
a secondary agent to already be resolved (indirectly, as in the target code I 
think was unchanged but the call chains surrounding it no longer call it in the 
problematic way) - so this PR targets only the main issue I had.
   
   ----- AI generated summary (checked) -----
   
   ### What problem does this PR solve?
   
   Issue Number: None
   
   Related PR: None
   
   Problem Summary: from_string_strict_mode_batch in the number and decimal 
serdes walked the source ColumnString with a running cursor that was left 
behind on the rows the null map marks NULL, so every later row was parsed as 
the skipped rows' bytes glued onto its own. It stays hidden while NULL rows 
carry an empty nested slice, which is what the null-producing insert entry 
points give them, but apply_null_map() only ORs the null map, so if(c = '-', 
NULL, c) leaves the original bytes and a non-zero offset delta in place. One 
skipped '-' ahead of 2628 then parses as -2628 and is stored silently, while a 
run of them fails with parse number fail. Index per row through get_data_at(i) 
instead, as the date, datetime, ipv4, ipv6 and time serdes already do.
   
   ### Release note
   
   Fix a strict-mode cast from string to a number or decimal that could return 
wrong values, or fail with parse number fail, when an expression such as if(c = 
'-', NULL, c) marked rows NULL without clearing their bytes from the underlying 
string column.
   
   ### 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?  -->
   
   - Test details
       - test_cast_string_to_number_with_nulls covers bigint and decimal 
targets, one NULL row and a run of them, and the insert ... select path that 
reaches strict mode through enable_insert_strict
       - ./run-be-ut.sh --run 
--filter=DataTypeNumberSerDeFromStringStrictModeBatchTest.*:DataTypeDecimalSerDeFromStringStrictModeBatchTest.*:ColumnNullableTest.*
   
   - 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