MDEV-40977 DOUBLE UNSIGNED column as shift right operand yields inconsistent results between direct query and scalar subquery (0 vs 1) due to different integer conversion paths. by DerZc · Pull Request #5722 · MariaDB/server

DerZc · 2026-09-21T04:59:08Z

A large shift count supplied by a DOUBLE UNSIGNED expression can produce inconsistent bit-shift results between direct evaluation and scalar-subquery evaluation. A shift count outside the 64-bit operand width should produce zero.

Bug report: https://jira.mariadb.org/browse/MDEV-40977

Root cause

Longlong_null's left- and right-shift operators narrow the evaluated shift count to uint before checking the operand width. High bits are discarded, so an out-of-range count can become a small in-range count and cause an unintended shift.

Changes

Keep the shift count as ulonglong in both shift operators.
Compare the full-width value with the operand's bit width before shifting.
Preserve the existing NULL result when either operand is NULL.

Regression coverage

The regression is integrated into the existing main.func_bit test.

The regression compares direct access to a DOUBLE UNSIGNED column with a scalar subquery over the same one-row table, for both left and right shifts. Both forms must return zero for the large shift count. Additional checks cover a count of 2^32, the 64-bit width boundary, and NULL arguments. SQL statements, column headings, results, and warnings use normal MTR output.

mysql-test/main/func_bit.result
mysql-test/main/func_bit.test

Validation

On 11.4 at d10e5d726799b1cd57cc866f40aad68c720803da:

The server build passed.
The complete main.func_bit test, including the integrated regression, failed on the unchanged target branch at the regression case and passed with this fix.
Existing MTR tests passed: main.select, main.subselect, main.type_decimal, main.type_float, main.func_bit.
MTR reported no test-state cleanup failures.
The full regression suite was not run.
Focused checks of the Longlong_null shift operators passed in both 32-bit and 64-bit executables. The original operators fail the large-count cases on both ABIs. A full 32-bit server MTR run was not performed locally.

CLAassistant · 2026-09-21T04:59:17Z

All committers have signed the CLA.

dr-m · 2026-09-21T05:14:30Z

@@ -0,0 +1,18 @@
+# Regression: bug_40977


Can you please come up with a descriptive name of the test, instead of using a ticket number as a basis of the test file name?

The name bug_40977 is not only non-descriptive, it is also misleading, because it could be mistaken for https://bugs.mysql.com/bug.php?id=40977 which has nothing to do with https://jira.mariadb.org/browse/MDEV-40977 which is what this test is about.

Thanks for pointing out the ambiguity. I've renamed the files to bit_shift_large_count.test and bit_shift_large_count.result, with the MDEV reference in the test heading.

Thank you, that is much better. A slightly more generic test name main.bit_shift would make it usable for other tests in the same area. There is some per-test-file overhead, because at the start and each test file the server state is being checked. Therefore, it is better to group many simple tests in a single file. An alternative might be to add this test to the end of the existing file main.func_bit. Currently, it only includes 0-bit shifts.

I am not responsible for this subsystem, so I can’t review this change.

Thanks, that makes sense. I've renamed the test to main.bit_shift, so related shift cases can share the same file. The revised test and the existing bit-function, SELECT, subquery, decimal, and floating-point tests all passed.

dr-m · 2026-09-21T05:17:33Z

+--enable_result_log
+--disable_column_names
+--sorted_result
+--query SELECT ((1503381287 << v) AND -553173254) AS result FROM (SELECT l.vp_rowid,(SELECT r.v FROM right_t r WHERE r.vp_rowid=l.vp_rowid) AS v FROM left_t l) AS vp_rel;


What is the justification for disable_column_names? The AS result is producing a short column name. Disabling almost all output makes the .result file much harder to read. Apparently, the 0 in it must be the result of this SELECT statement.

Is the AS vp_rel relevant for reproducing the bug?

Agreed, the SQL and column headings are now visible in the result file. The name vp_rel is not relevant to reproducing the bug, and the derived-table wrapper was unnecessary as well. I've reduced the setup to one table and compare direct column access with a scalar subquery, for both << and >>.

On unmodified 11.4, the direct query returns (0, 0) but the scalar-subquery form returns (1, 1); with the fix, both return (0, 0). The test also checks counts of 2^32 and 64, and NULL arguments. The revised testcase fails before the fix and passes afterward, together with main.func_bit, main.select, main.subselect, main.type_decimal, and main.type_float.

One thing that you and the reviewer should check is what would happen on a 32-bit server, such as x86-debian-12-fulltest. The queue on that one currently is a few hours. This builder does not report its status to GitHub. The status won’t be visible in the Buildbot grid view before the build has been scheduled.

Thanks for flagging the 32-bit case. I compiled and ran the actual Longlong_null class definitions from both revisions in separate 32-bit and 64-bit executables, using g++ -m32 and -m64. On both ABIs, uint is 32 bits and ulonglong is 64 bits, so the old conversion wraps a count of 2^32 to zero.

The patched operators passed 52 value checks and four NULL checks on each ABI, covering both shift directions, counts around 32 and 64, 2^32, and values reaching the high bit of the 64-bit count. The original operators failed 11 value checks on each ABI.

These are focused checks of the shift operators; I have not run a full 32-bit server MTR suite locally. The 64-bit MTR regression and related tests passed, but the x86-debian-12-fulltest result should still be checked for the complete server path.

…sistent results between direct query and scalar subquery (0 vs 1) due to different integer conversion paths. A large shift count supplied by a DOUBLE UNSIGNED expression can produce inconsistent bit-shift results between direct evaluation and scalar-subquery evaluation. A shift count outside the 64-bit operand width should produce zero. Longlong_null's left- and right-shift operators narrow the evaluated shift count to uint before checking the operand width. High bits are discarded, so an out-of-range count can become a small in-range count and cause an unintended shift. Keep the shift count as ulonglong in both shift operators. Compare the full-width value with the operand's bit width before shifting. Preserve the existing NULL result when either operand is NULL. The regression compares direct access to a DOUBLE UNSIGNED column with a scalar subquery over the same one-row table, for both left and right shifts. Both forms must return zero for the large shift count. Additional checks cover a count of 2^32, the 64-bit width boundary, and NULL arguments. SQL statements, column headings, results, and warnings use normal MTR output. Bug report: https://jira.mariadb.org/browse/MDEV-40977

gkodinov

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

Please squash the two commits into a single one.

gkodinov · 2026-09-23T10:18:30Z

+CREATE TABLE t1 (shift_count DOUBLE UNSIGNED NOT NULL);
+
+INSERT INTO t1 VALUES (9223372036854775807);
+


please don't do these empty lines. it's against the formatting of the rest of the tests.

Please use empty lines for semantical blocks devision, not like each query interleave below.

gkodinov · 2026-09-23T10:25:14Z

  Longlong_null operator<<(const Longlong_null &llshift) const
  {
    ulonglong res;
-    uint shift;


I believe this is a wrong fix. You cannot shift more than sizeof(longlong) * 8 anyway. Why expand the data type? Why not just reject it (return error) instead of returning a NULL?

Thank you @gkodinov for reviewing this. I agree that the actual shift must only execute when the count is below sizeof(longlong) * 8; that guard is unchanged.

The wider local variable preserves the shift count until the range check. Currently, the cast to uint happens first and can discard the bits that would make the check reject the count. For example, 4294967296 (2^32) becomes zero when cast to a 32-bit uint, so the guard accepts it and 1 << 4294967296 incorrectly evaluates as 1 << 0, returning 1. The same happens for 1 >> 4294967296. With ulonglong, the full count reaches the guard, no out-of-range C++ shift is executed, and the result stays zero. This example also demonstrates the problem without involving DOUBLE conversion.

The out-of-range branch returns numeric 0, not NULL: res is initialized to zero, and the value-taking Longlong_null(res) constructor sets the NULL flag to false. The default Longlong_null() constructor is used when an input operand is NULL; that behavior is unchanged. Returning an error for an excessive count would change the existing behavior—for example, 1 << 64 already returns zero. The MySQL numeric shift documentation specifies the same zero-result behavior.

Checking the original full-width value before narrowing it would also avoid this bug. The current patch keeps the full-width count through the existing guard in both operators, without changing the operand width or the valid shift range.

For validation, the integrated main.func_bit regression fails on the unmodified target revision and passes with the patch. The focused tests using the actual class definitions also pass all 52 value checks and four NULL checks on both 32-bit and 64-bit executables; the original operators fail 11 value checks on each. These are operator-level ABI checks, not a full 32-bit server MTR run, which I have not performed locally.

Do you see any point in supporting 1 << 4294967296? I believe it's more valuable to flag this as an error. Because that's what it is: an error in the higher level logic.
Even the C++ standard says that the behavior is undefined for these. Why try to define it?
It's a wrong idea and this IMHO should be a bug.

But, as I said, this is a preliminary review. And this is an optional comment. Please do the other two and we shall let the final reviewer decide on this.

dr-m reviewed Sep 21, 2026

View reviewed changes

DerZc force-pushed the fix-mdev-40977 branch from 213d969 to a02eeb3 Compare September 21, 2026 05:35

DerZc force-pushed the fix-mdev-40977 branch from a02eeb3 to 0bc67fe Compare September 21, 2026 06:16

midenok reviewed Sep 21, 2026

View reviewed changes

Comment thread mysql-test/main/bit_shift.test Outdated

MDEV-40977 Move regression into main.func_bit

30a7076

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Sep 23, 2026

gkodinov self-assigned this Sep 23, 2026

gkodinov requested changes Sep 23, 2026

View reviewed changes

		CREATE TABLE t1 (shift_count DOUBLE UNSIGNED NOT NULL);

		INSERT INTO t1 VALUES (9223372036854775807);

Uh oh!

Conversation

DerZc commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Changes

Regression coverage

Validation

Uh oh!

CLAassistant commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

DerZc commented Sep 21, 2026 •

edited

Loading

CLAassistant commented Sep 21, 2026 •

edited

Loading