Skip to content

Add SELECT DISTINCT ON support for PostgreSQL#77

Merged
cungminh2710 merged 2 commits into
mainfrom
feat-distinct-on-support-10533173539318089553
May 21, 2026
Merged

Add SELECT DISTINCT ON support for PostgreSQL#77
cungminh2710 merged 2 commits into
mainfrom
feat-distinct-on-support-10533173539318089553

Conversation

@cungminh2710
Copy link
Copy Markdown
Contributor

Implemented DISTINCT ON support for SelectQuery, specifically targeting the PostgreSQL dialect to match Drizzle ORM's behavior.

Key changes:

  • Defined FeatureSelectDistinctOn in pkg/dialect/feature.go.
  • Enabled the feature in pkg/dialect/postgres.go.
  • Added DistinctOn method to SelectQuery in pkg/rain/query_select.go.
  • Updated writeSQL, clone, isBareCompound, and compile in SelectQuery to handle the new clause.
  • Added unit tests in pkg/rain/query_select_test.go covering success and failure cases.
  • Updated existing tests in pkg/dialect/dialect_test.go and pkg/rain/query_write_test.go to account for the new feature flag.

PR created automatically by Jules for task 10533173539318089553 started by @cungminh2710

This change introduces the .DistinctOn(...schema.Expression) method to SelectQuery,
allowing developers to use PostgreSQL's DISTINCT ON clause.

- Added FeatureSelectDistinctOn to dialect features.
- Enabled FeatureSelectDistinctOn for PostgreSQL dialect.
- Updated SelectQuery with DistinctOn field and method.
- Implemented SQL rendering and validation for DISTINCT ON.
- Added comprehensive unit tests and updated existing feature tests.

Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR adds SELECT DISTINCT ON (...) support to SelectQuery, gated behind a new FeatureSelectDistinctOn dialect flag that is enabled only for PostgreSQL. All relevant compile paths (writeSQL, compile, compileAggregate, clone, isBareCompound) are correctly updated, and the previously flagged compileAggregate gap has been addressed in this revision.

  • Feature flag & dialect: FeatureSelectDistinctOn added to feature.go and enabled in postgres.go; unsupported dialects return a clear runtime error.
  • Query compilation: DISTINCT + DISTINCT ON mutual exclusion, compound-query guard, and aggregate-helper guard are all enforced with descriptive errors.
  • Tests: Eight new table-driven cases cover the happy path (single column, multiple columns, raw expression), unsupported dialects, and all four error conditions.

Confidence Score: 5/5

Safe to merge — the new DISTINCT ON path is cleanly isolated behind a dialect feature flag with no changes to existing query compilation paths.

All changed files are additive: a new field, a new method, guards in already-tested compile functions, and new test cases. The previous gap in compileAggregate has been addressed. The only finding is a style concern in the test dispatch logic that does not affect correctness.

No files require special attention; query_select_test.go has a minor test-design smell but no correctness impact.

Important Files Changed

