fix(postgres): preserve aggregate and array cast arity - #5845
Open
tensordreams wants to merge 1 commit into
Open
fix(postgres): preserve aggregate and array cast arity#5845tensordreams wants to merge 1 commit into
tensordreams wants to merge 1 commit into
Conversation
Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
Summary by CodeRabbit
WalkthroughThe changes preserve PostgreSQL array typing in JSON object casts for money and bigint columns. Aggregate average and sum selections now mark their column expressions as selected, with native money aggregate coverage added. PostgreSQL enum-array migration SQL now casts through text arrays, and migration tests cover populated enum arrays plus simultaneous scalar and array enum changes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@schema-engine/sql-migration-tests/tests/migrations/enums.rs`:
- Around line 545-546: Extend the migration test around the enum-array insert
and the `02remove` migration to query the seeded `Test` row afterward and assert
that `positions` remains `["First", "Second"]` in the original order. Keep the
existing migration-application assertion, and add the value-preservation check
specifically after `02remove`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 20f7df22-25c2-4b4b-ace4-b56865ec4ab2
📒 Files selected for processing (5)
quaint/src/visitor/postgres.rsquery-compiler/query-builders/sql-query-builder/src/read.rsquery-engine/connector-test-kit-rs/query-engine-tests/tests/queries/data_types/native/postgres.rsschema-engine/connectors/sql-schema-connector/src/flavour/postgres/renderer.rsschema-engine/sql-migration-tests/tests/migrations/enums.rs
Comment on lines
+545
to
+546
| api.raw_cmd(r#"INSERT INTO "Test" ("positions") VALUES (ARRAY['First', 'Second']::"Position"[])"#); | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Assert that enum-array values survive the migration.
The test seeds positions but only verifies that the migration applies. Query the row after 02remove and assert that ["First", "Second"] is unchanged; otherwise a lossy cast could pass this test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schema-engine/sql-migration-tests/tests/migrations/enums.rs` around lines 545
- 546, Extend the migration test around the enum-array insert and the `02remove`
migration to query the seeded `Test` row afterward and assert that `positions`
remains `["First", "Second"]` in the original order. Keep the existing
migration-application assertion, and add the value-preservation check
specifically after `02remove`.
Contributor
Wasm Query Compiler File Size
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the remaining PostgreSQL type-conversion gaps surfaced by MONEY aggregates in prisma/prisma#28681 and related array-cast paths.
Changes
_sumand_avginputs for result coercion so PostgreSQL evaluatesSUM(column::numeric)andAVG(column::numeric).numeric[]andtext[]casts for list fields.text[]while retaining scalartextcasts for scalar enum columns.Why
PostgreSQL resolves aggregate overloads from their input types, while JSON serialization and enum migrations must preserve whether the source is scalar or an array. Carrying arity through each renderer avoids locale-dependent MONEY output and prevents arrays from being coerced through scalar text representations.
Verification
cargo fmt -- --checkcargo test -p sql-query-builder --lib_sumand_avglocalhost:5438