Skip to content
/ server Public

MDEV-38752: Wrong result upon GROUP BY on a table with indexed virtual column after INSERT IGNORE#4759

Open
kjarir wants to merge 1 commit intoMariaDB:12.3from
kjarir:MDEV-38752-group-by-vcol-subst
Open

MDEV-38752: Wrong result upon GROUP BY on a table with indexed virtual column after INSERT IGNORE#4759
kjarir wants to merge 1 commit intoMariaDB:12.3from
kjarir:MDEV-38752-group-by-vcol-subst

Conversation

@kjarir
Copy link

@kjarir kjarir commented Mar 9, 2026

Description

This PR fixes MDEV-38752, a regression introduced in commit 8cdee25952763a0401e4c2a4d61e92c13499bdc6.

The issue was caused by the optimizer incorrectly substituting expressions in GROUP BY or ORDER BY with indexed virtual columns that had "narrower" data types (e.g., substituting an INT expression with a TINYINT virtual column). After an INSERT IGNORE, data truncation in the virtual column caused distinct values from the original expression to merge into a single group in the query result.

Changes

  • Updated subst_vcol_if_compatible() in sql/opt_vcol_substitution.cc to perform stricter type compatibility checks.
  • Substitution is now only allowed if the virtual column's Type_handler matches the expression and its max_length and decimals are sufficient to cover the expression's range without truncation.
  • Optimization remains active for safe cases where no data loss occurs.

Testing

  • Added regression test mysql-test/main/mdev_38752.test covering:
    • Numeric narrowing (INT to TINYINT).
    • String narrowing (VARCHAR(20) to VARCHAR(5)).
    • Verification that safe substitutions still use the virtual column index.
Screenshot 2026-03-09 at 1 49 38 PM Screenshot 2026-03-09 at 1 37 52 PM

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 9, 2026
@gkodinov gkodinov force-pushed the MDEV-38752-group-by-vcol-subst branch from aa9523d to 1002ee1 Compare March 9, 2026 09:04
Copy link
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This is a preliminary review.

Please rebase to 12.3: this is a bug fix and it needs to cover all affected versions

CREATE TABLE t1 (a INT PRIMARY KEY, va TINYINT AS (a), KEY(va));
INSERT IGNORE INTO t1 (a) VALUES (100),(150),(200);
--echo # Should return 3 rows (fix: substitution disallowed)
SELECT a, va, COUNT(*) FROM t1 GROUP BY a;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please increase verbosity so that the note about vcol impossible substitution is actually printed. Or do an EXPLAIN: it should do it too.

SELECT a, va, COUNT(*) FROM t2 GROUP BY a;
DROP TABLE t2;

--echo # Case 3: Unsafe narrowing for VARCHAR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for different decimals as well please.

--echo # 'apple' and 'apricot' both truncate to 'apple' in va
--echo # Should return 3 rows
SELECT a, va, COUNT(*) FROM t3 GROUP BY a;
DROP TABLE t3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we customarily end tests with a print like that:

--echo End of ... tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also make sure all of the related failing tests are re-recorded.

@kjarir kjarir force-pushed the MDEV-38752-group-by-vcol-subst branch 2 times, most recently from 05fd327 to b33ef30 Compare March 9, 2026 10:12
@CLAassistant
Copy link

CLAassistant commented Mar 9, 2026

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ forkfun
✅ dbart
✅ kjarir
✅ ericherman
❌ knielsen
You have signed the CLA already but the status is still pending? Let us recheck it.

@kjarir kjarir changed the base branch from main to 12.3 March 9, 2026 10:21
@kjarir kjarir force-pushed the MDEV-38752-group-by-vcol-subst branch 2 times, most recently from 21e3d7f to 7bf01fe Compare March 9, 2026 12:55
@kjarir kjarir force-pushed the MDEV-38752-group-by-vcol-subst branch from 7bf01fe to 17e3155 Compare March 9, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

3 participants