You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
docs(sdk): computed columns do carry default_kind, contrary to the .schema() note #576
docs/src/content/docs/sdk/queries.md:75-77 annotates the .schema() example with:
default_kind/default_expression appear only when the column declares a default. A MATERIALIZED or ALIAS column is computed by ClickHouse and cannot be inserted.
The first sentence is technically true, but the second immediately contrasts MATERIALIZED/ALIAS as a separate category — which invites the reading that computed columns don't carry default_kind. They do:
internal/discovery/discovery.go:280 sets HasDefault: defaultKind != "", and default_kind comes verbatim from system.columns, where MATERIALIZED and ALIAS are themselves default kinds.
tests/integration/discovery_metadata_test.go:57,69 assert exactly this: byName["day"].HasDefault is true with the comment "MATERIALIZED is a non-empty default_kind", and DefaultKind == "MATERIALIZED".
So an SDK consumer following this comment could skip computed-column metadata that is in fact always present.
Suggested fix
CodeRabbit's wording, which reads correctly against the contract:
-// `default_kind`/`default_expression` appear only when the column declares a default.+// `default_kind`/`default_expression` describe ClickHouse default-expression+// kinds, including computed kinds such as `MATERIALIZED` and `ALIAS`.
Raised by CodeRabbit on #555 as a Minor "outside diff range" comment, after it had already approved. At that point #555 was mergeStateStatus=CLEAN with both required approvals in, and every push on this stack has dismissed standing approvals — so landing a one-line docs clarification would have cost a human re-approval round-trip for no functional change. Fold it into the next change that touches this file, or cherry-pick it if #555 needs another push anyway.
Activity