Skip to content

Fix parsing of quoted identifiers#499

Open
IyeOnline wants to merge 2 commits into
ClickHouse:masterfrom
IyeOnline:topic/fix-tuple-parsing
Open

Fix parsing of quoted identifiers#499
IyeOnline wants to merge 2 commits into
ClickHouse:masterfrom
IyeOnline:topic/fix-tuple-parsing

Conversation

@IyeOnline
Copy link
Copy Markdown
Contributor

@IyeOnline IyeOnline commented May 20, 2026

Previously a type such as

Tuple( `a.b` Int8)

would fail to parse. This would cause Client::Impl::ReadBlock to throw
an "unsupported column type" exception, even though this is a valid
ClickHouse type and supported by the library.

This fix introduces a new token type for quoted identifiers and uses this
tuples. A dedicated token type seems a semantically cleaner choice, because
formally there could be cases where Name would be accepted, but a
quoted identifier would not be allowed.

This is effectively a followup to #491

@IyeOnline IyeOnline requested review from mzitnik and slabko as code owners May 20, 2026 13:23
@IyeOnline IyeOnline force-pushed the topic/fix-tuple-parsing branch 2 times, most recently from d828028 to 4197ed0 Compare May 20, 2026 13:43
Copy link
Copy Markdown
Contributor

@slabko slabko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @IyeOnline, can you please move the TupleWithQuotedFieldNamesFromServer test to a more appropriate place.
Additionally column and tuple names can have ` in their names. For example:

Tuple(`a\`b` UInt8, `a``b` UInt8)

Comment thread ut/roundtrip_tests.cpp Outdated
@slabko
Copy link
Copy Markdown
Contributor

slabko commented May 20, 2026

@IyeOnline did you check the case with inserting data to tables with tuples like Tuple(`a.b` Int8)?

// create table test_tuple (
//      id UInt64,
//      value Tuple(`a.b` Int8)
//  )
//  engine = MergeTree
//  order by id

auto block = client.BeginInsert("INSERT INTO test_tuple VALUES");
auto col_id = block.At(0)->AsStrict<ColumnUInt64>();
auto col_tuple = block.At(1)->AsStrict<ColumnTuple>();
auto col_value = col_tuple->At(0)->AsStrict<ColumnInt8>();

col_id->Append(1);
col_id->Append(2);

col_value->Append(5);
col_value->Append(7);
block.RefreshRowCount();

client.SendInsertBlock(block);
client.EndInsert();

Gives

 DB::Exception: Syntax error (data type): failed at position 8 (.): .b Int8). Expected one of: data type, identifier

From what I can see the library does not wrap the names when sending them to the server:

image

Previously a type such as

    Tuple( `a.b` Int8)

would fail to parse. This would cause `Client::Impl::ReadBlock` to throw
an "unsupported column type" exception, even though this is a valid
ClickHouse type and supported by the library.

This fix introduces a new token type for quoted identifiers and uses this
tuples. A dedicated token type seems a semantically cleaner choice, because
formally there could be cases where `Name` would be accepted, but a
quoted identifier would not be allowed.
@IyeOnline IyeOnline force-pushed the topic/fix-tuple-parsing branch from 4197ed0 to 1da3346 Compare May 20, 2026 20:17
@IyeOnline
Copy link
Copy Markdown
Contributor Author

@slabko I did discover this while trying to send data to a table with a Tuple like that (with the table previously generated from a JSON schema that used such a key) With this fix, it works there.

I will look into why it doesnt work in the simple standalone case tomorrow.

@IyeOnline
Copy link
Copy Markdown
Contributor Author

It turns out my our code actually didn't sent blocks with column names on the type as that code predates support for that. We just created the table with the named fields.

@IyeOnline IyeOnline requested a review from slabko May 21, 2026 11:32
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