Skip to content

perf(psql): use EXISTS for view checks - #735

Open
snowyukitty wants to merge 3 commits into
stephenafamo:mainfrom
snowyukitty:fix/psql-view-exists
Open

perf(psql): use EXISTS for view checks#735
snowyukitty wants to merge 3 commits into
stephenafamo:mainfrom
snowyukitty:fix/psql-view-exists

Conversation

@snowyukitty

Copy link
Copy Markdown

Summary

ViewQuery.Exists currently delegates to Count, so PostgreSQL computes a count(1) aggregate even though callers only need to know whether one matching row exists.

This change:

  • executes the original view query through PostgreSQL's EXISTS expression and scans the single boolean result;
  • preserves query hooks and the complete inner query, including CTEs, filters, grouping, limits, and offsets; and
  • adds a PostgreSQL-backed regression test whose query includes a CTE and verifies that EXISTS is emitted instead of count(1).

Fixes #731.

Validation

Passed locally on Windows with Go 1.25.0, PostgreSQL via testcontainers, and CGO enabled with GCC 16.1.0:

  • go test -race -run '^TestSomeViewExistsUsesExistsExpression$' ./dialect/psql
  • go test -race ./dialect/psql
  • go build ./...
  • go vet ./...
  • golangci-lint run --new-from-rev=HEAD (0 issues)
  • git diff --check

A full go test ./... run passed every package except the unrelated existing TestLibSQL Docker Desktop path, where the Windows host connection to the libsql container was reset during schema migration. The CGO-dependent SQLite mattn generator tests passed when run separately.

Coordination

Open PR #657 also touches dialect/psql/view.go as part of a much larger immutable-query refactor, but it does not implement #731 and is currently conflicted with main. If that refactor lands first, this focused change will need a small rebase onto its new ViewQuery representation.

@stephenafamo

Copy link
Copy Markdown
Owner

Will this work with a complex query that contains a CTE?

@snowyukitty

Copy link
Copy Markdown
Author

For SELECT-based CTEs, yes. Exists receives the complete v.BaseQuery, so Bob renders the WITH clause as part of the subquery rather than dropping or reconstructing it. The generated shape is:

SELECT EXISTS ((
  WITH matching AS (...)
  SELECT ...
  FROM matching
  WHERE ...
))

TestSomeViewExistsUsesExistsExpression intentionally exercises this path against PostgreSQL: it verifies that the CTE-backed query returns true, contains both EXISTS and WITH, and no longer emits count(1). Since the original BaseQuery is rendered intact, its parameters and remaining clauses retain their existing order.

I do not want to overstate the coverage, though: PostgreSQL requires data-modifying CTEs (INSERT/UPDATE/DELETE/MERGE) to be attached to the top-level statement. The current wrapper therefore supports SELECT-based CTEs, but it would not support that less common case. Bob's API can represent it, so if ViewQuery.Exists is expected to preserve data-modifying CTEs as well, I can update the implementation to keep the WITH clause at the outer level and add regression coverage for it.

The prior assertion only checked that "WITH" appeared somewhere in the
rendered query, which would also pass if the clause were hoisted ahead
of EXISTS. Assert instead that WITH is rendered inside the EXISTS
subquery, matching the SELECT-CTE guarantee the EXISTS optimization
relies on.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@snowyukitty

snowyukitty commented Jul 13, 2026

Copy link
Copy Markdown
Author

Withdrawn in e153723.

This follow-up was based on an incorrect rationale. The ordering assertion encoded the current SQL shape as a requirement, even though relative WITH placement is not the SELECT-CTE contract. More importantly, PostgreSQL requires data-modifying CTEs to be attached to the top-level statement, so the statement about hoisting changing their semantics was backwards.

I reverted the assertion; the original PostgreSQL-backed test remains unchanged. A correction is posted below for visibility.

Revert bb8a396, which treated nested WITH placement as a semantic requirement. The PostgreSQL-backed test already verifies SELECT-based CTE behavior. Hoisting can be valid for SELECT CTEs and is required for data-modifying CTEs, which PostgreSQL permits only at the top level; an ordering assertion would encode an implementation detail and block a correct future implementation.
@snowyukitty

Copy link
Copy Markdown
Author

Correction: I reverted bb8a396 in e153723.

The follow-up ordering assertion was too strict, and its rationale about data-modifying CTEs was incorrect. For the SELECT-based CTE covered by this PR, nesting the complete query inside EXISTS is valid, and the existing PostgreSQL-backed test verifies that behavior. However, the relative location of WITH is not itself a contract we should lock down: hoisting can also be valid for SELECT CTEs, and PostgreSQL requires data-modifying CTEs to be attached to the top-level statement.

The revert restores the original test and makes no production-code change. Sorry for the confusion.

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.

Should Exists call the actual Exists method in postgres instead of just count > 0

2 participants