Fix client-v2: request the format of internal queries thru settings, not a FORMAT clause - #3069
Open
polyglotAI-bot wants to merge 1 commit into
Open
Fix client-v2: request the format of internal queries thru settings, not a FORMAT clause#3069polyglotAI-bot wants to merge 1 commit into
polyglotAI-bot wants to merge 1 commit into
Conversation
The client sends the requested format of an operation in the X-ClickHouse-Format header of every request. getTableSchema/getTableSchemaFromQuery asked instead for TSKV with a FORMAT clause in the DESCRIBE query, and ping used a FORMAT clause too. A server before 26.8 used the format of the query, but since 26.8 the header wins, so the server answered with RowBinaryWithNamesAndTypes and the TSKV parser failed with "Failed to parse column null defined by type 'null'". The internal queries of the client now carry no FORMAT clause and set their format in the settings of the operation, so the header and the query always agree. Fixes: #3068
|
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
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.
Description
Fixes #3068.
client-v2sends the requested format of an operation in theX-ClickHouse-Formatheader of every request (HttpAPIClientHelper.addHeaders, from the format of the operation settings, defaulted toRowBinaryWithNamesAndTypesinClient.query).getTableSchema/getTableSchemaFromQueryasked instead forTSKVwith aFORMATclause in theDESCRIBEquery, andping()did the same withTabSeparated. A server before26.8used the format written in the query, so the mismatch was invisible. Since26.8the header wins over the clause, so the server answers aDESCRIBE ... FORMAT TSKVwithRowBinaryWithNamesAndTypes, theTSKVparser reads binary data, and every schema call fails withFailed to parse column null defined by type 'null'.jdbc-v2fails with it too, becauseConnectionImplandPreparedStatementImplread metadata thru those calls.Following the direction on the issue, the internal queries of the client now carry no
FORMATclause and request their format thru the settings of the operation, so the header and the query always agree on every server version.Changes
Client.getTableSchema(table, database)andClient.getTableSchemaFromQuery(sql, params): noFORMAT TSKVclause in theDESCRIBE/DESC (...)statement.Client.getTableSchemaImpl: theQuerySettingsof the call setsTSKV.Client.ping(timeout):SELECT 1withTabSeparatedin the settings instead of aFORMATclause.query(...)overloads: the old text said theFORMATclause of the SQL query always wins, which is not true since server26.8.SpanRecorderTest: the three assertions on the statement text of an internal query follow the new text (SELECT 1,DESCRIBE TABLE <t>,DESC (...)).CHANGELOG.md.Test
New
client-v2/src/test/java/com/clickhouse/client/api/RequestFormatUnitTest.java: aWireMockserver records the requests of the client, and one parametrized test asserts, per operation, the exact statement sent and theX-ClickHouse-Formatheader —getTableSchema->DESCRIBE TABLE test_table+TSKV,getTableSchemaFromQuery->DESC (SELECT id FROM test_table)+TSKV,ping->SELECT 1+TabSeparated. Those three rows fail onmain(the statement still holds the clause). Two more rows pin the unchanged behavior of a caller query: a plain query still asks forRowBinaryWithNamesAndTypesand a query with an explicitJSONEachRowsetting still asks forJSONEachRow; both pass onmainand after the change.Verified against a
26.8.1.1761server (clickhouse/clickhouse-server:head) in a container:MetadataTests.testGetTableSchema,testGetTableSchemaDifferentDbandtestCreateTableWithAllDataTypesfail withFailed to parse column null defined by type 'null'.testCreateTableWithAllDataTypesstill fails, but for an unrelated reason —Unknown data type: MultiPoint, which [client-v2, jdbc-v2] Add MultiPoint type support #3048 adds.client-v2unit tests: 561 pass.SpanRecorderTest: 6 pass.QueryTests: 96 run, see the note below.Compatibility: no public API change and no behavior change for a caller query. A header-only format request works on old servers as well — checked with
24.3.18.7and26.6.2.160, where a request withX-ClickHouse-Format: TSKVand noFORMATclause returnsTSKV.Note — a related problem this PR does not fix
The same server change also affects a caller query that holds a
FORMATclause: the client sends its defaultRowBinaryWithNamesAndTypesheader, which now wins, so the query silently returns other bytes.QueryTests.testSettingsNotChanged(select 1 FORMAT JSONEachRowwith empty settings) fails on26.8for that reason, onmainand with this PR alike; it passes on26.6.jdbc-v2StatementImpl.executeQueryImplsends the SQL of the user verbatim with no format in the settings, soSELECT ... FORMAT JSONEachRowthru JDBC has the same problem, anddocs/client-v2-json-support.mdanddocs/features.mdstill recommend that pattern. Deciding whether the client keeps honouring aFORMATclause (detect it and align the header) or the clause becomes unsupported (then the docs, the examples and that test need to follow) is a separate decision, so it is left out of this fix. Please tell us which way you want it and we will raise it separately.Pre-PR validation gate
26.8.1.1761)SpanRecorderTestassertions on the statement text follow the intentionally changed text)AGENTS.md(CHANGELOG.mdupdated, one logical change,@DataProviderinstead of near-identical test methods, no issue numbers in test code)docs/features.mdneeds no update