-
-
Notifications
You must be signed in to change notification settings - Fork 56
feat: add support for DATE_TRUNC
#493
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
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@janklan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (9)
WalkthroughAdds a new PostgreSQL-style DQL function DATE_TRUNC (variadic, 2–3 args) with truncation-field literal validation and optional timezone validation, a new InvalidTruncFieldException, documentation updates for registration and usage, plus unit and integration tests covering SQL generation and validation paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/USE-CASES-AND-EXAMPLES.md (1)
100-111: Fix DATE_TRUNC example with four arguments and invalid timezoneThe second DATE_TRUNC example,
SELECT DATE_TRUNC('day', e.timestampWithTz, '2 hours', 'UTC') FROM Entity edoes not match the implemented function:
DateTrunconly allows 2 or 3 arguments (field,source, optionaltime_zone).- If used as written, it will trigger
InvalidArgumentForVariadicFunctionExceptiondue to too many arguments.'2 hours'is not a valid timezone string for the current timezone validation.Consider updating to a 3‑argument example with a valid timezone, e.g.:
-SELECT DATE_TRUNC('day', e.timestampWithTz, '2 hours', 'UTC') FROM Entity e +SELECT DATE_TRUNC('day', e.timestampWithTz, 'UTC') FROM Entity eThis keeps the “timezone parameter is optional” wording accurate and matches the actual API.
🧹 Nitpick comments (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
15-34: Exception wiring is fine; consider renaming parameter for clarity
InvalidTruncFieldExceptioncorrectly provides:
forNonLiteralNode()for non-literal field arguments.forInvalidField()including the list of allowedDateTrunc::FIELDS.Only minor nit: the first parameter of
forInvalidField()is named$timezonebut represents the truncation field value. Renaming to$fieldwould better reflect its purpose without changing behavior:- public static function forInvalidField(string $timezone, string $functionName): self + public static function forInvalidField(string $field, string $functionName): self { return new self(\sprintf( 'Invalid date_trunc field value "%s" provided for %s. Must be one of: %s.', - $timezone, + $field, $functionName, \implode(', ', DateTrunc::FIELDS) )); }tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php (1)
16-143: DateTrunc tests are solid; consider adding a non‑literal field caseThe test suite nicely covers:
- DQL → SQL translation for 2‑ and 3‑argument usages and WHERE clause.
- Argument count validation via
InvalidArgumentForVariadicFunctionException.- Field value validation against
DateTrunc::FIELDS.- Timezone validation via
InvalidTimezoneException.To fully exercise
InvalidTruncFieldException::forNonLiteralNode(), you could add a case where the first argument isn’t a string literal (e.g. using a field expression as the truncation unit). For example:public function test_throws_exception_for_non_literal_field(): void { $this->expectException(InvalidTruncFieldException::class); $this->expectExceptionMessage( 'The date_trunc field parameter for date_trunc must be a string literal' ); $dql = \sprintf("SELECT DATE_TRUNC(e.datetimetz1, e.datetimetz1) FROM %s e", ContainsDates::class); $this->buildEntityManager()->createQuery($dql)->getSQL(); }(adjust the exact message assertion once DateTrunc uses
InvalidTruncFieldException::forNonLiteralNode()).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/DATE-AND-RANGE-FUNCTIONS.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(1 hunks)docs/INTEGRATING-WITH-LARAVEL.md(1 hunks)docs/INTEGRATING-WITH-SYMFONY.md(1 hunks)docs/USE-CASES-AND-EXAMPLES.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: PostgreSQL's to_timestamp function returns a fallback result (like '2005-01-01 00:00:00+00') when given an invalid format string, rather than throwing an exception. This behavior is different from some other formatting functions and should be tested accordingly.
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/DATE-AND-RANGE-FUNCTIONS.mddocs/INTEGRATING-WITH-DOCTRINE.mddocs/INTEGRATING-WITH-LARAVEL.mdtests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/DATE-AND-RANGE-FUNCTIONS.mddocs/INTEGRATING-WITH-DOCTRINE.mddocs/INTEGRATING-WITH-LARAVEL.mdtests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 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.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpdocs/INTEGRATING-WITH-SYMFONY.mddocs/INTEGRATING-WITH-LARAVEL.md
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-04-22T00:03:37.733Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
🧬 Code graph analysis (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTimezoneException.php (1)
InvalidTimezoneException(14-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (3)
InvalidTruncFieldException(15-35)forNonLiteralNode(17-24)forInvalidField(26-34)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/TimezoneValidationTrait.php (1)
validateTimezone(25-36)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php (5)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
DateTrunc(23-94)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
InvalidArgumentForVariadicFunctionException(7-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTimezoneException.php (1)
InvalidTimezoneException(14-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
InvalidTruncFieldException(15-35)
🔇 Additional comments (4)
docs/INTEGRATING-WITH-DOCTRINE.md (1)
158-165: DATE_TRUNC registration for Doctrine configuration looks correctThe DATE_TRUNC mapping to
MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateTrunc::classis consistent with the new function and aligned with existing date-specific registrations.docs/DATE-AND-RANGE-FUNCTIONS.md (1)
9-18: DATE_TRUNC entry in date/time function table is consistentThe new
date_trunc→DATE_TRUNC→DateTruncrow matches the implementation and follows the existing documentation pattern.docs/INTEGRATING-WITH-SYMFONY.md (1)
226-233: Symfony DQL registration for DATE_TRUNC is correctThe new
DATE_TRUNCentry understring_functionscorrectly points toMartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateTruncand is consistent with the rest of the date-specific configuration.docs/INTEGRATING-WITH-LARAVEL.md (1)
243-250: Laravel custom_string_functions mapping for DATE_TRUNC is consistentThe
'DATE_TRUNC' => DateTrunc::classentry fits the existing pattern for date-specific functions and matches the new DateTrunc implementation.
4822f49 to
3da060e
Compare
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 (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
26-33: Clarify parameter naming in forInvalidField()
forInvalidField()currently uses$timezoneas the parameter name, even though it represents the invalid truncation field. The behavior is correct but the name is confusing when reading the code next toDateTrunc::FIELDS.Consider renaming the parameter to better reflect its meaning, for example:
- public static function forInvalidField(string $timezone, string $functionName): self + public static function forInvalidField(string $field, string $functionName): self { return new self(\sprintf( 'Invalid date_trunc field value "%s" provided for %s. Must be one of: %s.', - $timezone, + $field, $functionName, \implode(', ', DateTrunc::FIELDS) )); }If you prefer to avoid changing the parameter name for potential named-argument callers, you could keep
$timezonein the signature but assign it to a local$fieldvariable and use that in the message.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/DATE-AND-RANGE-FUNCTIONS.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(1 hunks)docs/INTEGRATING-WITH-LARAVEL.md(1 hunks)docs/INTEGRATING-WITH-SYMFONY.md(1 hunks)docs/USE-CASES-AND-EXAMPLES.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/INTEGRATING-WITH-LARAVEL.md
- docs/DATE-AND-RANGE-FUNCTIONS.md
- tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
- docs/USE-CASES-AND-EXAMPLES.md
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: PostgreSQL's to_timestamp function returns a fallback result (like '2005-01-01 00:00:00+00') when given an invalid format string, rather than throwing an exception. This behavior is different from some other formatting functions and should be tested accordingly.
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
docs/INTEGRATING-WITH-DOCTRINE.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpdocs/INTEGRATING-WITH-SYMFONY.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
docs/INTEGRATING-WITH-DOCTRINE.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpdocs/INTEGRATING-WITH-SYMFONY.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 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.
Applied to files:
docs/INTEGRATING-WITH-SYMFONY.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
🧬 Code graph analysis (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
DateTrunc(23-94)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (4)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTimezoneException.php (1)
InvalidTimezoneException(14-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (3)
InvalidTruncFieldException(15-35)forNonLiteralNode(17-24)forInvalidField(26-34)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/TimezoneValidationTrait.php (1)
validateTimezone(25-36)
🪛 GitHub Actions: CI
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
[error] 1-1: PHP CS Fixer: no_unused_imports violation. Removed unused import 'InvalidTimezoneException'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.5
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
🔇 Additional comments (2)
docs/INTEGRATING-WITH-SYMFONY.md (1)
226-233: DATE_TRUNC registration in Symfony config is consistentThe new
DATE_TRUNCmapping aligns with existing date functions and correctly referencesDateTrunc::class.docs/INTEGRATING-WITH-DOCTRINE.md (1)
158-165: DATE_TRUNC registration in Doctrine config follows existing patternThe
DATE_TRUNCcustom string function is registered consistently with other date functions and points to the correctDateTruncclass.
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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/DATE-AND-RANGE-FUNCTIONS.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(1 hunks)docs/INTEGRATING-WITH-LARAVEL.md(1 hunks)docs/INTEGRATING-WITH-SYMFONY.md(1 hunks)docs/USE-CASES-AND-EXAMPLES.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/INTEGRATING-WITH-SYMFONY.md
- docs/USE-CASES-AND-EXAMPLES.md
- docs/INTEGRATING-WITH-LARAVEL.md
- docs/INTEGRATING-WITH-DOCTRINE.md
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: PostgreSQL's to_timestamp function returns a fallback result (like '2005-01-01 00:00:00+00') when given an invalid format string, rather than throwing an exception. This behavior is different from some other formatting functions and should be tested accordingly.
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
docs/DATE-AND-RANGE-FUNCTIONS.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
docs/DATE-AND-RANGE-FUNCTIONS.mdsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phptests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 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.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
📚 Learning: 2025-04-22T00:03:37.733Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Applied to files:
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
🧬 Code graph analysis (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (3)
InvalidTruncFieldException(15-35)forNonLiteralNode(17-24)forInvalidField(26-34)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/TimezoneValidationTrait.php (1)
validateTimezone(25-36)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
DateTrunc(22-93)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php (6)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
DateTrunc(22-93)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
InvalidArgumentForVariadicFunctionException(7-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTimezoneException.php (1)
InvalidTimezoneException(14-33)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
InvalidTruncFieldException(15-35)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TestCase.php (1)
buildEntityManager(115-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.5
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.4
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer latest
- GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
- GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (11)
docs/DATE-AND-RANGE-FUNCTIONS.md (1)
14-14: LGTM!The documentation entry correctly maps the PostgreSQL
date_truncfunction to theDATE_TRUNCDQL identifier and follows the established table format.tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php (6)
18-28: LGTM!The fixture setup and function mapping follow the established test patterns in this codebase.
30-46: LGTM!The test cases cover the key scenarios: with timezone (3 arguments), without timezone (2 arguments), and usage in WHERE clauses. The expected SQL correctly reflects PostgreSQL's
date_truncsyntax.
48-73: LGTM!The test correctly validates argument count boundaries and verifies appropriate exception messages for both under and over-specification of arguments.
75-95: LGTM!Using a data provider that iterates over
DateTrunc::FIELDSensures all valid truncation fields are tested, maintaining consistency with the implementation.
97-119: LGTM!The test coverage for invalid fields includes edge cases like empty strings, whitespace, and clearly invalid values, ensuring robust validation.
121-143: LGTM!The timezone validation tests cover similar edge cases and verify that the trait-based validation correctly rejects invalid timezone identifiers.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (4)
28-42: LGTM!The
FIELDSconstant correctly lists all valid PostgreSQL truncation units as documented in the PostgreSQL reference, providing a clear contract for validation.
44-62: LGTM!The function configuration correctly specifies:
StringPrimarynode mapping for literal string arguments- 2–3 argument range matching PostgreSQL's
date_trunc('field', source [, time_zone])signature
64-74: LGTM!The validation sequence is correct: parent validation for argument count, then field validation, and finally optional timezone validation when a third argument is present.
81-92: LGTM!The field validation logic correctly:
- Ensures the argument is a string literal
- Normalizes the field value by trimming quotes and converting to lowercase
- Validates against the
FIELDSconstant- Throws appropriate
InvalidTruncFieldExceptionfor invalid cases
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
Outdated
Show resolved
Hide resolved
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
Outdated
Show resolved
Hide resolved
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
Outdated
Show resolved
Hide resolved
DATE_TRUNC
martin-georgiev
left a comment
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.
Integration tests are missing.
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: 0
🧹 Nitpick comments (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
22-22: Align with Rector’s NewlineBetweenClassLikeStmtsRector to satisfy CI toolingCI reports a Rector
NewlineBetweenClassLikeStmtsRectorsuggestion for this class. It appears purely stylistic, but to keep tooling happy, please run the project’s Rector/CS fixer locally and commit the resulting diff so CI no longer warns about this file.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/DATE-AND-RANGE-FUNCTIONS.md(1 hunks)docs/INTEGRATING-WITH-DOCTRINE.md(1 hunks)docs/INTEGRATING-WITH-LARAVEL.md(1 hunks)docs/INTEGRATING-WITH-SYMFONY.md(1 hunks)docs/USE-CASES-AND-EXAMPLES.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php(1 hunks)tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php(1 hunks)tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/INTEGRATING-WITH-LARAVEL.md
- tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTruncTest.php
- docs/INTEGRATING-WITH-DOCTRINE.md
- docs/INTEGRATING-WITH-SYMFONY.md
- docs/USE-CASES-AND-EXAMPLES.md
- docs/DATE-AND-RANGE-FUNCTIONS.md
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: PostgreSQL's to_timestamp function returns a fallback result (like '2005-01-01 00:00:00+00') when given an invalid format string, rather than throwing an exception. This behavior is different from some other formatting functions and should be tested accordingly.
📚 Learning: 2025-08-09T15:31:42.054Z
Learnt from: landure
Repo: martin-georgiev/postgresql-for-doctrine PR: 411
File: src/MartinGeorgiev/Doctrine/DBAL/Types/Exceptions/InvalidLtreeForPHPException.php:11-19
Timestamp: 2025-08-09T15:31:42.054Z
Learning: In the postgresql-for-doctrine library, exception classes for invalid PHP values (Invalid*ForPHPException) follow a consistent pattern using var_export() in their create() method to format error messages. When adding new exception classes, maintain consistency with existing patterns rather than introducing new approaches, even if technically superior alternatives exist.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-03-29T03:31:17.114Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 318
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/XmlAggTest.php:1-9
Timestamp: 2025-03-29T03:31:17.114Z
Learning: Tests in the `Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase, and therefore don't need an explicit import.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-05-23T11:11:57.951Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 383
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RadiansTest.php:1-9
Timestamp: 2025-05-23T11:11:57.951Z
Learning: Tests in the `Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions` namespace extend a custom `TestCase` class from the same namespace (`Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\TestCase`), rather than PHPUnit's TestCase directly, and therefore don't need an explicit import.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: The repository should separate value object validation exceptions (extending \InvalidArgumentException) from DBAL conversion exceptions (extending ConversionException) - the Ltree implementation demonstrates this ideal pattern with InvalidLtreeException for validation and InvalidLtreeForPHPException/InvalidLtreeForDatabaseException for conversion errors.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-25T21:29:08.737Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-08-25T21:29:08.737Z
Learning: Value object validation in this repository should use dedicated exception classes extending \InvalidArgumentException (like InvalidLtreeException, InvalidPointException) with clear documentation separating validation concerns from DBAL conversion concerns, and should include protected static assertion methods for validation logic.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php
📚 Learning: 2025-04-18T10:33:52.412Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 350
File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php:78-88
Timestamp: 2025-04-18T10:33:52.412Z
Learning: In SQL generation code for the postgresql-for-doctrine library, it's better to fail explicitly with clear errors than to use fallback behavior that makes assumptions about node types, as this could lead to functional bugs or security issues.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.phpsrc/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-09-01T18:48:28.508Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 434
File: tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/PostGIS/ST_CrossesTest.php:12-31
Timestamp: 2025-09-01T18:48:28.508Z
Learning: When analyzing unit test files in this codebase, always verify the actual file structure and existing patterns before suggesting changes. The ContainsGeometries fixture exists at ./fixtures/MartinGeorgiev/Doctrine/Entity/ContainsGeometries.php and the PostGIS unit tests use the namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\PostGIS consistently.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-05-27T16:10:35.054Z
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: When reviewing PostgreSQL formatting function tests, the error handling behavior should match PostgreSQL's actual implementation rather than assuming consistency across all formatting functions, as different functions may handle invalid inputs differently.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-04-22T00:03:37.733Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 357
File: tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php:28-62
Timestamp: 2025-04-22T00:03:37.733Z
Learning: This project focuses on providing Doctrine ORM interfaces to PostgreSQL functions. Tests should validate correct DQL-to-SQL translation rather than PostgreSQL functionality itself. Test cases should focus on parameter passing and SQL generation, not on testing specific PostgreSQL regex pattern behaviors.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-12-05T19:04:28.955Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 0
File: :0-0
Timestamp: 2025-12-05T19:04:28.955Z
Learning: In the postgresql-for-doctrine project, avoid suggesting unusual PHP version conditional checks (e.g., if PHP_VERSION_ID >= 80400) in test setup code when the root cause is a dependency version constraint issue. The proper solution is to update dependency constraints in composer.json, not add workarounds in the codebase.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-05-27T16:10:35.054Z
Learnt from: karpilin
Repo: martin-georgiev/postgresql-for-doctrine PR: 386
File: tests/Integration/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/ToTimestampTest.php:34-39
Timestamp: 2025-05-27T16:10:35.054Z
Learning: PostgreSQL's to_timestamp function returns a fallback result (like '2005-01-01 00:00:00+00') when given an invalid format string, rather than throwing an exception. This behavior is different from some other formatting functions and should be tested accordingly.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what TYPE_NAME constants contain and getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the point[] array type should be documented as "point[]" not "_point" in the AVAILABLE-TYPES.md table, to be consistent with all other array types like text[], jsonb[], inet[], etc.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-08-19T13:07:15.184Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 421
File: docs/AVAILABLE-TYPES.md:31-33
Timestamp: 2025-08-19T13:07:15.184Z
Learning: In martin-georgiev/postgresql-for-doctrine, the AVAILABLE-TYPES.md documentation table's second column shows DBAL type names (what getName() method returns) not PostgreSQL internal catalogue names. Array types use the format like 'text[]', 'jsonb[]', 'inet[]' - not the underscore-prefixed PostgreSQL internal names like '_text', '_jsonb', '_inet'.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-09-06T22:15:32.757Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 443
File: tests/Unit/MartinGeorgiev/Utils/PostgresArrayToPHPArrayTransformerTest.php:169-172
Timestamp: 2025-09-06T22:15:32.757Z
Learning: In martin-georgiev/postgresql-for-doctrine, TextArray users should expect the database to normalize data to strings only. The design assumes that intended PHP data for TextArray is always strings, not other data types, which is a reasonable tradeoff that aligns with PostgreSQL text[] type semantics.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
📚 Learning: 2025-03-11T12:32:10.726Z
Learnt from: martin-georgiev
Repo: martin-georgiev/postgresql-for-doctrine PR: 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.
Applied to files:
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
🧬 Code graph analysis (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
DateTrunc(22-93)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (3)
InvalidTruncFieldException(15-35)forNonLiteralNode(17-24)forInvalidField(26-34)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-143)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/TimezoneValidationTrait.php (1)
validateTimezone(25-36)
🪛 GitHub Actions: CI
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php
[warning] 22-22: Rector would apply NewlineBetweenClassLikeStmtsRector: code style change suggested (DateTrunc class).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.4
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.3
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.3
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.4
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.3
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.2
- GitHub Check: PostgreSQL 16 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.5 + PHP 8.1
- GitHub Check: PostgreSQL 18 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.2
- GitHub Check: PostgreSQL 17 + PostGIS 3.6 + PHP 8.1
- GitHub Check: PostgreSQL 17 + PostGIS 3.4 + PHP 8.1
- GitHub Check: PostgreSQL 16 + PostGIS 3.4 + PHP 8.1
- GitHub Check: wait-for-tests-worflows-before-upload
🔇 Additional comments (2)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateTrunc.php (1)
24-74: DATE_TRUNC argument validation and timezone handling look correctMin/max argument counts (2–3), first‑argument field validation against
FIELDS, and optional third‑argument timezone validation viaTimezoneValidationTraitare wired correctly on top ofBaseVariadicFunction::validateArguments. Indexing into$arguments[0]and$arguments[2]is safe given the enforced bounds, and the behaviour matches PostgreSQL’sdate_trunc('field', source [, time_zone])signature.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTruncFieldException.php (1)
15-34: InvalidTruncFieldException wiring and messages are consistent and usefulUsing
ConversionExceptionas a base plus the two focused factories for non‑literal nodes and invalid field values (with the allowed set pulled fromDateTrunc::FIELDS) matches the existing error‑handling style in this library and should give clear diagnostics when DQL is mis‑shaped.
I added integration tests. FYI, they were/are missing for some other date_* functions. |
The subject says it all.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.