sqlc icon indicating copy to clipboard operation
sqlc copied to clipboard

Fix SQLite Engine Comparison Operators and ORDER BY Parsing

Open mgilbir opened this issue 2 months ago • 3 comments

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

  1. Comparison operators not parsed correctly - All comparison expressions were defaulting to = operator instead of extracting the actual operator (<, >, <=, >=, !=, <>, IS, LIKE, GLOB, MATCH, REGEXP)
  2. NOT operator variants not supported - NOT LIKE, NOT GLOB, NOT IN, IS NOT NULL, etc. were not handled properly
  3. ORDER BY generates wrong AST nodes - ORDER BY clauses were incorrectly creating CaseExpr nodes instead of proper SortBy nodes
  4. Missing ORDER BY integration - SELECT statements weren't properly setting the SortClause field
  5. NOT EXISTS flattened incorrectly - SQLite treated NOT EXISTS as a bare subquery, producing exists variables instead of the expected not_exists boolean 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 proper SortBy AST nodes with correct direction (ASC/DESC) and null handling (NULLS FIRST/LAST)
  • Enhanced convertMultiSelect_stmtContext: Added ORDER BY integration to properly set SortClause
  • Added convertNullComparison: Handle IS NULL/IS NOT NULL expressions
  • Extended convertUnaryExpr: Detects unary NOT via the unary-operator node, including wrapping NOT EXISTS as a BoolExpr
  • Hardened convertInSelectNode: Recognises NOT EXISTS when 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 EXISTS return types: Generated Go methods now return bool (and emit variables named exists/not_exists) instead of the previous int64 placeholder. 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, id in BarNotExists).

Related Issues

Fixes parsing issues with SQLite comparison operators and ORDER BY clauses that could lead to incorrect SQL generation and runtime errors.

mgilbir avatar Oct 06 '25 00:10 mgilbir

Hey @mgilbir, I am guessing that this PR will fix: https://github.com/sqlc-dev/sqlc/issues/4188, yes?

anthonyg876 avatar Nov 20 '25 19:11 anthonyg876

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.

mgilbir avatar Nov 20 '25 20:11 mgilbir

@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?

anthonyg876 avatar Dec 01 '25 17:12 anthonyg876