Skip to content

chore(clickhouse): bump hclexp to the v1.0.2 SQL parser - #96691

Merged
trunk-io[bot] merged 1 commit into
masterfrom
pawel/chore/hclexp-parser-v1.0.2
Sep 8, 2026
Merged

trunk-io[bot] merged 1 commit into
masterfrom
pawel/chore/hclexp-parser-v1.0.2

Conversation

@orian

@orian orian commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

The pinned hclexp build decides how every golden in this directory is canonicalized, so a change to its SQL parser can rewrite them without anyone changing a layer.

  • bin/image.txt pins the build that check.sh runs in CI and that gen-golden.sh writes goldens with.
  • chschema chore: bump clickhouse-sql-parser to v1.0.2 chschema#249 moves clickhouse-sql-parser from v1.0.1 to v1.0.2. It is a dependency bump with no intended behavior change.
  • A parser that formats one expression differently would move the goldens, and the gate would fail on every unrelated PR until someone regenerated them.

Changes

  • The pin moves to sha-9d9192f. Nothing else changes: no layer, golden, or sql file moves.
  • The parser rewrites nothing. Regenerating every golden and sql file under the new pin leaves all 70 byte-identical to the committed ones.

How did you test this code?

  • check.sh passes under the new pin: five envs validate, 34 compositions match their committed goldens, duplicates match the baseline of 2, sql is fresh.
  • gen-golden.sh and gen-sql.sh were rerun under the new pin and produced no diff at all, which is the check that would catch a canonicalization change.
  • Reconcile against the node dumps in PostHog/clickhouse-schema is unchanged at 32 / 29 / 12 operations for dev / prod-us / prod-eu, with 16 unsafe rows.
  • hogli ci:preflight --strict reports 0 failures.
  • Not run: check-live.sh, which needs a live ClickHouse.
  • No new tests. The change is a one-line pin, gated by check.sh in CI.

Automatic notifications

  • Publish to changelog?

Docs update

None. The bump changes no documented behavior.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Claude Code.

Stacked on #96339, which carries the schema work. This layer is only the pin, so it can land or be reverted on its own.

Public artifact: the diff is a single image tag. No hostnames, credentials, or customer identifiers.

Base automatically changed from pawel/refactor/hcl-apm-ingest-shared to master September 8, 2026 13:48
@trunk-io

trunk-io Bot commented Sep 8, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

Playwright — all passed

All tests passed.

View test results →

@orian
orian marked this pull request as ready for review September 8, 2026 13:48
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 8, 2026 13:50
Picks up clickhouse-sql-parser v1.0.2 (chschema#249).

The pin decides how goldens are canonicalized, so a parser change can
rewrite them. This one does not: regenerating every golden and sql file
under the new pin leaves all 70 byte-identical, and the reconcile holds
at 32/29/12 operations with 16 unsafe rows.
@orian
orian force-pushed the pawel/chore/hclexp-parser-v1.0.2 branch from a874439 to 349746c Compare September 8, 2026 13:52
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff (1)

  1. posthog/clickhouse/hcl/README.md, line 134 (link)

    P2 Inconsistent column-order policy

    The new guidance requires every hclexp diff and hclexp plan to use -ignore-column-order, but codegen/gen_migration.py still runs hclexp plan without it. An order-only difference that the validation scripts suppress can therefore still produce a needless migration plan. Please update the migration-generation path or narrow this requirement.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: posthog/clickhouse/hcl/README.md
    Line: 134
    
    Comment:
    **Inconsistent column-order policy**
    
    The new guidance requires every `hclexp diff` and `hclexp plan` to use `-ignore-column-order`, but `codegen/gen_migration.py` still runs `hclexp plan` without it. An order-only difference that the validation scripts suppress can therefore still produce a needless migration plan. Please update the migration-generation path or narrow this requirement.
    
    ---
    
    For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
### Issue 1
posthog/clickhouse/hcl/README.md:134
**Inconsistent column-order policy**

The new guidance requires every `hclexp diff` and `hclexp plan` to use `-ignore-column-order`, but `codegen/gen_migration.py` still runs `hclexp plan` without it. An order-only difference that the validation scripts suppress can therefore still produce a needless migration plan. Please update the migration-generation path or narrow this requirement.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: a874439 | Re-trigger Greptile

@trunk-io

trunk-io Bot commented Sep 8, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@trunk-io
trunk-io Bot merged commit d0cad03 into master Sep 8, 2026
265 checks passed
@trunk-io
trunk-io Bot deleted the pawel/chore/hclexp-parser-v1.0.2 branch September 8, 2026 14:55
@deployment-status-posthog

deployment-status-posthog Bot commented Sep 8, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-09-08 15:25 UTC Run
prod-us ✅ Deployed 2026-09-08 15:42 UTC Run
prod-eu ✅ Deployed 2026-09-08 15:46 UTC Run

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