Skip to content

[chore](test) Make results of array_union sorted by array_sort in test cases#61333

Merged
yiguolei merged 2 commits intoapache:masterfrom
mrhhsg:fix_array_union
Mar 15, 2026
Merged

[chore](test) Make results of array_union sorted by array_sort in test cases#61333
yiguolei merged 2 commits intoapache:masterfrom
mrhhsg:fix_array_union

Conversation

@mrhhsg
Copy link
Member

@mrhhsg mrhhsg commented Mar 14, 2026

What problem does this PR solve?

The result of array_union does not guarantee element order, so the outputs of its related test cases are also unstable.
Use array_sort to make the results deterministic.

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

@github-actions
Copy link
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/suites/nereids_p0/sql_functions/array_functions/sql/q04.sql

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 Mar 14, 2026
@mrhhsg
Copy link
Member Author

mrhhsg commented Mar 14, 2026

run buildall

@mrhhsg
Copy link
Member Author

mrhhsg commented Mar 14, 2026

/review

@github-actions
Copy link
Contributor

Code Review Summary

PR: chore Make results of array_union sorted by array_sort in test cases

This PR wraps array_union() calls with array_sort() in regression test cases to make test results deterministic, since array_union does not guarantee element order. The change is purely test-stabilization — no product code is modified.

Critical Checkpoint Conclusions

  • Goal and correctness: The goal is to eliminate flaky test results caused by non-deterministic array_union output ordering. The approach of wrapping with array_sort() is correct and minimal. All array_union calls producing checked results are now wrapped; exception-test paths are correctly left unwrapped.

  • Modification scope: The change is minimal, clear, and focused — only array_sort() wrappers are added around existing array_union() calls. No test logic, table definitions, or test coverage is altered.

  • Concurrency: N/A — test-only changes with no concurrency implications.

  • Lifecycle management: N/A — no lifecycle changes.

  • Configuration items: N/A — no configuration changes.

  • Incompatible changes: N/A — no format or symbol changes.

  • Parallel code paths: The three repeated blocks in test_array_functions_by_literal.groovy (for different enable_fold_constant_by_be settings) are all consistently updated.

  • Special conditional checks: N/A.

  • Test coverage: The existing test coverage is preserved. The .out files reflect the sorted output. The q04.out file changes are consistent with a regenerated output after adding array_sort() and an extra CREATE TABLE statement in q04.sql (tag numbering shift from 5 to 6 statements is correct).

  • Observability: N/A — test-only changes.

  • Transaction/persistence: N/A.

  • Data writes: N/A.

  • FE-BE variable passing: N/A.

  • Performance: N/A — test-only changes.

  • Other issues: None identified. The use of qt_sql (rather than order_qt) is acceptable because: (1) table queries operate on single-row tables, and (2) literal-only queries have no row-ordering ambiguity.

Regression Test Standards Compliance

  • array_sort() used for deterministic output: Yes
  • Expected errors use test { sql ...; exception ... }: Yes (lines 401-414 in test_array_function.groovy)
  • Tables dropped before use, not after: Consistent with existing pattern
  • .out files appear auto-generated (consistent formatting): Yes

Verdict: No issues found. This is a clean test-stabilization change.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 14, 2026
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@yiguolei yiguolei merged commit 5fa5e0e into apache:master Mar 15, 2026
34 of 36 checks passed
mrhhsg added a commit to mrhhsg/doris that referenced this pull request Mar 15, 2026
…t cases (apache#61333)

The result of `array_union` does not guarantee element order, so the
outputs of its related test cases are also unstable.
Use `array_sort` to make the results deterministic.

Related PR: #xxx

Problem Summary:

None

- 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 -->

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
mrhhsg added a commit to mrhhsg/doris that referenced this pull request Mar 15, 2026
yiguolei pushed a commit that referenced this pull request Mar 15, 2026
…t cases (#61333) (#61344)

pick #61333

The result of `array_union` does not guarantee element order, so the
outputs of its related test cases are also unstable. Use `array_sort` to
make the results deterministic.

Related PR: #xxx

Problem Summary:

None

- 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 -->

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->

### 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 -->
yiguolei pushed a commit that referenced this pull request Mar 16, 2026
…t cases (#61333) (#61344)

pick #61333

The result of `array_union` does not guarantee element order, so the
outputs of its related test cases are also unstable. Use `array_sort` to
make the results deterministic.

Related PR: #xxx

Problem Summary:

None

- 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 -->

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->

### 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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.0-merged lfs-detected! Warning Label for use when LFS is detected in the commits of a Pull Request reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants