-
Notifications
You must be signed in to change notification settings - Fork 962
feat(postgres): Database-only mode #4154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
kyleconroy
wants to merge
4
commits into
main
Choose a base branch
from
claude/skip-parser-config-011CUQUPkEKJ8uDAN1qtSo8E
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat(postgres): Database-only mode #4154
kyleconroy
wants to merge
4
commits into
main
from
claude/skip-parser-config-011CUQUPkEKJ8uDAN1qtSo8E
Conversation
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
This commit adds a new configuration option `analyzer.skip_parser` that
allows sqlc to skip the parser and catalog entirely, relying solely on
the database analyzer for query analysis. This is particularly useful
when:
- Working with complex PostgreSQL syntax not fully supported by the parser
- Wanting to ensure queries are validated against the actual database schema
- Dealing with database-specific features or extensions
Key changes:
- Add `skip_parser` field to `Analyzer` config struct
- Implement `parseQueriesWithAnalyzer` method using sqlfile.Split
- Skip parser and catalog initialization when `skip_parser` is enabled
- Add validation requiring database analyzer when using skip_parser
- Only PostgreSQL is supported for this feature initially
Usage example:
```yaml
version: "2"
sql:
- name: "example"
engine: "postgresql"
queries: "query.sql"
schema: []
database:
uri: "postgresql://user:pass@localhost:5432/db"
analyzer:
skip_parser: true
gen:
go:
package: "db"
out: "db"
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit adds multiple levels of testing for the skip_parser feature: ## Unit Tests ### Config Tests (internal/config/skip_parser_test.go) - Test parsing skip_parser: true from YAML - Test default behavior (nil) - Test explicit skip_parser: false ### Compiler Validation Tests (internal/compiler/skip_parser_test.go) - Test that skip_parser requires database configuration - Test that skip_parser requires database analyzer enabled - Test that skip_parser only works with PostgreSQL - Test valid skip_parser configuration - Test normal operation with skip_parser disabled/default ### Query Splitting Tests (internal/compiler/split_test.go) - Test splitting multiple queries from a single file - Test complex queries with CASE statements and operators - Test PostgreSQL dollar-quoted strings ($$) - Test queries with comments ## End-to-End Example (examples/skip_parser/) ### Files - schema.sql: PostgreSQL schema with arrays, JSONB, indexes - query.sql: CRUD operations testing various PostgreSQL features - sqlc.yaml: Configuration with skip_parser: true - db_test.go: Comprehensive integration tests - README.md: Documentation and usage instructions ### Features Tested - BIGSERIAL auto-increment - NUMERIC decimal types - TIMESTAMPTZ timestamps - TEXT[] arrays - JSONB binary JSON - ANY() array operators - GIN indexes - RETURNING clauses All tests pass successfully: - go test ./internal/config -run TestSkipParser ✓ - go test ./internal/compiler -run TestSkipParser ✓ - go test ./internal/compiler -run TestSqlfileSplit ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit adds proper endtoend tests for the skip_parser feature, replacing the previous examples and unit tests with tests that follow the standard sqlc testing infrastructure. ## Removed - examples/skip_parser/ - Example directory - internal/compiler/skip_parser_test.go - Unit tests - internal/compiler/split_test.go - Unit tests - internal/config/skip_parser_test.go - Unit tests ## Added Endtoend Tests ### 1. skip_parser/postgresql/pgx/v5 Success test case demonstrating skip_parser functionality: - Uses managed database (contexts: ["managed-db"]) - Tests CRUD operations with PostgreSQL-specific types - Schema: products table with BIGSERIAL, TEXT, NUMERIC, TEXT[] - Queries: GetProduct, ListProducts, CreateProduct - Expected output: Generated Go code with pgx/v5 ### 2. skip_parser_error_no_database/postgresql Error test case verifying validation: - Attempts to use skip_parser without database configuration - Expected stderr: "skip_parser requires database configuration" - Verifies error handling ## Test Structure Tests follow the standard endtoend test pattern: - Located in internal/endtoend/testdata/ - Each test has sqlc.json, schema.sql, query.sql - Expected output files in go/ subdirectory - exec.json specifies test context requirements - stderr.txt contains expected error messages ## Running Tests ```bash # Error test (no database needed) go test -tags=examples -run 'TestReplay/base/skip_parser_error' ./internal/endtoend # Success test (requires database) go test -tags=examples -run 'TestReplay/managed-db/skip_parser' ./internal/endtoend ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The skip_parser feature was failing with a nil pointer dereference when used with managed databases because parseCatalog() was trying to use the parser and catalog, which are nil when skip_parser is enabled. ## Root Cause When skip_parser is enabled: 1. Parser and catalog are set to nil in NewCompiler 2. parseCatalog() was unconditionally calling c.parser.Parse() (line 46) 3. This caused a nil pointer dereference However, parseCatalog() still needs to be called even in skip_parser mode because: - The schema SQL text needs to be stored in c.schema - The database analyzer needs c.schema to pass to PostgreSQL ## Fix Modified parseCatalog() to check if skip_parser is enabled: - If skip_parser: Read schema files and store in c.schema, skip parsing - If normal mode: Parse schemas and update catalog as before Also reverted the change in generate.go that was skipping ParseCatalog entirely, since we always need to call it (it now handles skip_parser internally). ## Testing This fixes the panic in the managed-db context test: - TestReplay/managed-db/skip_parser/postgresql/pgx/v5 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Add a mode that skips parsing SQL entirely and relies on PostgreSQL for all analysis. This is very work-in-progress, as it won't support expanding
*commands nor will it work correctly withsqlc.*marcos.