MDEV-39453: mtr: make main.group_by MDEV-6129 UNION query deterministic#4991
MDEV-39453: mtr: make main.group_by MDEV-6129 UNION query deterministic#4991Alex-Tsvetanov wants to merge 1 commit intoMariaDB:11.4from
Conversation
The regression test for MDEV-6129 (server crash on UNION with ORDER BY <field IS NULL>) was: SET sql_mode='ONLY_FULL_GROUP_BY'; SELECT 1 AS test UNION SELECT 2 AS test ORDER BY test IS NULL ASC; The ORDER BY key evaluates to 0 for every row, so the resulting inter-row order is unspecified. Under parallel test load the server sometimes returns 2,1 instead of the 1,2 baked into the .result file, producing a spurious test failure. The bug this test guards is a crash, not a particular ordering, so the query itself should stay unchanged. Add --sorted_result before the SELECT so mtr sorts the client output before diffing. No .result change needed -- 1,2 is already in sorted order. Same line is present on 10.6..12.2.
|
Alex Tsvetanov seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Pull request overview
This PR makes the main.group_by MTR test for MDEV-6129 deterministic under parallel execution by removing reliance on unspecified row ordering from a UNION ... ORDER BY <constant expression> query.
Changes:
- Add
--sorted_resultbefore the MDEV-6129 regression query to make the observed row order stable for result comparison.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No source change. The previous CI run on this PR failed only in "server 11.4 test suite" on main.group_by, a nondeterministic UNION ordering in MariaDB/server's own mysql-test suite -- unrelated to the Schannel/async fix in 7cab116. Upstream fix at MariaDB/server#4991.
|
This has already been corrected in the 10.11 branch and is awaiting merge
up.
…On Saturday, April 25, 2026, Copilot ***@***.***> wrote:
***@***.**** commented on this pull request.
Pull request overview
This PR makes the main.group_by MTR test for MDEV-6129 deterministic
under parallel execution by removing reliance on unspecified row ordering
from a UNION ... ORDER BY <constant expression> query.
*Changes:*
- Add --sorted_result before the MDEV-6129 regression query to make
the observed row order stable for result comparison.
------------------------------
💡 Add Copilot custom instructions
<http:///MariaDB/server/new/main?filename=.github/instructions/*.instructions.md>
for smarter, more guided reviews. Learn how to get started
<https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot>
.
—
Reply to this email directly, view it on GitHub
<#4991 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADQ3T7SXE43GHCBC5TP2JL4XOZAXAVCNFSM6AAAAACYFRKOKSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCNZSG4YDOMRYGY>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AADQ3T2KWC4FGV6KRNRWXT34XOZAXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJXGI3TANZSHA3KM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSVGM33PORSXEX3JN5ZQ>
and Android
<https://github.com/notifications/mobile/android/AADQ3T6OZOJ7E5FTDA6UKKT4XOZAXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJXGI3TANZSHA3KM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSXGM33PORSXEX3BNZSHE33JMQ>.
Download it today!
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
As I mentioned, it blocked the full test suite from executing for a PR I opened regarding async API crashes. Are there any estimates on when we can expect this merged?
|
gkodinov
added
the
External Contribution
gkodinov
changed the title
|
Thank you for your contribution! As Daniel pointed out, this has been fixed already in the right version: #4946. On your ETA question. This has been pushed to 10.11 relatively soon. Usually it takes a couple of weeks to merge it up fully. Благодаря за търпението! :) |
Problem
main.group_byis flaky in the mtr suite. The failing case is the regression test for MDEV-6129 (server crash on UNION withORDER BY <expr> IS NULL):Both rows have
test IS NULL = 0, so the ORDER BY key is constant and the row order is unspecified. Under parallel test load (mtr --parallel=auto) the server sometimes returns2,1instead of the1,2baked intogroup_by.result:Repro seen recently on the
mariadb-connector-cCI (parallel build, clean 11.4 tree): https://github.com/mariadb-corporation/mariadb-connector-c/actions/runs/24903791217/job/72927889893 — and on the scheduled 3.4 run of 2026-04-19 against the same 11.4 server (https://github.com/mariadb-corporation/mariadb-connector-c/actions/runs/24619796993/job/71988293365), which had no libmariadb changes at all.Fix
MDEV-6129 was a crash fix; the regression test exists to prove the query parses, executes, and returns without crashing — the inter-row order of
1vs2is incidental. So keep the exact query unchanged and add--sorted_result, which tells mtr to sort the client-visible output before diffing:SET sql_mode='ONLY_FULL_GROUP_BY'; +--sorted_result SELECT 1 AS test UNION SELECT 2 AS test ORDER BY test IS NULL ASC; SET sql_mode='';No
.resultfile change needed —1\n2is already in sorted order, and the variant seen in the wild (2\n1) sorts to the same thing.Branch coverage
The same line appears unchanged on
10.6,10.11,11.0,11.2,11.4,11.8,12.0,12.1,12.2. Targeting11.4per MariaDB's merge-up convention; maintainers may want to cherry-pick to older branches (10.6/10.11) as well.Scope
Single line added in
mysql-test/main/group_by.test. No server code, no other tests, no.resultfiles changed.