buildkite / buildkite/clickhouse-sql
Positional nil validation for composite placeholder types
- 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