buildkite / buildkite/clickhouse-sql

Positional nil validation for composite placeholder types

Open
#2 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Follow-up from [PR #1 review feedback](https://github.com/buildkite/clickhouse-sql/pull/1#discussion_r3592753532).

`ClickHouse::SQL::Query#validate_nil_placeholders!` guards nils in container values with a containment heuristic:

```ruby
next if type.include?("Nullable(")
```

Once any member of a composite type is `Nullable(...)`, nils are permitted anywhere in the bound value. For example, a nil *key* bound to `Map(String, Nullable(String))` passes validation and is serialized as `NULL`, only failing (or worse, being silently coerced, e.g. `String` parsing `\N` as empty string) at ClickHouse.

A positionally correct check requires parsing composite ClickHouse types (`Map(K, V)`, `Tuple(...)`, nested `Array(...)`) and validating each nil against the specific member type it binds to. `PlaceholderType` would be the natural home for that parsing.

Deemed out of scope for the initial import; the heuristic's limits are documented in a code comment as of fda546ced48.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with ClickHouse::SQL::Query#validate_nil_placeholders! and inspect PlaceholderType, the proposed home for composite-type parsing. Trace Map(K, V), Tuple(...), and nested Array(...) handling, then verify that each nil is checked against its positional member type rather than any Nullable(...) member.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, ruby
Domain
database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.