PS-11483: Wrong query results when an internal temporary table is converted to on-disk InnoDB - #6125
Draft
jaideepkarande wants to merge 2 commits into
Draft
PS-11483: Wrong query results when an internal temporary table is converted to on-disk InnoDB#6125jaideepkarande wants to merge 2 commits into
jaideepkarande wants to merge 2 commits into
Conversation
Problem: ======== When an in-memory temporary table is converted to an on-disk table, queries using CONST access on the materialized table can return wrong results. The bug reproducer returns 0 for the low tmp_table_size case where 1 is expected. Root Cause: =========== Reading rows from the in-memory temporary table advances table state. After conversion, the table keeps a started state and later reads from the on-disk temporary table can miss the expected row for CONST access. In this repro, `tmp_table_size=1024000` stays on the in-memory path, while `tmp_table_size=1024` triggers conversion to an on-disk temporary table. Solution: ========= Reset the temporary table to not-started after conversion by calling `wtable->set_not_started()` in `create_ondisk_from_heap()`. This patch also adds `main.bugfix_const_access_disk_temporary_table` coverage and keeps the testcase data generator trunk-compatible by using `LPAD(num, 32, '0')` in place of `md5(num)`. We thank Jingqi Tian for the contribution. Change-Id: I77ef06d37927ab276cc66af1fb23dd7941e22630
Problem: main.with_recursive_innodb_tmp_table, ported to PXC together with Oracle commit cde9f3e (Bug#37308710), fails at the Bug#37308710 subtest with two diff hunks: unexpected "Tmp_table_size is set below 1MiB" warnings on each SET, and Created_tmp_disk_tables reporting 0 where upstream expects 1. Cause: The Bug#37308710 subtest verifies that CONST access on a materialized derived table still returns the correct row after the tmp table is converted from MEMORY to InnoDB. It reaches that path by first running the query with tmp_table_size = 102400 (in-memory) and then with tmp_table_size = 1024 (expected to overflow to disk). Percona-only patch PS-8647 in sql/sys_vars.cc silently rewrites any tmp_table_size below 1 MiB up to exactly 1 MiB (and pushes a warning). The rewrite is unconditional and independent of internal_tmp_mem_storage_engine, so it also applies when the session is using the MEMORY engine, not just TempTable. As a result, on PXC the effective per-session tmp_table_size in the subtest is 1 MiB in both runs. The MEMORY table holding the 99-row derived result (~40-byte reclength) fits comfortably in 1 MiB, so create_ondisk_from_heap() is never entered, no disk temp table is created, and the MEMORY->InnoDB transition that the backport is supposed to exercise never happens. The backported set_not_started() call is therefore unreachable in this test, and the "Created_tmp_disk _tables 1" assertion fails. The warning hunks are the same clamp firing at SET time. Solution: Test-only change. The backported source fix in sql/sql_tmp_table.cc is correct and stays as-is; only the reproducer needs enough data to overflow the 1-MiB floor that PS-8647 imposes on this branch. Widen col1 to VARCHAR(300) CHARACTER SET latin1 and grow the data generator to 4000 rows, so the derived table materialized by the UNION is ~1.2 MiB - large enough to overflow 1 MiB but well below 16 MiB. Explicitly save/restore max_heap_table_size and set it to 16 MiB in this section; the preceding with_recursive_wl9248 block left it at 61000, which would otherwise cap MEMORY well below tmp_table_size and defeat the size differential. Use tmp_table_size values at or above the PS-8647 threshold: 16 MiB for the in-memory case (data fits, Created_tmp_disk_tables = 0) and exactly 1 MiB for the on-disk case (data does not fit, Created_tmp_disk_tables = 1). Both values are >= 1 MiB, so the PS-8647 check (strict "<") never fires and no unexpected warnings appear. The count(distinct d.col1) = 1 assertion - the actual bug being verified - remains untouched. Add a comment block at the top of the subtest documenting the divergence from upstream, so the next porter does not "fix" the values back to the small upstream ones. The result file is regenerated with mtr --record; the query and insert-loop bodies are wrapped in --disable_query_log/--enable_query _log where appropriate to keep the recorded output stable.
inikep
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backports upstream MySQL fix for Bug #116741 (Oracle commit
cde9f3eaaf9, ships in MySQL 8.4.11) to the Percona Server 8.4 series, plus the Percona-specific test adjustment required to make the upstream reproducer actually exercise the fixed path on this branch.8.4(branch base is 8.4.10-10)Without the fix, a query can silently return an incorrect result set — no error, no warning — when an internal temporary table is converted from MEMORY to an on-disk InnoDB table mid-execution. Rows that exist are dropped from the result. The only "workaround" is avoiding the conversion, which does not hold in production as data volume grows.
Commits
7ab214aBug#37308710 — Innodb tmp table causes incorrect query resultssql/sql_tmp_table.cc(+7),main.with_recursive_innodb_tmp_tabletest + resulta8ea488CUSTOM-258main.with_recursive_innodb_tmp_tabletest + result onlyWhole-PR diff is exactly three files:
The only source change is one statement in
create_ondisk_from_heap():Reading rows from the in-memory temporary table advances table state. After conversion the table kept a started state, so subsequent reads from the on-disk temporary table could miss the expected row for CONST access.
Why the second commit is needed (Percona divergence from upstream)
The upstream subtest reaches the conversion path by running the query at
tmp_table_size = 102400(in-memory) and then attmp_table_size = 1024(expected to spill to disk).Percona-only patch PS-8647 in
sql/sys_vars.ccsilently rewrites anytmp_table_sizebelow 1 MiB up to exactly 1 MiB and pushes a warning. The rewrite is unconditional and independent ofinternal_tmp_mem_storage_engine, so it also applies when the session uses the MEMORY engine.Consequence on this branch: the effective
tmp_table_sizeis 1 MiB in both runs. The 99-row derived result (~40-byte reclength) fits comfortably in 1 MiB,create_ondisk_from_heap()is never entered, and the MEMORY→InnoDB transition the backport is meant to exercise never happens. The upstream test therefore failed with two diff hunks: unexpectedTmp_table_size is set below 1MiBwarnings on eachSET, andCreated_tmp_disk_tablesreporting 0 where upstream expects 1.a8ea488is test-only — the backported source fix stays as-is. It gives the reproducer enough data to overflow the 1-MiB floor PS-8647 imposes:col1widened toVARCHAR(300) CHARACTER SET latin1, generator grown to 4000 rows → derived table ~1.2 MiBmax_heap_table_sizeexplicitly saved/restored and set to 16 MiB for this section (the precedingwith_recursive_wl9248block left it at 61000, which would cap MEMORY well belowtmp_table_sizeand defeat the size differential)tmp_table_sizevalues at or above the PS-8647 threshold: 16 MiB in-memory (Created_tmp_disk_tables = 0) and exactly 1 MiB on-disk (Created_tmp_disk_tables = 1) — both ≥ 1 MiB, so the strict<check never fires and no unexpected warnings appearThe
count(distinct d.col1) = 1assertion — the actual bug being verified — is untouched. A comment block documents the divergence so the next porter does not "fix" the values back to the upstream ones.Testing
Full MTR (
FULL_MTR=yes,--big-test,CI_FS_MTR=yes,WITH_PS_PROTOCOL=yes), x86_64:percona-server-8.4-pipeline-parallel-mtrpercona-server-8.4-pipeline-parallel-mtrpercona-server-8.4-ASAN-pipeline-parallel-mtrThe target test now passes in every configuration
main.with_recursive_innodb_tmp_table w3 [ pass ] 42288main.with_recursive_innodb_tmp_table w8 [ pass ] 3369main.with_recursive_innodb_tmp_table w3 [ pass ] 80485Baseline diff
Baselines are the most recent runs of the same jobs, #1430 (Debug) and #21 (ASAN) — both on
release-8.4.11-11, i.e. a different patch base than this branch (8.4.10-10). Deltas below are interpreted with that in mind.Debug — #1431 vs #1430
One failure,
main.all_persisted_variables, deterministic (failed, thenretry-fail). Root cause is a persisted-variable count mismatch, not a behaviour change:The 8.4.10-10 base has one more persistable system variable than the test's hard-coded expectation. This PR registers no system variable — its entire diff is three files, none of them
sql/sys_vars.{cc,h}orall_persisted_variables.{test,result}— so it cannot move this count. Pre-existing on the branch base; the 8.4.11-11 baseline passes because the count was reconciled there.The baseline's own failure (the
unit_testsctest roll-up) does not reproduce in #1431.RelWithDebInfo — #1432
rocksdb_rpl.rpl_rocksdb_row_img_idx_{noblob,full,min}.row, oneshutdown_report, and theunit_testsroll-up (also failing in baseline #1430). None touch the temporary-table path. No same-config/same-OS baseline exists for RelWithDebInfo on this job — the nearest, #1425, ran ondebian:bullseyewithKEYRING_VAULT_MTR=yes.ASAN — #22 vs #21
69 vs 28. All 28 baseline failures reproduce; the ~41 additional ones are dominated by an environment regression in the
ubuntu:resoluteimage, not by this change. Tests that shell out to external binaries (main.symlink,main.import_symlink,main.mysqld_safe,main.mysql_system_cmd_unix,main.log_errchk,main.temp_table_debug) now fail on LeakSanitizer reports raised against Ubuntu resolute's rust-coreutils, not againstmysqld. Sample, frommain.temp_table_debug:ubuntu:resoluteis a development release and the image is pulled fresh per build; #21 ran 5 days earlier. The remaining clusters (10 ×rocksdb, 2 ×rocksdb_stress, 9 ×group_replicationclone/recovery) are whole-worker environmental/timing failures under instrumentation.Both #21 and #22 carry the identical pre-existing ASAN
DEADLYSIGNALSEGV signature (pc 0x0,<unknown module>), so that one is not new either.Caveats / follow-ups
set_not_started()state reset allocates nothing, but formally classifying them needs an ASAN run on the unmodified 8.4.10-10 base. Recommend firing that before merge; happy to do it on request.main.all_persisted_variableson 8.4.10-10 (489 vs 490) deserves its own ticket — it will fail for anyone testing this base.SLACK_CHANNELinPercona-Lab/ps-buildjenkins/pipeline-parallel-mtr.groovyis interpolated as"#${SLACK_CHANNEL}"against a default that already carries#, producingchannel: ##ps-upstream-merges. Slack notifications from these jobs never land. Worth a separate ticket.🤖 Generated with Claude Code