Fix SQLite Engine Comparison Operators and ORDER BY Parsing
Summary
This PR fixes critical parsing issues in the SQLite engine where comparison operators were incorrectly defaulting to = and ORDER BY clauses were generating incorrect AST nodes. It also restores correct handling of EXISTS/NOT EXISTS so SQLite queries emit the right boolean expressions and generated code.
Issues Fixed
- Comparison operators not parsed correctly - All comparison expressions were defaulting to
=operator instead of extracting the actual operator (<,>,<=,>=,!=,<>,IS,LIKE,GLOB,MATCH,REGEXP) - NOT operator variants not supported -
NOT LIKE,NOT GLOB,NOT IN,IS NOT NULL, etc. were not handled properly - ORDER BY generates wrong AST nodes - ORDER BY clauses were incorrectly creating
CaseExprnodes instead of properSortBynodes - Missing ORDER BY integration - SELECT statements weren't properly setting the
SortClausefield NOT EXISTSflattened incorrectly - SQLite treatedNOT EXISTSas a bare subquery, producingexistsvariables instead of the expectednot_existsboolean output
Changes Made
Core Fixes (internal/engine/sqlite/convert.go)
- Enhanced
convertComparison: Now properly extracts actual comparison operators instead of defaulting to= - Added
extractComparisonOperator: Comprehensive function handling all SQLite comparison operators including NOT variants - Fixed
convertOrderby_stmtContext: Now generates properSortByAST nodes with correct direction (ASC/DESC) and null handling (NULLS FIRST/LAST) - Enhanced
convertMultiSelect_stmtContext: Added ORDER BY integration to properly setSortClause - Added
convertNullComparison: HandleIS NULL/IS NOT NULLexpressions - Extended
convertUnaryExpr: Detects unaryNOTvia the unary-operator node, including wrappingNOT EXISTSas aBoolExpr - Hardened
convertInSelectNode: RecognisesNOT EXISTSwhen the context begins with the NOT token so the AST preserves the negation
New Test Coverage (internal/engine/sqlite/convert_test.go)
- 35 comprehensive test cases covering all fixed functionality
- Comparison operator tests: All SQLite operators (
<,>,<=,>=,=,!=,<>,<<,>>,&,|,IS,LIKE,GLOB,MATCH,REGEXP) - NOT variant tests:
NOT LIKE,NOT GLOB,NOT IN,IS NOT NULL, etc. - ORDER BY tests: Direction handling, null ordering, multiple columns
- Complex query tests: Real-world scenarios with multiple operators and clauses
Operators Now Supported
- Comparison:
<,>,<=,>=,=,!=,<> - Bitwise:
<<,>>,&,| - Pattern matching:
LIKE,GLOB,MATCH,REGEXP - Null testing:
IS NULL,IS NOT NULL - List membership:
IN,NOT IN - All NOT variants:
NOT LIKE,NOT GLOB,NOT MATCH,NOT REGEXP
Testing
# All new tests pass
go test -v ./internal/engine/sqlite -run TestConvert
# Existing functionality preserved
go test ./internal/engine/sqlite
# Regression coverage for managed DB fixtures
go test ./internal/endtoend -run 'TestReplay/managed-db/select_exists/sqlite'
go test ./internal/endtoend -run 'TestReplay/managed-db/select_not_exists/sqlite'
Example Impact
Before (incorrect):
SELECT * FROM users WHERE age >= 18 ORDER BY name DESC
-- Generated AST: A_Expr with "=" operator, CaseExpr for ordering
After (correct):
SELECT * FROM users WHERE age >= 18 ORDER BY name DESC
-- Generated AST: A_Expr with ">=" operator, SortBy with DESC direction
Breaking Changes
- SQLite
EXISTS/NOT EXISTSreturn types: Generated Go methods now returnbool(and emit variables namedexists/not_exists) instead of the previousint64placeholder. Projects that already consume these helpers must update their call sites to expect a boolean result. - Parameter propagation for
NOT EXISTS: Because the AST now retains the inner select, generated methods once again surface the query arguments. Call sites that relied on the buggy zero-argument signature will need to pass the correct inputs (for example,idinBarNotExists).
Related Issues
Fixes parsing issues with SQLite comparison operators and ORDER BY clauses that could lead to incorrect SQL generation and runtime errors.
Hey @mgilbir, I am guessing that this PR will fix: https://github.com/sqlc-dev/sqlc/issues/4188, yes?
Hey @mgilbir, I am guessing that this PR will fix: #4188, yes?
Yes, I left a description on how to reproduce it with your schema in the linked issue.
@kyleconroy Hey Kyle, wanted to ask what the PR review process is like? I wanted to see if there is anything that can be done to get eyes on this ticket to possibly be merged into the next version drop?