Skip to content

[fix](regression) Deflake segcompaction agg keys case#65382

Open
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:fix-segcompaction-agg-keys-master
Open

[fix](regression) Deflake segcompaction agg keys case#65382
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:fix-segcompaction-agg-keys-master

Conversation

@shuke987

@shuke987 shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Proposed changes

Follow up the existing test_segcompaction_agg_keys deflake from #61026.

The current master case already avoids the old golden file by allowing two concrete full-row results for col_0=47. This patch keeps the same intent but removes the hard-coded full-row candidates. The case now checks stable invariants instead:

  • col_0=47 returns exactly one aggregate row.
  • The row has the expected 50 columns.
  • The key is 47.
  • All REPLACE value columns belong to the known fruit value domain in the ORC input.

This avoids pinning the test to a specific duplicate-key REPLACE winner while still checking that the load and aggregation returned a valid row.

Why

The ORC input contains multiple rows with col_0=47. With AGGREGATE KEY(col_0) and REPLACE value columns, the case should not require one specific source row to win unless the data/model makes that winner deterministic.

Validation

Remote branch-4.1 regression environment:

ssh root@172.20.56.184
cd /deploy/teamcity_agent13/work/a0477ef061788419/case_output/doris
export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64
export PATH=$JAVA_HOME/bin:$PATH
./run-regression-test.sh --run -d segcompaction_p2 -s test_segcompaction_agg_keys

Result:

Test 1 suites, failed 0 suites, fatal 0 scripts, skipped 0 scripts

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

@shuke987

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

run buildall

@shuke987

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator 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.

Review result: Request changes.

Critical checkpoints:

  • Goal and test coverage: the PR intends to deflake test_segcompaction_agg_keys, but the new assertion no longer proves the full aggregate-key REPLACE winner row; see the inline comment.
  • Scope: the change is focused to one regression test file. No production code, config, session propagation, persistence, or compatibility change is present.
  • Parallel paths: sibling segcompaction outputs expose the complete duplicate source rows, so the test can preserve row-level coverage without pinning one deterministic winner.
  • Concurrency, lifecycle, data writes, and observability: not applicable to this test-only assertion change.
  • Validation: static review only in this runner. .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc are absent, so I did not run the regression suite locally.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions:

  • optimizer-rewrite: no new valuable findings; the optimizer/rewrite concern was dismissed as not applicable to this one-file test assertion change.
  • tests-session-config: TSC-001 was accepted as MAIN-001 and submitted inline.
  • Convergence: round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.

assertEquals(50, result[0].size())
assertEquals("47", result[0][0].toString())

def validValues = [

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.

This is too weak for the regression: after this change any row whose 49 value cells are fruit names passes, even if the row is assembled from different duplicate rows or the wrong source columns. The same data set already exposes the 12 complete col_0=47 source rows in test_segcompaction_dup_keys.out, and the aggregate-key REPLACE result should be one complete winner row, not just 49 independently valid vocabulary entries. To keep the deflake while preserving coverage, please compare result[0] against the set of complete allowed source rows instead of checking each cell against validValues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants