Skip to content

Unknown type names inside a container decode silently as wrong data #589

Open

Description

@Hakaou

Follow-up to the second point of the suggested fix in #571, which #575 leaves aside: validating the elements of a container, so that an unknown type name fails loudly instead of desynchronising the stream.

A type name the parser does not know becomes a Void terminal. At the root, ValidateAST rejects it and the client throws "unsupported column type". Inside a container nothing checks it: the column is created as ColumnNothing, which reads one byte per row, and the rest of the block is misread without any error.

In the client itself, on master (fc5d98f):

CreateColumnByType("Int256");         // nullptr, the client reports "unsupported column type: Int256"
CreateColumnByType("Array(Int256)");  // a column typed Array(Void), no error

Against ClickHouse 26.11:

SELECT CAST(1, 'Int256') AS wide     -- UnimplementedError: unsupported column type: Int256
SELECT [CAST(1, 'Int256')] AS wide   -- one row, no exception, wrong content

Other names a server sends as is and the parser does not know, checked with toTypeName on 26.11: Variant, Dynamic, BFloat16, UInt256, the Interval* types, LineString, MultiPoint, QBit, AggregateFunction. At the root each of them is an error. Inside a container that accepts it, as Array(Int256) shows, the data is misread without any error. Nothing goes through the same path and only works by accident, one byte per row on both sides, which SELECT [] and SELECT NULL rely on.

#578 handles the neighbouring case, a known name the factory cannot build inside a wrapper, which crashed. This one is about names the parser does not know at all, and the two change different functions.

#590 is a draft for it, stacked on #575 because it needs the Nested node from there. It validates the elements of those six containers, leaves the arguments of every other type alone (JSON settings and typed paths, the function name of SimpleAggregateFunction), and registers Nothing. An unknown name inside a container then makes CreateColumnByType return nullptr, so the client throws the same "unsupported column type" as at the root.

Would you rather have a diagnostic that names the inner type? The tests hold either way.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions