-
-
Notifications
You must be signed in to change notification settings - Fork 56
feat: add the ability to specify range bounds #371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new abstract class, Changes
Sequence Diagram(s)sequenceDiagram
participant DQL_User
participant Doctrine
participant BaseRangeFunction
participant PostgreSQL
DQL_User->>Doctrine: Write DQL using range function (e.g., INT4RANGE(a, b, '[)')
Doctrine->>BaseRangeFunction: Parse and map function call
BaseRangeFunction->>PostgreSQL: Generate SQL (e.g., int4range(a, b, '[)'))
PostgreSQL-->>BaseRangeFunction: Return query result
BaseRangeFunction-->>Doctrine: Pass result
Doctrine-->>DQL_User: Return final result
Possibly related PRs
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (13)
⏰ Context from checks skipped due to timeout of 90000ms (20)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (1)
21-32: Use a non-default bounds literal to exercise the new feature.
'[)'is PostgreSQL’s default bounds, so the two‐argument and three-argument variants currently generate identical SQL semantics.
To genuinely verify that the third argument is parsed and propagated, use a different bounds literal such as'()','[]', or'(]'.- 'SELECT tsrange(c0_.datetime1, c0_.datetime2, \'[)\') AS sclr_0 FROM ContainsDates c0_', + 'SELECT tsrange(c0_.datetime1, c0_.datetime2, \'()\' ) AS sclr_0 FROM ContainsDates c0_', ... - \sprintf('SELECT TSRANGE(e.datetime1, e.datetime2, \'[)\') FROM %s e', ContainsDates::class), + \sprintf('SELECT TSRANGE(e.datetime1, e.datetime2, \'()\' ) FROM %s e', ContainsDates::class),This will fail if the bounds argument is not wired correctly, giving the test real discriminative power.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (1)
15-20: Bounds-string inputs like'infinity'won’t match the default node pattern
BaseRangeFunctionexpects the first two arguments to beArithmeticPrimary.
PostgreSQL allows numeric ranges to use textual literals such as'infinity'/'-infinity'.
Those literals are parsed by Doctrine asStringPrimary, which means a DQL such as:NUMRANGE('infinity', 10)would throw a
QueryExceptionbecause the node mapping pattern rejects the first argument.If supporting these literals is desirable (they are frequently used when the upper or lower
bound is open-ended), consider overridinggetNodeMappingPattern()here so that the first
two positions accept eitherArithmeticPrimaryorStringPrimary:protected function getNodeMappingPattern(): array { - return parent::getNodeMappingPattern(); + return [ + 'ArithmeticPrimary|StringPrimary', + 'ArithmeticPrimary|StringPrimary', + 'StringPrimary', + ]; }No change is needed if the project explicitly forbids the textual form, but please double-check
the intended coverage.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (1)
21-24: Nice coverage – consider parametrising more bound variantsAdding the
'[)'case strengthens the test suite 🎉.
Because PostgreSQL accepts four bound specifications ('[)','[ ]','( ]','( )'),
you may squeeze even more value from this test by parametrising the bounds argument and
asserting the translation for all four possibilities. That keeps behaviour tight if a
future refactor accidentally hard-codes a single default.Example outline:
/** @dataProvider boundsProvider */ public function testTstzrangeWithBounds(string $bounds) { $dql = sprintf( 'SELECT TSTZRANGE(e.datetimetz1, e.datetimetz2, %s) FROM %s e', $bounds, ContainsDates::class ); $this->assertDqlProducesSql($dql); } public static function boundsProvider(): array { return ["'[)'", "'[]'", "'(]'", "'()'"]; }Also applies to: 29-32
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (1)
22-28: Consider reducing duplication in node-mapping overridesEvery range function whose elements are expressed as strings (
tsrange,tstzrange, most use–cases fordaterange) now has to fully overridegetNodeMappingPattern()just to swapArithmeticPrimary→StringPrimaryfor the first two arguments. This is a bit of boiler-plate and risks drift if the default pattern ever changes.Two possible follow-ups:
-abstract class BaseRangeFunction extends BaseVariadicFunction +abstract class BaseRangeFunction extends BaseVariadicFunction +{ + /** Override in children when the first two args are not ArithmeticPrimary */ + protected function getElementNodeType(): string + { + return 'ArithmeticPrimary'; + } + + protected function getNodeMappingPattern(): array + { + $element = $this->getElementNodeType(); + return [$element, $element, 'StringPrimary']; + }Then
Tsrangeonly needs:protected function getElementNodeType(): string { return 'StringPrimary'; }This keeps the per-function classes minimal and centralises the mapping logic.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
24-32: Bounds argument is fixed toStringPrimary– allow parameters?At present the third argument can only be a literal node; a bound parameter such as
:boundswould be rejected. If you foresee dynamic bounds (e.g. user-selectable inclusivity), consider adding'StringPrimary' | 'InputParameter'to the mapping or documenting the constraint.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php (1)
17-20: Minor nit – final newlineThe file ends without a newline (POSIX convention). Adding it avoids diff noise in future edits.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (1)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#263
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php:19-21
Timestamp: 2025-03-11T12:32:10.726Z
Learning: In the postgresql-for-doctrine repository, PostgreSQL range functions have distinct implementations for different data types. The `Numrange` function works with numeric/decimal values and is tested using the `ContainsDecimals` fixture with properties typed as `float`. In contrast, the `Int4range` function works with 32-bit integers and is tested using the `ContainsIntegers` fixture with properties typed as `int`. While the PHP implementations share a similar structure (extending `BaseFunction`), they are semantically different as they handle different PostgreSQL data types.
🧬 Code Graph Analysis (10)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (4)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (1)
getDqlStatements(27-33)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (4)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (1)
getDqlStatements(27-33)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
BaseRangeFunction(13-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (1)
getFunctionName(17-20)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
BaseRangeFunction(13-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (1)
getFunctionName(17-20)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (4)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (1)
getDqlStatements(27-33)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (4)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (1)
getDqlStatements(27-33)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
BaseRangeFunction(13-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (1)
getFunctionName(17-20)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (4)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php (1)
getDqlStatements(27-33)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php (1)
getDqlStatements(27-33)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (2)
BaseRangeFunction(13-33)getNodeMappingPattern(15-22)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (2)
getFunctionName(17-20)getNodeMappingPattern(22-29)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (2)
BaseRangeFunction(13-33)getNodeMappingPattern(15-22)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (1)
getFunctionName(17-20)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (2)
getFunctionName(17-20)getNodeMappingPattern(22-29)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
- GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer latest
🔇 Additional comments (11)
README.md (1)
73-75: Documentation addition for range functions looks good!This addition nicely categorizes the PostgreSQL range functions supported by the library, making it clear to users which range types are available. The grouping into arithmetic ranges and date ranges is particularly helpful for navigation.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php (2)
23-23: Implementation of range bound test looks good!This test case correctly extends the coverage to include the optional third parameter for specifying range bounds as '[)' in the SQL output, which aligns with the PR objective of supporting custom range bounds.
31-31: DQL test with range bounds parameter is correctly implemented.This test case properly demonstrates the usage of INT8RANGE with three parameters, where the third parameter '[)' specifies the bounds of the range. This matches the SQL expectation defined in the getExpectedSqlStatements method and ensures the function correctly handles the optional bounds parameter.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php (2)
23-23: SQL test verification for NUMRANGE with bounds parameter looks good!This test case correctly validates that the SQL output includes the third parameter for specifying range bounds as '[)', which is consistent with the implementation for other range functions.
31-31: DQL test with range bounds parameter is properly implemented.This test case correctly demonstrates the usage of NUMRANGE with three parameters, with the third parameter specifying the bounds as '[)'. This is consistent with the approach used for other range functions and aligns with PostgreSQL's range type construction capabilities.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php (2)
23-23: SQL test verification for INT4RANGE with bounds parameter looks good!This test case correctly validates that the SQL output includes the third parameter for specifying range bounds as '[)', maintaining consistency with the implementation for other range functions.
31-31: DQL test with range bounds parameter is well implemented.This test case properly demonstrates the usage of INT4RANGE with the optional third parameter for specifying bounds. The test uses the ContainsIntegers fixture appropriately, in line with the retrieved learning that Int4range works with 32-bit integers.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php (1)
15-20: 👍 Refactor looks good.Migrating to
BaseRangeFunctionremoves boilerplate while keeping behaviour unchanged.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (1)
15-30: Implementation aligns perfectly with the newBaseRangeFunctionabstractionThe class cleanly overrides only what differs from the default: the SQL name and the
StringPrimarymapping that timestamp-TZ expressions require. This keeps the surface
area minimal and consistent with sibling range functions – great work!src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (1)
17-20: Function name override looks good
getFunctionName()cleanly delegates the literal to the parent class – concise and correct.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (1)
15-22: Validate thatArithmeticPrimaryis always appropriate for the first two arguments
ArithmeticPrimarycovers numeric literals/expressions, but not quoted date/time literals which Doctrine usually parses asStringPrimary. Fordaterangeor when using bound parameters (:from,:to) this mapping might fail.Because some child classes override the pattern (good), consider either:
- Documenting that children must override when their element type is not purely numeric, or
- Generalising the default pattern (see earlier suggestion in
Tsrange.php).This avoids subtle bugs where a developer assumes parameters will just work across all range functions.
| 'StringPrimary', | ||
| 'StringPrimary', | ||
| 'StringPrimary', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', |
| 'StringPrimary', | ||
| 'StringPrimary', | ||
| 'StringPrimary', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai - does this look alright to you? Analyse the usage of getNodeMappingPattern and cross-check with Postgresql's function signature. You may need to revise similar changes in the current PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@martin-georgiev I've analyzed the usage of getNodeMappingPattern and cross-checked with PostgreSQL's function signatures, and found a potential issue.
The code in Tsrange.php correctly overrides getNodeMappingPattern() to use three StringPrimary nodes, which is appropriate for timestamp ranges where all parameters (including bounds) are passed as strings.
However, I noticed an inconsistency:
BaseRangeFunctiondefines the default pattern as[ArithmeticPrimary, ArithmeticPrimary, StringPrimary]TsrangeandTstzrangeoverride it to use threeStringPrimarynodesDaterangealso uses string literals in the tests, but doesn't override the pattern
This means Daterange inherits the default pattern with ArithmeticPrimary nodes, but is actually used with string literals in tests. For consistency, Daterange should also override getNodeMappingPattern() similar to Tsrange and Tstzrange.
Would you like me to provide a code suggestion for the Daterange class?
(_/)
(•ㅅ•)
/ づ ♥ ~thinking...
🏁 Script executed:
#!/bin/bash
# First, let's examine the BaseRangeFunction class
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php
# Let's also see if there are any other range functions that have been modified
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php
# Let's check some of the tests to understand the usage
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php
Length of output: 2443
🏁 Script executed:
#!/bin/bash
# Let's check the other range functions to see if they properly override getNodeMappingPattern
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php
Length of output: 2125
🏁 Script executed:
#!/bin/bash
# Let's check the Daterange tests to see how dates are passed
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DaterangeTest.php
Length of output: 853
🏁 Script executed:
#!/bin/bash
# Look at how these range functions are used in the tests, specifically in DQL statements
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DaterangeTest.php
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php
Length of output: 822
🌐 Web query:
PostgreSQL tsrange function signature and parameters
💡 Result:
PostgreSQL's tsrange is a built-in range type that represents a range of timestamp values without time zone. This range type is useful for working with time periods in PostgreSQL databases.
Function Signature and Parameters
The tsrange constructor function can be used with two different parameter patterns:
-
Two-parameter form: Creates a range with inclusive lower bound and exclusive upper bound (normal form)
tsrange(lower_bound timestamp, upper_bound timestamp)
-
Four-parameter form: Creates a range with explicitly specified bound inclusivity
tsrange(lower_bound timestamp, upper_bound timestamp, lower_inclusive boolean, upper_inclusive boolean)
Where:
lower_bound: The starting timestamp of the rangeupper_bound: The ending timestamp of the rangelower_inclusive: Boolean indicating whether the lower bound is inclusive (true) or exclusive (false)upper_inclusive: Boolean indicating whether the upper bound is inclusive (true) or exclusive (false)
Usage Examples
The tsrange type is especially useful for scenarios involving time periods, such as reservations, events, or availability windows:
-- Create a reservation table using tsrange
CREATE TABLE reservation (
room int,
during tsrange
);
-- Insert a reservation for a specific time period
INSERT INTO reservation VALUES (1108, '[2010-01-01 14:30, 2010-01-01 15:30)');You can also query records that fall within a specific time range:
-- Select orders created within a specific time range
SELECT * FROM orders
WHERE created_at <@ tsrange('2023-03-14 04:00:00', '2023-03-14 10:00:00');PostgreSQL also provides a corresponding multirange type called tsmultirange, which represents an ordered list of non-contiguous, non-empty, non-null timestamp ranges[1].
Citations:
- 1: https://www.postgresql.org/docs/current/rangetypes.html
- 2: https://www.dbvis.com/thetable/a-complete-guide-on-postgresql-range-types-functions-and-operators/
- 3: https://www.postgresql.org/docs/9.3/functions-range.html
- 4: https://www.postgresql.org/docs/current/sql-expressions.html
- 5: https://docs.mobilitydb.com/MobilityDB/develop/ch02.html
- 6: https://www.postgresql.org/docs/current/functions-info.html
@martin-georgiev I've analyzed the usage of getNodeMappingPattern in the Tsrange class and cross-checked with PostgreSQL's function signatures.
The implementation looks correct for the following reasons:
-
TsrangeoverridesgetNodeMappingPattern()to use threeStringPrimarynodes, which is appropriate since PostgreSQL timestamp ranges expect string parameters for both timestamps and bounds. -
Other date/time range functions follow the same pattern:
Tstzrangeuses threeStringPrimarynodesDaterangealso correctly overrides the pattern with threeStringPrimarynodes
-
The default pattern in
BaseRangeFunction([ArithmeticPrimary, ArithmeticPrimary, StringPrimary]) is suited for numeric ranges likeint4range,int8range, andnumrange, where the first two parameters are numbers. -
All the tests confirm this design, showing that:
- Date/time ranges are used with string literals
- Numeric ranges are used with numeric literals
This implementation correctly handles PostgreSQL's range function signatures, including the optional third parameter for specifying bounds. No revisions appear necessary for the current PR.
🧬 Code Graph Analysis Results
Relevant snippets for analyzing getNodeMappingPattern usage in Tsrange.php and cross-checking with PostgreSQL function signature:
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (lines 1-29)
<?php
declare(strict_types=1);
namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;
/**
* Implementation of PostgreSQL TSRANGE().
*
* @see https://www.postgresql.org/docs/17/rangetypes.html
* @since 2.9
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Tsrange extends BaseRangeFunction
{
protected function getFunctionName(): string
{
return 'tsrange';
}
protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
}
}File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (lines 13-33)
abstract class BaseRangeFunction extends BaseVariadicFunction
{
protected function getNodeMappingPattern(): array
{
return [
'ArithmeticPrimary',
'ArithmeticPrimary',
'StringPrimary',
];
}
protected function getMinArgumentCount(): int
{
return 2;
}
protected function getMaxArgumentCount(): int
{
return 3;
}
}- This base class defines a default
getNodeMappingPatternwith twoArithmeticPrimaryand oneStringPrimarynodes. - It also defines minimum and maximum argument counts (2 and 3 respectively).
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (lines 17-29)
protected function getFunctionName(): string
{
return 'daterange';
}
protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
}- This function uses the same node mapping pattern as
Tsrange.
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (lines 17-29)
protected function getFunctionName(): string
{
return 'tstzrange';
}
protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
}- This function also uses the same node mapping pattern as
Tsrange.
Summary of relevant context:
TsrangeoverridesgetNodeMappingPatternto return three'StringPrimary'elements.- The base class
BaseRangeFunctionreturns a different pattern: two'ArithmeticPrimary'and one'StringPrimary'. - Other range functions like
DaterangeandTstzrangealso use the three'StringPrimary'pattern. - PostgreSQL's
tsrangefunction signature (per official docs) accepts:- Two timestamp values (lower and upper bounds),
- An optional text parameter for range bounds (e.g., '[)', '()').
This matches the three parameters represented as strings in the node mapping pattern.
Additional snippet for comparison (RegexpReplace.php) showing complex node mapping patterns:
protected function getNodeMappingPattern(): array
{
/*
* PostgreSQL overloads the 4th argument depending on its type:
* - if the 4th arg is a string, it’s taken as flags.
* - if the 4th arg is an integer, it’s taken as start position. This can be extended with the Nth argument.
*/
return [
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary,StringPrimary', // with start, N and flags: regexp_replace(string, pattern, replacement, 3, 2, 'i')
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary', // with start and N: regexp_replace(string, pattern, replacement, 3, 2)
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary', // with start: regexp_replace(string, pattern, replacement, 3)
'StringPrimary,StringPrimary,StringPrimary,StringPrimary', // with flags: regexp_replace(string, pattern, replacement, 'i')
'StringPrimary,StringPrimary,StringPrimary', // basic replacement: regexp_replace(string, pattern, replacement)
];
}- Shows that
getNodeMappingPatterncan return multiple patterns to handle function overloading. Tsrangereturns a single pattern array with three'StringPrimary'elements.
Summary for reviewer:
- The
Tsrangeclass overridesgetNodeMappingPatternto expect three string-type nodes, consistent with the PostgreSQLtsrangefunction signature: two timestamps and an optional text parameter. - The base class
BaseRangeFunctionuses a different default pattern (two arithmetic and one string), butTsrangeand similar range functions (Daterange,Tstzrange) override it to three strings. - This pattern aligns with the PostgreSQL documentation for
tsrange(lower timestamp, upper timestamp, text bounds). - Other range functions in the same namespace follow the same pattern, indicating consistency.
- The
getNodeMappingPatternmethod can return multiple patterns for overloaded functions, butTsrangeuses a single pattern.
This context should assist in verifying the correctness of getNodeMappingPattern usage in Tsrange.php and guide any necessary revisions in the current PR or similar functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? Explain why RegexpCount and RegexpInstr look different then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Let's check the implementations of RegexpCount and RegexpInstr
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCount.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstr.php
# Let's also look at what base class they extend from
grep -A 1 "class RegexpCount" src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCount.php
grep -A 1 "class RegexpInstr" src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstr.php
# Compare with Tsrange implementation again for reference
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php
# Compare with BaseRangeFunction
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php
# Let's also check if there's a base class for regexp functions
ls -la src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRegexpFunction.php 2>/dev/null || echo "BaseRegexpFunction.php not found"
Length of output: 4544
🏁 Script executed:
#!/bin/bash
# Let's check the BaseVariadicFunction to understand how the node mapping pattern is used
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php
# Also check the tests for RegexpCount and RegexpInstr to understand how they're used
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCountTest.php 2>/dev/null || echo "RegexpCountTest.php not found"
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php 2>/dev/null || echo "RegexpInstrTest.php not found"
Length of output: 12031
@martin-georgiev I understand your question now. The difference between RegexpCount/RegexpInstr and Tsrange implementations is intentional and relates to how they handle function overloads in PostgreSQL:
Why They Look Different
-
Different Function Complexity:
- RegexpCount/RegexpInstr: Support multiple function signatures with varying parameter counts (2-4 and 2-7 parameters respectively)
- Tsrange: Supports a simpler function with 2-3 parameters of consistent types
-
Node Mapping Format:
- RegexpCount/RegexpInstr use comma-separated strings in an array:
protected function getNodeMappingPattern(): array { return [ 'StringPrimary,StringPrimary,ArithmeticPrimary,StringPrimary', // 4 params 'StringPrimary,StringPrimary,ArithmeticPrimary', // 3 params 'StringPrimary,StringPrimary', // 2 params ]; }
- Tsrange uses individual array elements:
protected function getNodeMappingPattern(): array { return [ 'StringPrimary', 'StringPrimary', 'StringPrimary', ]; }
- RegexpCount/RegexpInstr use comma-separated strings in an array:
-
How BaseVariadicFunction Processes Them:
- For RegexpCount/RegexpInstr: It tries each pattern string, splitting it by commas and matching arguments until one succeeds
- For Tsrange: It uses the single pattern with consistent parameter types
Both implementations are correct but serve different purposes based on the complexity of the PostgreSQL functions they represent. The regexp functions need to handle multiple overloads with varying parameter counts, while range functions have a more consistent signature.
🧬 Code Graph Analysis Results
Relevant snippets for understanding Tsrange.php and the user comment about RegexpCount and RegexpInstr differences:
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (lines 1-29)
<?php
declare(strict_types=1);
namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;
/**
* Implementation of PostgreSQL TSRANGE().
*
* @see https://www.postgresql.org/docs/17/rangetypes.html
* @since 2.9
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Tsrange extends BaseRangeFunction
{
protected function getFunctionName(): string
{
return 'tsrange';
}
protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
}
}File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (lines 13-33)
abstract class BaseRangeFunction extends BaseVariadicFunction
{
protected function getNodeMappingPattern(): array
{
return [
'ArithmeticPrimary',
'ArithmeticPrimary',
'StringPrimary',
];
}
protected function getMinArgumentCount(): int
{
return 2;
}
protected function getMaxArgumentCount(): int
{
return 3;
}
}- Abstract base class for range functions.
- Defines a default node mapping pattern expecting two
ArithmeticPrimaryand oneStringPrimaryarguments. - Defines minimum and maximum argument counts (2 to 3).
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (lines 17-20)
protected function getFunctionName(): string
{
return 'numrange';
}- Example of another range function overriding the function name.
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (lines 17-29)
protected function getFunctionName(): string
{
return 'daterange';
}
protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
}- Shows a range function overriding the node mapping pattern to three
StringPrimaryarguments, similar toTsrange.
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpReplace.php (lines 22-41)
protected function getNodeMappingPattern(): array
{
/*
* PostgreSQL overloads the 4th argument depending on its type:
* - if the 4th arg is a string, it’s taken as flags.
* - if the 4th arg is an integer, it’s taken as start position. This can be extended with the Nth argument.
*/
return [
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary,StringPrimary', // with start, N and flags: regexp_replace(string, pattern, replacement, 3, 2, 'i')
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary', // with start and N: regexp_replace(string, pattern, replacement, 3, 2)
'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary', // with start: regexp_replace(string, pattern, replacement, 3)
'StringPrimary,StringPrimary,StringPrimary,StringPrimary', // with flags: regexp_replace(string, pattern, replacement, 'i')
'StringPrimary,StringPrimary,StringPrimary', // basic replacement: regexp_replace(string, pattern, replacement)
];
}
protected function getFunctionName(): string
{
return 'regexp_replace';
}- Shows a complex node mapping pattern with multiple overloads depending on argument types.
- Demonstrates why some functions like
RegexpReplacehave more complex patterns compared to simpler range functions.
Summary of relevance:
Tsrange.phpoverridesgetNodeMappingPattern()to expect threeStringPrimaryarguments, differing from the base classBaseRangeFunctionwhich expects twoArithmeticPrimaryand oneStringPrimary.- Other range functions like
Daterangealso override the node mapping pattern similarly. - The
RegexpReplacefunction shows a more complex and varied node mapping pattern due to PostgreSQL's function overloading and argument type variations. - This contrast explains why
RegexpCountandRegexpInstrmight have different node mapping patterns compared to simpler range functions likeTsrange.
These snippets provide the factual context needed to understand the differences in node mapping patterns and function implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai Now revise your approval of the PR and request changes where they are relevant!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A summary of the edits CodeRabbit can apply:
Request changes to Tsrange.php so that getNodeMappingPattern() returns a single-element array ['StringPrimary'], fixing the mapping-bug in BaseVariadicFunction.
Request changes to Daterange.php so that getNodeMappingPattern() returns ['StringPrimary'] only, avoiding the three-entry bug.
Request changes to Tstzrange.php so that getNodeMappingPattern() returns ['StringPrimary'] only, correcting the pattern application bug.
Modify Daterange.php so getNodeMappingPattern returns a single 'StringPrimary' element.
Modify Tsrange.php so getNodeMappingPattern returns a single 'StringPrimary' element.
Modify Tstzrange.php so getNodeMappingPattern returns a single 'StringPrimary' element.
The Tstzrange.php file will be updated to use a single-element node mapping pattern ['StringPrimary'] so that all parameters are correctly treated as StringPrimary.
I will request changes in Tsrange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.
I will request changes in Daterange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.
I will request changes in Tstzrange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.
The getNodeMappingPattern in Tsrange.php will be simplified to a single 'StringPrimary'.
The getNodeMappingPattern in Tstzrange.php will be simplified to a single 'StringPrimary'.
The getNodeMappingPattern in Daterange.php will be simplified to a single 'StringPrimary'.
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):
✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php
Remove the existing multi-line return array in getNodeMappingPattern():
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
Replace it with a single-element return immediately after the method signature:
return ['StringPrimary'];
Save Tsrange.php and run the TsrangeTest suite to verify that both 2-argument and 3-argument forms of tsrange now parse without errors.
✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php
Remove the existing multi-line return array in getNodeMappingPattern():
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
Replace it with:
return ['StringPrimary'];
Save Daterange.php and run the DaterangeTest suite to confirm both 2-argument and 3-argument usages of daterange parse correctly.
✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php
Remove the existing multi-line return array in getNodeMappingPattern():
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
];
Replace it with:
return ['StringPrimary'];
Save Tstzrange.php and run the TstzrangeTest suite to ensure both 2-argument and 3-argument invocations of tstzrange parse correctly.
A summary of the context that CodeRabbit is considering across the codebase:
We need to locate the BaseVariadicFunction definition and see how it handles a single-element node mapping pattern to confirm that returning ['StringPrimary'] will correctly apply that type to all arguments.
Locate every subclass of BaseRangeFunction that overrides getNodeMappingPattern, so we know which files need updating.
Inspect Daterange.php to determine the exact lines to replace.
Inspect Tsrange.php to determine the exact lines to replace.
Inspect Tstzrange.php to determine the exact lines to replace.
Confirm that BaseRangeFunction defines a minimum of 2 and a maximum of 3 arguments, so a single‐element pattern will correctly apply to both two‐ and three‐parameter invocations.
Identify all date/time range function classes to ensure consistent updates across Tsrange, Daterange, and Tstzrange.
Locate the existing override of getNodeMappingPattern() in Tsrange.php to determine the exact lines to replace.
Locate the existing override of getNodeMappingPattern() in Daterange.php to determine the exact lines to replace.
Locate the existing override of getNodeMappingPattern() in Tstzrange.php to determine the exact lines to replace.
Identify all range‐function classes that override getNodeMappingPattern so we can update their patterns to a single 'StringPrimary'.
- Created local commit - (🔄 Check again to try again)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
CodeRabbit has failed to create a local commit with your edits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
CodeRabbit has failed to create a local commit with your edits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
CodeRabbit has failed to create a local commit with your edits.
|
|
||
| /** | ||
| * @see https://www.postgresql.org/docs/17/rangetypes.html | ||
| * @since 3.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @since 3.1 | |
| * @since 3.2 |
| 'StringPrimary', | ||
| 'StringPrimary', | ||
| 'StringPrimary', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', |
| 'StringPrimary', | ||
| 'StringPrimary', | ||
| 'StringPrimary', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', | |
| 'StringPrimary', |
| 'ArithmeticPrimary', | ||
| 'ArithmeticPrimary', | ||
| 'StringPrimary', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'ArithmeticPrimary', | |
| 'ArithmeticPrimary', | |
| 'StringPrimary', | |
| 'ArithmeticPrimary', | |
| 'StringPrimary', |
|
Superseded by #380 |
I'm really missing the ability to create a range with non-default bounds, so here is a PR that should add it.
See https://www.postgresql.org/docs/17/rangetypes.html#RANGETYPES-CONSTRUCT for PostgreSQL details.
TL;DR: We can now pass the third argument to the range functions.
Summary by CodeRabbit
Documentation
New Features
Tests