Add SELECT DISTINCT ON support for PostgreSQL#77
Conversation
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>
|
👋 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 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 SummaryThis PR adds
Confidence Score: 5/5Safe 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.
|
| 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.]
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
|
@jules Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes. Issue 1 of 1pkg/rain/query_select.go:879-881
|
Thank you for the catch! I've updated |
…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>
Implemented
DISTINCT ONsupport forSelectQuery, specifically targeting the PostgreSQL dialect to match Drizzle ORM's behavior.Key changes:
FeatureSelectDistinctOninpkg/dialect/feature.go.pkg/dialect/postgres.go.DistinctOnmethod toSelectQueryinpkg/rain/query_select.go.writeSQL,clone,isBareCompound, andcompileinSelectQueryto handle the new clause.pkg/rain/query_select_test.gocovering success and failure cases.pkg/dialect/dialect_test.goandpkg/rain/query_write_test.goto account for the new feature flag.PR created automatically by Jules for task 10533173539318089553 started by @cungminh2710