Filename Overview
pkg/dialect/feature.go Adds FeatureSelectDistinctOn as a new iota constant at the end of the feature list — clean and non-breaking.
pkg/dialect/postgres.go Enables FeatureSelectDistinctOn in the Postgres dialect's feature mask — straightforward one-liner addition.
pkg/rain/query_select.go Core implementation: adds distinctOn []schema.Expression field, DistinctOn() method, SQL rendering, clone() copy, isBareCompound() guard, and compileAggregate error guard — all handled correctly.
pkg/rain/query_select_test.go Adds TestSelectDistinctOnToSQL with 8 cases covering success, unsupported dialects, and error conditions; uses a fragile name-based dispatch (strings.HasPrefix) to route aggregate-helper tests through Count() rather than a dedicated struct field.
pkg/dialect/dialect_test.go Updates Postgres feature-flag assertion to include FeatureSelectDistinctOn — correctly aligned with the new dialect definition.
pkg/rain/query_write_test.go Updates the Postgres feature-flag assertion in the write-test suite to include FeatureSelectDistinctOn.
pkg/rain/query_runtime_internal_test.go Updates the expected error string for the prepared-query aggregate guard to include DISTINCT ON — correctly tracks the error message change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SelectQuery.compile] --> B{distinct && distinctOn?}
    B -- yes --> ERR1[error: DISTINCT and DISTINCT ON cannot be used together]
    B -- no --> C{firstOperand != nil?}
    C -- yes --> D{distinctOn set?}
    D -- yes --> ERR2[error: compound queries do not support DISTINCT ON]
    D -- no --> E[compile compound query]
    C -- no --> F[SelectQuery.writeSQL]
    F --> G{distinct set?}
    G -- yes --> H[emit DISTINCT]
    G -- no --> I{distinctOn set?}
    I -- yes --> J{dialect supports FeatureSelectDistinctOn?}
    J -- no --> ERR3[error: SELECT DISTINCT ON not supported]
    J -- yes --> K[emit DISTINCT ON exprs]
    I -- no --> L[emit plain SELECT]
    K --> L
    H --> L
    L --> M[emit columns, FROM, WHERE, etc.]

    N[compileAggregate] --> O{distinct or distinctOn or groupBy or having?}
    O -- yes --> ERR4[error: aggregate helpers do not support DISTINCT / DISTINCT ON / GROUP BY / HAVING]
    O -- no --> P[emit COUNT/SUM/etc.]
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
pkg/rain/query_select_test.go:991-997
The test case routing uses `strings.HasPrefix(tt.name, "aggregate helper fails")` to decide whether to call `Count()` or `ToSQL()`. This couples the execution path to the test name string — if anyone renames this case (or adds another aggregate-error case with a different prefix), the dispatch silently shifts to the `ToSQL()` path. The test won't silently pass (because `wantErr` is set and `ToSQL()` succeeds), but it will fail with a confusing error rather than a clear compilation signal. A dedicated boolean field in the struct makes the intent explicit and refactor-safe.

```suggestion
	type tc struct {
		name         string
		dialect      string
		build        func(*rain.DB) *rain.SelectQuery
		wantSQL      string
		wantErr      string
		useAggregate bool // if true, test via Count() instead of ToSQL()
	}
```

Reviews (2): Last reviewed commit: "feat(rain): implement SELECT DISTINCT ON..." | Re-trigger Greptile

@cungminh2710
Copy link
Copy Markdown
Contributor Author

@jules Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.


Issue 1 of 1

pkg/rain/query_select.go:879-881
compileAggregate silently drops distinctOn expressions

compileAggregate guards against q.distinct but not len(q.distinctOn) > 0. A caller doing db.Select().Table(users).DistinctOn(users.ID).Count(ctx) will silently receive a full-table COUNT(*) instead of an error, giving a numerically wrong result. The q.distinct path already demonstrates the intended guard — distinctOn needs the same treatment.

	if q.distinct || len(q.distinctOn) > 0 || len(q.groupBy) > 0 || len(q.having) > 0 {
		return compiledQuery{}, errors.New("rain: aggregate helpers do not support DISTINCT, DISTINCT ON, GROUP BY, or HAVING clauses")
	}

@google-labs-jules
Copy link
Copy Markdown
Contributor

@jules Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.


Issue 1 of 1

pkg/rain/query_select.go:879-881
compileAggregate silently drops distinctOn expressions

compileAggregate guards against q.distinct but not len(q.distinctOn) > 0. A caller doing db.Select().Table(users).DistinctOn(users.ID).Count(ctx) will silently receive a full-table COUNT(*) instead of an error, giving a numerically wrong result. The q.distinct path already demonstrates the intended guard — distinctOn needs the same treatment.

	if q.distinct || len(q.distinctOn) > 0 || len(q.groupBy) > 0 || len(q.having) > 0 {
		return compiledQuery{}, errors.New("rain: aggregate helpers do not support DISTINCT, DISTINCT ON, GROUP BY, or HAVING clauses")
	}

Thank you for the catch! I've updated compileAggregate to guard against DISTINCT ON expressions and added a test case to verify this behavior.

…gate guard

This change introduces the .DistinctOn(...schema.Expression) method to SelectQuery,
allowing developers to use PostgreSQL's DISTINCT ON clause.

- Added FeatureSelectDistinctOn to dialect features.
- Enabled FeatureSelectDistinctOn for PostgreSQL dialect.
- Updated SelectQuery with DistinctOn field and method.
- Implemented SQL rendering and validation for DISTINCT ON.
- Updated compileAggregate to guard against DISTINCT ON expressions.
- Added comprehensive unit tests and updated existing tests.

Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
@cungminh2710 cungminh2710 merged commit 4a1ed21 into main May 21, 2026
5 checks passed
@cungminh2710 cungminh2710 deleted the feat-distinct-on-support-10533173539318089553 branch May 21, 2026 01:26
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.

1 participant