googlesql: type inference via the new analysis core (analyze only) - #4533
Merged
Conversation
Redo GoogleSQL analyze support on top of the core catalog and analyzer landed in #4521, rather than the legacy compiler catalog the original attempt used. - A googlesql dialect seed registers the built-in GoogleSQL types, the comparison/arithmetic operators over them, and COUNT as an int64 aggregate, so the core catalog can resolve schema types and predicates. - The compiler builds a core.Catalog for the googlesql engine; schema DDL is applied through internal/core/schema and queries are resolved by internal/core/analyzer, never entering the legacy analyze path. - `sqlc analyze --dialect googlesql` accepts the new engine. Core analyzer: - Support INSERT, UPDATE, and DELETE in addition to SELECT: bind the target relation, infer parameter types from the columns they are assigned to or compared against, and project RETURNING lists. - Resolve COUNT(*) through a catalog-registered aggregate when the dialect provides one, falling back to int8 as before. - The core compile path now rewrites named parameters before analysis, so "@name" resolves to a numbered parameter carrying its name. GoogleSQL engine: - Include a statement's leading comment in its reported location so the "-- name:" annotation is captured. This also fixes name extraction for `sqlc parse --dialect googlesql`. - Tag operator A_Expr nodes with A_Expr_Kind_OP, matching the other engines, so the analyzer recognizes them as operator expressions. - Keep named parameters in their native "@name" form when rewriting, since GoogleSQL (and Spanner in particular) requires them. End-to-end coverage lands as analyze_basic, analyze_select (star expansion, joins, aggregates), and analyze_dml cases; the parse_basic/googlesql golden picks up the corrected statement spans and the now-detected query name and command.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Redo of #4522 on top of the core catalog and analyzer landed in #4521. The original wired GoogleSQL into the legacy compiler (
googlesql.NewCatalog()+analyzeQuery/outputColumns); this wires it the way ClickHouse is wired —sqlc analyze --dialect googlesqlresolves result columns and parameters entirely throughinternal/core, with the legacy analyze path never entered.Engine wiring
internal/engine/googlesql/seed.go— a dialect seed registering the built-in GoogleSQL types, comparison and arithmetic operators over them, andCOUNTas anint64aggregate.internal/compiler/engine.go— thegooglesqlengine builds acore.Catalog; schema DDL is applied viainternal/core/schemaand queries are resolved byinternal/core/analyzer.sqlc analyzeacceptsgooglesqlas a dialect.Core analyzer
Engine-neutral work the GoogleSQL fixtures required, which ClickHouse gets too:
internal/core/analyzer/dml.go— INSERT, UPDATE, and DELETE support alongside SELECT: bind the target relation, infer parameter types from the column each value is assigned to or compared against, and projectRETURNING/THEN RETURNlists.COUNT(*)resolves through a catalog-registered aggregate when the dialect provides one, falling back toint8as before. This closes thedata_type: "any"limitation Add analyze support for GoogleSQL via zetajones #4522 flagged as a follow-up.@nameresolves to a numbered parameter that carries its name.GoogleSQL engine
-- name:annotation is captured. PreviouslyStmtLocationpointed past the comment, soanalyzefound no named queries. This also fixes name extraction forsqlc parse --dialect googlesql.A_Exprnodes withA_Expr_Kind_OP, matching the other engines, so the analyzer recognizes them as operator expressions.@nameform when rewriting, since GoogleSQL supports them natively and Spanner requires them.Differences from #4522
Dropped, because they only mattered on the legacy path:
ReturningList,TargetList,FromClause). The panics they fixed were in legacy compiler code the core path does not run.quoteIdentchange, used only by legacy star expansion.Tests
End-to-end fixtures under
internal/endtoend/testdata/, run byTestReplay:analyze_basic/googlesqlandanalyze_dml/googlesql— reproduce Add analyze support for GoogleSQL via zetajones #4522's fixtures byte for byte (the DML case also coversUPDATE ... THEN RETURNandDELETE ... THEN RETURN).analyze_select/googlesql— new: star expansion, joins with aliases, andCOUNT(*).parse_basic/googlesqlgolden, which picks up the corrected statement spans and the now-detected query name and command.go test --tags=examples -timeout 25m ./...is green with PostgreSQL and MySQL running;gofmtandgo vetare clean on the touched files.Known limitations
SELECT *expands in the analyzer's column output but the SQL text is not rewritten (same as ClickHouse — fine foranalyze, needs work beforegenerate).INSERT ... SELECTreturns an explicit "not supported" error rather than silently producing no columns or parameters.🤖 Generated with Claude Code
Generated by Claude Code