Skip to content

[fix](regression) fix one_level_nestedtypes_with_s3data#64832

Merged
yiguolei merged 1 commit into
apache:masterfrom
csun5285:fix-doris26631-one-level-out
Jun 26, 2026
Merged

[fix](regression) fix one_level_nestedtypes_with_s3data#64832
yiguolei merged 1 commit into
apache:masterfrom
csun5285:fix-doris26631-one-level-out

Conversation

@csun5285

@csun5285 csun5285 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

#61995 (fix fix split_by_delimiter missing backslash escape handling) made the complex-type splitter skip escaped characters so that '"' inside a value no longer flips quote state.

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • 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
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…ter apache#61995

apache#61995 ([fix](serde) fix split_by_delimiter missing backslash escape
handling) made the complex-type splitter skip escaped characters so that
'\"' inside a value no longer flips quote state. That PR updated the .out
for the JSON-cast tests that run in per-PR CI (test_jsonb_cast,
test_jsonb_with_unescaped_string) but missed
one_level_nestedtypes_with_s3data, which only runs in the internal s3
daily. Its expected output therefore went stale and the daily regression
fails on the string-array columns (c_char/c_varchar/c_string) whose data
contains literal backslashes.

This regenerates the stale .out to match the intended post-apache#61995
behavior (same kind of fix as apache#62488). Only the string-array result
blocks change; numeric/bool/date array blocks are unaffected. Verified
against the s3 regression data on a local build.

Fixes DORIS-26631

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@csun5285 csun5285 changed the title [fix](regression) regenerate one_level_nestedtypes_with_s3data.out af… [fix](regression) fix one_level_nestedtypes_with_s3data Jun 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Possible file(s) that should be tracked in LFS detected: 🚨

The following file(s) exceeds the file size limit: 1048576 bytes, as set in the .yml configuration files:

  • regression-test/data/datatype_p0/nested_types/base_cases/one_level_nestedtypes_with_s3data.out

Consider using git-lfs to manage large files.

@github-actions github-actions Bot added the lfs-detected! Warning Label for use when LFS is detected in the commits of a Pull Request label Jun 25, 2026
@csun5285

Copy link
Copy Markdown
Contributor Author

run buildall

@csun5285

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot 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.

Reviewed PR #64832. The authoritative GitHub diff contains only regression-test/data/datatype_p0/nested_types/base_cases/one_level_nestedtypes_with_s3data.out, a generated expected-output refresh for the S3 nested-types regression. I did not find a concrete blocker or a review comment worth leaving inline.

Checkpoint summary:

  • Correctness/regression risk: no changed source, session/config, optimizer, rewrite, or execution code path; the hunks are confined to string-array expected results consistent with the escaped-delimiter behavior described in the PR.
  • Tests/output: the matching suite uses ordered outputs, the .out file keeps the same 253 result labels and 5780 lines as base, and there are no existing review threads to duplicate.
  • Subagent convergence: both required focused agents (optimizer-rewrite and tests-session-config) converged on NO_NEW_VALUABLE_FINDINGS for the final zero-inline-comment set.
  • Verification run: gh pr diff 64832 --name-only and git diff --check d48744d072d14436f58065023305c0c579b91486 4deace06fe983aad7515a867a84b8b514b8f4ebc -- regression-test/data/datatype_p0/nested_types/base_cases/one_level_nestedtypes_with_s3data.out.

Not run: the S3 regression itself, because it depends on the configured S3 daily test environment.

@yiguolei yiguolei merged commit 2893331 into apache:master Jun 26, 2026
35 of 36 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 26, 2026
#61995 ([fix](serde) fix split_by_delimiter missing backslash escape
handling) made the complex-type splitter skip escaped characters so that
'\"' inside a value no longer flips quote state.
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

### Release note

None

### Check List (For Author)

- Test <!-- At least one of them must be included. -->
    - [ ] Regression test
    - [ ] 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:
    - [ ] No.
    - [ ] Yes. <!-- Explain the behavior change -->

- Does this need documentation?
    - [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#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 -->

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev/4.1.x lfs-detected! Warning Label for use when LFS is detected in the commits of a Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants