Add support for index field sort direction (ASC/DESC)#941
Add support for index field sort direction (ASC/DESC)#941
Conversation
Support specifying sort direction (ASC/DESC) on entity-level @index fields using the syntax: @index(fields: [["field", "DESC"], "otherField"]) Each field in the index can be either a plain string (defaults to ASC) or a two-element list [fieldName, direction]. Direction is case-insensitive. Changes across the full pipeline: - Rust parsing: new IndexField/IndexFieldDirection types in entity_parsing.rs - Codegen templates: CompositeIndexFieldTemplate carries direction info - Handlebars template: generates compositeIndexField records with direction - ReScript Table.res: new compositeIndexField type with direction variant - PgStorage.res: new makeCreateCompositeIndexQuery emits "field DESC" in SQL - Tests: 7 new Rust tests + 3 new ReScript tests for direction parsing/SQL https://claude.ai/code/session_01Ujt5CWYiH3vwP2q3A7m1rB
📝 WalkthroughWalkthroughThis PR introduces direction support for composite database indices across the stack. New types ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
codegenerator/cli/npm/envio/src/db/Table.res (1)
346-369:⚠️ Potential issue | 🟠 MajorDirection is silently dropped for single-field composite indices.
When a composite index has exactly one field (e.g.,
@index(fields: [["myField", "DESC"]])),getSingleIndicesextracts only thefieldNameand discards thedirection. These single-field indices are then created viamakeCreateIndexQuerywhich doesn't support direction. The user's explicitDESCdirective would be silently ignored.Two possible fixes:
- Keep single-field-with-direction indices in
getCompositeIndicesso they go through the direction-aware path.- Or, add direction support to
makeCreateIndexQueryas well.Option 1: Don't demote single-field indices that have a non-default direction
let getSingleIndices = (table): array<string> => { let indexFields = table.fields->Array.keepMap(field => switch field { | Field(field) if field.isIndex => Some(field->getDbFieldName) | _ => None } ) table ->getUnfilteredCompositeIndicesUnsafe - ->Array.keepMap(cidx => - switch cidx { - | [{fieldName}] => Some([fieldName]) - | _ => None - } - ) + ->Array.keepMap(cidx => + switch cidx { + | [{fieldName, direction: Asc}] => Some([fieldName]) + | _ => None + } + ) ->Array.concat([indexFields]) ->Array.concatMany ->Set.String.fromArray ->Set.String.toArray ->Js.Array2.sortInPlace } -let getCompositeIndices = (table): array<array<compositeIndexField>> => { +let getCompositeIndices = (table): array<array<compositeIndexField>> => { table ->getUnfilteredCompositeIndicesUnsafe - ->Array.keep(ind => ind->Array.length > 1) + ->Array.keep(ind => + ind->Array.length > 1 || + (ind->Array.length == 1 && ind->Belt.Array.getExn(0).direction != Asc) + ) }codegenerator/cli/src/config_parsing/entity_parsing.rs (1)
556-569:⚠️ Potential issue | 🔴 CriticalDirection silently lost for single-field
@index(fields: [["fieldName", "DESC"]])definitions.When a user defines a single-field index with a direction (e.g.,
@index(fields: [["tokenId", "DESC"]])), the direction is parsed and stored inIndexField, but:
get_single_field_index()(Line 956) returns onlyOption<String>— extracting just the field name, not direction.get_composite_indices()(Line 562) filters forlen > 1— excluding single-field indexes entirely.- Downstream, the template renders single-field indexes with only
~isIndex: bool, while multi-field indexes use~compositeIndiceswith direction information.The user would receive no error or warning, but their explicit DESC specification would be ignored in the generated index. Consider either:
- Routing single-field indexes with non-default direction through
get_composite_indices(or a dedicated path that preserves direction), or- Emitting a validation error/warning when direction is specified on a single-field index.
🧹 Nitpick comments (3)
codegenerator/cli/npm/envio/src/PgStorage.res (1)
40-42: Index name doesn't encode direction — potential name collision for same-fields-different-directions.If two composite indices are declared on the same fields but with different sort orders (e.g.,
[a ASC, b ASC]and[a DESC, b DESC]), the generated index name will be identical (tableName_a_b).CREATE INDEX IF NOT EXISTSwill silently skip the second one. Consider encoding the direction in the index name (e.g., appending_descfor DESC fields) to avoid silent collisions.Suggested approach
let makeCreateCompositeIndexQuery = (~tableName, ~indexFields: array<Table.compositeIndexField>, ~pgSchema) => { let indexName = - tableName ++ "_" ++ indexFields->Js.Array2.map(f => f.fieldName)->Js.Array2.joinWith("_") + tableName ++ "_" ++ indexFields->Js.Array2.map(f => { + let suffix = switch f.direction { + | Table.Asc => "" + | Table.Desc => "_desc" + } + f.fieldName ++ suffix + })->Js.Array2.joinWith("_")codegenerator/cli/src/config_parsing/entity_parsing.rs (2)
952-954: Nit: prefer returning&[IndexField]over&Vec<IndexField>.Returning a slice is more idiomatic in Rust and more flexible for callers.
♻️ Suggested change
- pub fn get_index_fields(&self) -> &Vec<IndexField> { + pub fn get_index_fields(&self) -> &[IndexField] { &self.0 }
2556-2696: Good test coverage for the new direction feature.Tests cover DESC parsing, mixed formats, default ASC, invalid direction, wrong list length, case insensitivity, and composite index retrieval. Consider adding a test for a single-field index with direction (e.g.,
@index(fields: [["tokenId", "DESC"]])) to document the current behavior and prevent regressions — especially in light of the direction-loss concern noted above.
2 participants
Summary
This PR adds support for specifying sort direction (ASC/DESC) on database index fields. Previously, all index fields defaulted to ascending order. Now users can explicitly specify descending order for individual fields in composite indices.
Key Changes
Schema & Parsing (Rust)
IndexFieldDirectionenum withAscandDescvariantsIndexFieldstruct to hold field name and direction (replaces plainString)@indexdirective parsing to support two formats:"fieldName"(defaults to ASC)["fieldName", "DESC"]or["fieldName", "ASC"]["field1", ["field2", "DESC"]]MultiFieldIndexto work withIndexFieldinstead ofStringCode Generation (ReScript)
Table.resto usecompositeIndexFieldtype withfieldNameanddirectionPgStorage.resto generate SQL with proper direction syntax:directionToSqlhelper function to convert direction to SQL syntaxmakeCreateCompositeIndexQueryto handle direction in generated SQLTemplates & Output
CompositeIndexFieldTemplatestruct for template serializationTests
Implementation Details
https://claude.ai/code/session_01Ujt5CWYiH3vwP2q3A7m1rB
Summary by CodeRabbit
Release Notes
New Features
Improvements