Replace git submodules with pinned CMake FetchContent - #57
Merged
Conversation
ClickHouse consumes this repository as a submodule and builds it with its own contrib CMake. The nested zlib, googletest, and clickhouse-cpp submodules (used only for this repository's own build and tests) made main unusable as a direct submodule pin: ClickHouse must not carry recursive submodules, which forced a separately maintained remove-recursive-submodules integration branch that had to be advanced by hand after every change. Fetch the three dependencies with FetchContent instead, pinned to the exact commits the submodules tracked, using GitHub archive tarballs so configure needs no git. The Populate-then-add_subdirectory form keeps EXCLUDE_FROM_ALL semantics (and zlib's install rules out of our install tree) while staying compatible with the project's CMake 3.16 minimum. With no .gitmodules left, a plain clone builds and ClickHouse can pin main directly; the integration branch becomes unnecessary. Former submodule paths are gitignored so existing checkouts stay clean.
iskakaushik
added a commit
to iskakaushik/ClickHouse
that referenced
this pull request
Aug 19, 2026
The SDK is now directly consumable from its main branch: upstream replaced its nested submodules with pinned CMake FetchContent (ClickHouse/ai-sdk-cpp#57), retiring the separately maintained flattened integration branch. The new pin also carries the response_messages terminal-reply fix requested in review (ClickHouse/ai-sdk-cpp#55) and GPT-5-family request normalization: the family rejects temperature/top_p and defaults to reasoning, which returned a hard 400 for any configured ai.temperature and burned small ai.max_tokens budgets on hidden reasoning with no visible SQL (ClickHouse/ai-sdk-cpp#58). Add the SDK's new http_sse_stream.cpp to the explicit source list; without it every target linking the SDK fails. Throw NETWORK_ERROR instead of LOGICAL_ERROR when a generation fails: a provider/API error is external, and LOGICAL_ERROR aborts debug builds on any API failure (observed as SIGABRT in unit_tests_dbms during live runs). Teach the generation prompt to treat SQL fragments embedded in the request as untrusted. Without instruction, current models faithfully reproduce statement stacking and UNION SELECT ... FROM passwords when the request embeds them (deterministic on claude-haiku-4-5 at temperature 0), so SQLInjectionProtection asserted behavior nothing had asked of the model. Update live-test model IDs that referenced retired models (gpt-4o-mini, gpt-4, claude-3-haiku, claude-3-opus all 404 today) and scope the injection test's comment assertion to quote-terminated statement stacking: models may legitimately echo the user's own comment text, and the result is prepopulated for review, not executed. Validated locally with unit_tests_dbms: offline AI suites pass, and the live AITestFixture suite passes 8/8 against both OpenAI (gpt-5-mini, gpt-5.6) and Anthropic (claude-haiku-4-5, claude-sonnet-5), including the multi-step SchemaExploration case the underlying fix exists for.
iskakaushik
added a commit
to iskakaushik/ClickHouse
that referenced
this pull request
Aug 19, 2026
The SDK is now directly consumable from its main branch: upstream replaced its nested submodules with pinned CMake FetchContent (ClickHouse/ai-sdk-cpp#57), retiring the separately maintained flattened integration branch. The new pin also carries the response_messages terminal-reply fix requested in review (ClickHouse/ai-sdk-cpp#55) and GPT-5-family request normalization: the family rejects temperature/top_p and defaults to reasoning, which returned a hard 400 for any configured ai.temperature and burned small ai.max_tokens budgets on hidden reasoning with no visible SQL (ClickHouse/ai-sdk-cpp#58). Add the SDK's new http_sse_stream.cpp to the explicit source list; without it every target linking the SDK fails. Throw NETWORK_ERROR instead of LOGICAL_ERROR when a generation fails: a provider/API error is external, and LOGICAL_ERROR aborts debug builds on any API failure (observed as SIGABRT in unit_tests_dbms during live runs). Teach the generation prompt to treat SQL fragments embedded in the request as untrusted. Without instruction, current models faithfully reproduce statement stacking and UNION SELECT ... FROM passwords when the request embeds them (deterministic on claude-haiku-4-5 at temperature 0), so SQLInjectionProtection asserted behavior nothing had asked of the model. Update live-test model IDs that referenced retired models (gpt-4o-mini, gpt-4, claude-3-haiku, claude-3-opus all 404 today) and scope the injection test's comment assertion to quote-terminated statement stacking: models may legitimately echo the user's own comment text, and the result is prepopulated for review, not executed. Validated locally with unit_tests_dbms: offline AI suites pass, and the live AITestFixture suite passes 8/8 against both OpenAI (gpt-5-mini, gpt-5.6) and Anthropic (claude-haiku-4-5, claude-sonnet-5), including the multi-step SchemaExploration case the underlying fix exists for.
1 task
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
1 participant
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.
ClickHouse consumes this repository as a submodule and builds it with
its own contrib CMake. The nested zlib, googletest, and clickhouse-cpp
submodules (used only for this repository's own build and tests) made
main unusable as a direct submodule pin: ClickHouse must not carry
recursive submodules, which forced a separately maintained
remove-recursive-submodules integration branch that had to be advanced
by hand after every change.
Fetch the three dependencies with FetchContent instead, pinned to the
exact commits the submodules tracked, using GitHub archive tarballs so
configure needs no git. The Populate-then-add_subdirectory form keeps
EXCLUDE_FROM_ALL semantics (and zlib's install rules out of our install
tree) while staying compatible with the project's CMake 3.16 minimum.
With no .gitmodules left, a plain clone builds and ClickHouse can pin
main directly; the integration branch becomes unnecessary. Former
submodule paths are gitignored so existing checkouts stay clean.
Note:
.github/workflows/ci.ymlstill has a now-inertsubmodules: recursivecheckout option — my push token lacks workflow scope to remove it; safe to drop in a follow-up.