-
-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Doctrine\ORM\Query\AST\Literal; | ||
| use Doctrine\ORM\Query\AST\Node; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTruncFieldException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\TimezoneValidationTrait; | ||
|
|
||
| /** | ||
| * Implementation of PostgreSQL DATE_TRUNC(). | ||
| * | ||
| * @see https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC | ||
| * @since 3.7 | ||
| * | ||
| * @author Jan Klan <jan@klan.com.au> | ||
| * | ||
| * @example Using it in DQL: "SELECT DATE_TRUNC('day', e.timestampWithTz, 'Australia/Adelaide') FROM Entity e" | ||
| */ | ||
| class DateTrunc extends BaseVariadicFunction | ||
| { | ||
| use TimezoneValidationTrait; | ||
| /** | ||
| * @see https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC | ||
| */ | ||
| public const FIELDS = [ | ||
| 'microseconds', | ||
| 'milliseconds', | ||
| 'second', | ||
| 'minute', | ||
| 'hour', | ||
| 'day', | ||
| 'week', | ||
| 'month', | ||
| 'quarter', | ||
| 'year', | ||
| 'decade', | ||
| 'century', | ||
| 'millennium', | ||
| ]; | ||
|
|
||
| protected function getNodeMappingPattern(): array | ||
| { | ||
| return ['StringPrimary']; | ||
| } | ||
|
|
||
| protected function getFunctionName(): string | ||
| { | ||
| return 'date_trunc'; | ||
| } | ||
|
|
||
| protected function getMinArgumentCount(): int | ||
| { | ||
| return 2; | ||
| } | ||
|
|
||
| protected function getMaxArgumentCount(): int | ||
| { | ||
| return 3; | ||
| } | ||
|
|
||
| protected function validateArguments(Node ...$arguments): void | ||
| { | ||
| parent::validateArguments(...$arguments); | ||
|
|
||
| $this->validateTruncField($arguments[0]); | ||
|
|
||
| // Validate that the third parameter is a valid timezone if provided | ||
| if (\count($arguments) === 3) { | ||
| $this->validateTimezone($arguments[2], $this->getFunctionName()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validates that the given node represents a valid trunc field value. | ||
| * | ||
| * @throws InvalidTruncFieldException If the field value is invalid | ||
| */ | ||
| protected function validateTruncField(Node $node): void | ||
| { | ||
| if (!$node instanceof Literal || !\is_string($node->value)) { | ||
| throw InvalidTruncFieldException::forNonLiteralNode($node::class, $this->getFunctionName()); | ||
| } | ||
|
|
||
| $field = \strtolower(\trim((string) $node->value, "'\"")); | ||
|
|
||
| if (!\in_array($field, self::FIELDS, true)) { | ||
| throw InvalidTruncFieldException::forInvalidField($field, $this->getFunctionName()); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not let PostgreSQL decide whether this is fine? Excessive validation may limit support for future PostgreSQL versions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personal preference of code that fails fast - this will fail before any SQL query gets executed.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand your personal preference. |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateTrunc; | ||
|
|
||
| /** | ||
| * @since 3.7 | ||
| * | ||
| * @author Jan Klan <jan@klan.com.au> | ||
| */ | ||
| class InvalidTruncFieldException extends ConversionException | ||
| { | ||
| public static function forNonLiteralNode(string $nodeClass, string $functionName): self | ||
| { | ||
| return new self(\sprintf( | ||
| 'The date_trunc field parameter for %s must be a string literal, got %s', | ||
| $functionName, | ||
| $nodeClass | ||
| )); | ||
| } | ||
|
|
||
| public static function forInvalidField(string $field, string $functionName): self | ||
| { | ||
| return new self(\sprintf( | ||
| 'Invalid field value "%s" provided for %s. Must be one of: %s.', | ||
| $field, | ||
| $functionName, | ||
| \implode(', ', DateTrunc::FIELDS) | ||
| )); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Tests\Unit\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsDates; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\BaseVariadicFunction; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateTrunc; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTimezoneException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTruncFieldException; | ||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
| use PHPUnit\Framework\Attributes\Test; | ||
|
|
||
| class DateTruncTest extends BaseVariadicFunctionTestCase | ||
| { | ||
| protected function createFixture(): BaseVariadicFunction | ||
| { | ||
| return new DateTrunc('DATE_TRUNC'); | ||
| } | ||
|
|
||
| protected function getStringFunctions(): array | ||
| { | ||
| return [ | ||
| 'DATE_TRUNC' => DateTrunc::class, | ||
| ]; | ||
| } | ||
|
|
||
| protected function getExpectedSqlStatements(): array | ||
| { | ||
| return [ | ||
| 'with timezone (3 arguments)' => /* @lang PostgreSQL */ "SELECT date_trunc('day', c0_.datetimetz1, 'Australia/Adelaide') AS sclr_0 FROM ContainsDates c0_", | ||
| 'without timezone (2 arguments)' => /* @lang PostgreSQL */ "SELECT date_trunc('day', c0_.datetimetz1) AS sclr_0 FROM ContainsDates c0_", | ||
| 'used in WHERE clause' => /* @lang PostgreSQL */ "SELECT c0_.datetimetz1 AS datetimetz1_0 FROM ContainsDates c0_ WHERE date_trunc('day', c0_.datetimetz1) = '2023-01-02 00:00:00'", | ||
| ]; | ||
| } | ||
|
|
||
| protected function getDqlStatements(): array | ||
| { | ||
| return [ | ||
| 'with timezone (3 arguments)' => \sprintf("SELECT DATE_TRUNC('day', e.datetimetz1, 'Australia/Adelaide') FROM %s e", ContainsDates::class), | ||
| 'without timezone (2 arguments)' => \sprintf("SELECT DATE_TRUNC('day', e.datetimetz1) FROM %s e", ContainsDates::class), | ||
| 'used in WHERE clause' => \sprintf("SELECT e.datetimetz1 FROM %s e WHERE DATE_TRUNC('day', e.datetimetz1) = '2023-01-02 00:00:00'", ContainsDates::class), | ||
| ]; | ||
| } | ||
|
|
||
| #[DataProvider('provideInvalidArgumentCountCases')] | ||
| #[Test] | ||
| public function throws_exception_for_invalid_argument_count(string $dql, string $expectedMessage): void | ||
| { | ||
| $this->expectException(InvalidArgumentForVariadicFunctionException::class); | ||
| $this->expectExceptionMessage($expectedMessage); | ||
|
|
||
| $this->buildEntityManager()->createQuery($dql)->getSQL(); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, array{string, string}> | ||
| */ | ||
| public static function provideInvalidArgumentCountCases(): array | ||
| { | ||
| return [ | ||
| 'too few arguments' => [ | ||
| \sprintf("SELECT DATE_TRUNC('day') FROM %s e", ContainsDates::class), | ||
| 'date_trunc() requires at least 2 arguments', | ||
| ], | ||
| 'too many arguments' => [ | ||
| \sprintf("SELECT DATE_TRUNC('day', e.datetimetz1, 'Australia/Adelaide', 'extra_arg') FROM %s e", ContainsDates::class), | ||
| 'date_trunc() requires between 2 and 3 arguments', | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| #[DataProvider('provideValidFieldValues')] | ||
| #[Test] | ||
| public function accepts_valid_field_value(string $validField): void | ||
| { | ||
| $dql = \sprintf("SELECT DATE_TRUNC('%s', e.datetimetz1) FROM %s e", $validField, ContainsDates::class); | ||
|
|
||
| $this->assertEquals( | ||
| \sprintf("SELECT date_trunc('%s', c0_.datetimetz1) AS sclr_0 FROM ContainsDates c0_", $validField), | ||
| $this->buildEntityManager()->createQuery($dql)->getSQL() | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @return \Generator<string, array{string}> | ||
| */ | ||
| public static function provideValidFieldValues(): \Generator | ||
| { | ||
| foreach (DateTrunc::FIELDS as $field) { | ||
| yield $field => [$field]; | ||
| } | ||
| } | ||
|
|
||
| #[DataProvider('provideInvalidFieldValues')] | ||
| #[Test] | ||
| public function throws_exception_for_invalid_field(string $invalidField): void | ||
| { | ||
| $this->expectException(InvalidTruncFieldException::class); | ||
| $this->expectExceptionMessage(\sprintf('Invalid field value "%s" provided for date_trunc. Must be one of: %s.', $invalidField, \implode(', ', DateTrunc::FIELDS))); | ||
|
|
||
| $dql = \sprintf("SELECT DATE_TRUNC('%s', e.datetimetz1) FROM %s e", $invalidField, ContainsDates::class); | ||
| $this->buildEntityManager()->createQuery($dql)->getSQL(); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, array{string}> | ||
| */ | ||
| public static function provideInvalidFieldValues(): array | ||
| { | ||
| return [ | ||
| 'empty string' => [''], | ||
| 'whitespace only' => [' '], | ||
| 'numeric value' => ['123'], | ||
| 'invalid field' => ['invalid'], | ||
| ]; | ||
| } | ||
|
|
||
| #[DataProvider('provideInvalidTimezoneValues')] | ||
| #[Test] | ||
| public function throws_exception_for_invalid_timezone(string $invalidTimezone): void | ||
| { | ||
| $this->expectException(InvalidTimezoneException::class); | ||
| $this->expectExceptionMessage(\sprintf('Invalid timezone "%s" provided for date_trunc. Must be a valid PHP timezone identifier.', $invalidTimezone)); | ||
|
|
||
| $dql = \sprintf("SELECT DATE_TRUNC('day', e.datetimetz1, '%s') FROM %s e", $invalidTimezone, ContainsDates::class); | ||
| $this->buildEntityManager()->createQuery($dql)->getSQL(); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, array{string}> | ||
| */ | ||
| public static function provideInvalidTimezoneValues(): array | ||
| { | ||
| return [ | ||
| 'empty string' => [''], | ||
| 'whitespace only' => [' '], | ||
| 'numeric value' => ['123'], | ||
| 'invalid timezone' => ['Invalid/Timezone'], | ||
| ]; | ||
| } | ||
| } |
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.
Can you elaborate on why this is a public constant? There is no such pattern in any other classes in the repository. On the surface, it is needed for tests primarily.
Uh oh!
There was an error while loading. Please reload this page.
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.
I wanted to both (a) enumerate the allowed options in InvalidTruncFieldException message and (b) have the list of valid options available for input validation.
It's in a public constant because it will likely never change (hence not a static variable), it's exposed as a class constant because it's needed in two separate classes, and it's not hardcoded as two separate arrays in each class because if it does change, someone will have to remember changing it in both places.
At the end of the day if you think it's better as two separate hardcoded arrays (because it's unlikely to change), let's do it that way for sake of overall consistency.
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.
I prefer PostgreSQL to throw its native error message and wrap it, rather than trying to validate lower-level PostgreSQL behaviour.