Skip to content

Conversation

@kyleconroy
Copy link
Collaborator

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 with sqlc.* marcos.

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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.

3 participants