Skip to content

fix: use a derived query ID for insert schema lookups - #481

Merged
abonander merged 3 commits into
ClickHouse:mainfrom
hsdfat:metadata-query-without-query-id
Oct 6, 2026
Merged

abonander merged 3 commits into
ClickHouse:mainfrom
hsdfat:metadata-query-without-query-id

Conversation

@hsdfat

@hsdfat hsdfat commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #477

When insert validation fetches a table schema, its internal DESCRIBE TABLE inherits the client's query_id. Reusing that ID for the subsequent INSERT makes the query log ambiguous and can trigger QUERY_WITH_SAME_ID_IS_ALREADY_RUNNING.

The schema lookup now uses {query_id}-describe, preserving its link to the original INSERT while assigning the two requests distinct IDs. The INSERT retains the caller's ID. session_id and all other settings are preserved, and absent or empty IDs retain server-generated ID behavior. This also covers Inserter.

Unit tests verify derived IDs, preserved client/session settings, and absent/empty IDs. Integration tests inspect system.query_log and assert that the linked ID belongs to Describe and the original ID belongs to Insert, for both direct inserts and Inserter.

Validation after rebasing onto current main:

  • cargo test --lib --all-features: 109 passed.
  • cargo test --test it --all-features insert -- --test-threads=2: 40 passed against ClickHouse 26.9.3.38, including temporary-table/session tests and both regressions.
  • Both new query-log regressions also pass against ClickHouse 25.8.33.6.
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo clippy --all-targets --no-default-features -- -D warnings
  • cargo fmt --all -- --check

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided so that we can include it in CHANGELOG later

@CLAassistant

CLAassistant commented Sep 27, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@abonander

Copy link
Copy Markdown
Contributor

@hsdfat I cannot merge your PRs unless you sign the CLA as shown above.

Comment thread src/lib.rs Resolved
@hsdfat

hsdfat commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

The CLA assistant now reports all committers signed on both PRs: #481 and #478. Those status updates came after your comment. Could you recheck whether anything else is needed on my side?

hsdfat added 2 commits October 3, 2026 00:01
The DESCRIBE TABLE issued to fetch the schema before an insert inherited
all client settings, including query_id, so the same id was used for both
the DESCRIBE and the INSERT. Drop query_id from that query only; session_id
and other settings are kept.

Fixes ClickHouse#477
Signed-off-by: phatlc <phatle.hsd@gmail.com>
@hsdfat
hsdfat force-pushed the metadata-query-without-query-id branch from c0eb2e7 to 830b586 Compare October 2, 2026 17:12
@hsdfat hsdfat changed the title fix: don't send the client's query_id with the insert DESCRIBE TABLE fix: use a derived query ID for insert schema lookups Oct 2, 2026
@abonander
abonander merged commit 4e6ef63 into ClickHouse:main Oct 6, 2026
7 checks passed
@hsdfat
hsdfat deleted the metadata-query-without-query-id branch November 6, 2026 11:28
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.

query_id setting leaks into the internal DESCRIBE TABLE issued before INSERT

3 participants