Skip to content

Make InsertData independent of block boundaries and order - #576

Open
Hakaou wants to merge 1 commit into
ClickHouse:masterfrom
Hakaou:insert-data-expected-rows
Open

Hakaou wants to merge 1 commit into
ClickHouse:masterfrom
Hakaou:insert-data-expected-rows

Conversation

@Hakaou

@Hakaou Hakaou commented Oct 17, 2026 •

Copy link
Copy Markdown

Noticed while running the client suite under UBSan for #575.

InsertData inserts two blocks of four rows, then checks the selected rows with a branch on which block is being read, indexing TEST_DATA with the running row number. When the server returns the eight rows in a single block, the first branch runs for all eight rows and reads TEST_DATA[4..7], past the array:

client_ut.cpp:625:21: runtime error: index 4 out of bounds for type 'const struct (unnamed struct)[4]'
client_ut.cpp:626:21: runtime error: index 4 out of bounds ...
client_ut.cpp:627:21: runtime error: index 4 out of bounds ...

The reads happened to return the expected values, which nothing guarantees.

The select also has no ORDER BY. With insert squashing disabled (min_insert_block_size_rows = 0, min_insert_block_size_bytes = 0) and max_block_size = 1, a 25.12.13.2 server keeps the two inserted blocks separate and sometimes returns 2, 4, 6, 8 first: 3 of 600 runs of the current test failed that way locally. 26.11 stores them as one block, so it did not reproduce there.

To reproduce on 25.12: add the three settings to the default profile of ci/docker-compose/users.xml, start the server with it, then run ./clickhouse-cpp-ut --gtest_filter='*InsertData*' --gtest_repeat=300 on master.

Change: one Row struct, both datasets as vectors, the select orders by id, the expectation is both datasets sorted by id, and every received row is compared against it with ASSERT_LT(row, expected_rows.size()) instead of the branch. Block boundaries and block order no longer matter.

Checked locally (macOS arm64) against 25.12, 26.3 and 26.11: the test passes under AddressSanitizer and UBSan with halt_on_error=1, 600 runs in a row with the settings above on 25.12, and the whole ClientCase suite stays green.

@CLAassistant

CLAassistant commented Oct 17, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Hakaou
Hakaou force-pushed the insert-data-expected-rows branch 2 times, most recently from 4265ed3 to b460f49 Compare October 18, 2026 13:17
The expectation branched on which block was being read and indexed
TEST_DATA with the running row number. When the server returns the
eight rows in one block, that reads past TEST_DATA (UBSan: index 4 out
of bounds, client_ut.cpp:625-627). The reads happened to return the
expected values, which nothing guarantees.

The select had no ORDER BY either. When the server keeps the two
inserted blocks separate, it may return the second one first.

The select now orders by id, and every received row is compared
against both datasets sorted by id.
@Hakaou
Hakaou force-pushed the insert-data-expected-rows branch from b460f49 to b90972e Compare October 18, 2026 14:34
@Hakaou Hakaou changed the title Compare InsertData rows against both datasets regardless of block boundaries Make InsertData independent of block boundaries and order Oct 18, 2026
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