-
-
Notifications
You must be signed in to change notification settings - Fork 56
feat: add support for DATE_ADD(), DATE_SUBTRACT() and DATE_BIN()
#345
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
da67d14
refactor: stricter method argument types when handling variadic funct…
martin-georgiev e5e50b3
no message
martin-georgiev dbb2677
feat: add support for `DATE_ADD()`, `DATE_SUBTRACT()` and `DATE_BIN()`
martin-georgiev f9ebc62
Merge branch 'main' into new-funcs
martin-georgiev c456dfe
more tests
martin-georgiev 1940bd2
Merge branch 'new-funcs' of https://github.com/martin-georgiev/postgr…
martin-georgiev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
45 changes: 45 additions & 0 deletions
45
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Doctrine\ORM\Query\AST\Node; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\TimezoneValidationTrait; | ||
|
|
||
| /** | ||
| * Implementation of PostgreSQL DATE_ADD(). | ||
| * | ||
| * Adds an interval to a timestamp with time zone, computing times of day and daylight-savings | ||
| * adjustments according to the time zone. | ||
| * | ||
| * @see https://www.postgresql.org/docs/16/functions-datetime.html | ||
| * @since 3.1 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| * | ||
| * @example Using it in DQL: "SELECT DATE_ADD(e.timestampWithTz, '1 day', 'Europe/Sofia') FROM Entity e" | ||
| */ | ||
| class DateAdd extends BaseVariadicFunction | ||
| { | ||
| use TimezoneValidationTrait; | ||
|
|
||
| protected function customizeFunction(): void | ||
| { | ||
| $this->setFunctionPrototype('date_add(%s)'); | ||
| } | ||
|
|
||
| protected function validateArguments(Node ...$arguments): void | ||
| { | ||
| $argumentCount = \count($arguments); | ||
| if ($argumentCount < 2 || $argumentCount > 3) { | ||
| throw InvalidArgumentForVariadicFunctionException::between('date_add', 2, 3); | ||
| } | ||
|
|
||
| // Validate that the third parameter is a valid timezone if provided | ||
| if ($argumentCount === 3) { | ||
| $this->validateTimezone($arguments[2], 'DATE_ADD'); | ||
| } | ||
| } | ||
| } |
28 changes: 28 additions & 0 deletions
28
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateBin.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| /** | ||
| * Implementation of PostgreSQL DATE_BIN(). | ||
| * | ||
| * Bins input into specified interval aligned with specified origin. | ||
| * | ||
| * @see https://www.postgresql.org/docs/14/functions-datetime.html | ||
| * @since 3.1 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| * | ||
| * @example Using it in DQL: "SELECT DATE_BIN('15 minutes', e.createdAt, '2001-02-16 20:05:00') FROM Entity e" | ||
| */ | ||
| class DateBin extends BaseFunction | ||
| { | ||
| protected function customizeFunction(): void | ||
| { | ||
| $this->setFunctionPrototype('date_bin(%s, %s, %s)'); | ||
| $this->addNodeMapping('StringPrimary'); | ||
| $this->addNodeMapping('StringPrimary'); | ||
| $this->addNodeMapping('StringPrimary'); | ||
| } | ||
| } |
45 changes: 45 additions & 0 deletions
45
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtract.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Doctrine\ORM\Query\AST\Node; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidArgumentForVariadicFunctionException; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits\TimezoneValidationTrait; | ||
|
|
||
| /** | ||
| * Implementation of PostgreSQL DATE_SUBTRACT(). | ||
| * | ||
| * Subtracts an interval from a timestamp with time zone, computing times of day and daylight-savings | ||
| * adjustments according to the time zone. | ||
| * | ||
| * @see https://www.postgresql.org/docs/16/functions-datetime.html | ||
| * @since 3.1 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| * | ||
| * @example Using it in DQL: "SELECT DATE_SUBTRACT(e.timestampWithTz, '1 day', 'Europe/Sofia') FROM Entity e" | ||
| */ | ||
| class DateSubtract extends BaseVariadicFunction | ||
| { | ||
| use TimezoneValidationTrait; | ||
|
|
||
| protected function customizeFunction(): void | ||
| { | ||
| $this->setFunctionPrototype('date_subtract(%s)'); | ||
| } | ||
|
|
||
| protected function validateArguments(Node ...$arguments): void | ||
| { | ||
| $argumentCount = \count($arguments); | ||
| if ($argumentCount < 2 || $argumentCount > 3) { | ||
| throw InvalidArgumentForVariadicFunctionException::between('date_subtract', 2, 3); | ||
| } | ||
|
|
||
| // Validate that the third parameter is a valid timezone if provided | ||
| if ($argumentCount === 3) { | ||
| $this->validateTimezone($arguments[2], 'DATE_SUBTRACT'); | ||
| } | ||
| } | ||
| } |
33 changes: 33 additions & 0 deletions
33
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidTimezoneException.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
|
|
||
| /** | ||
| * @since 3.1 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| class InvalidTimezoneException extends ConversionException | ||
| { | ||
| public static function forNonLiteralNode(string $nodeClass, string $functionName): self | ||
| { | ||
| return new self(\sprintf( | ||
| 'The timezone parameter for %s must be a string literal, got %s', | ||
| $functionName, | ||
| $nodeClass | ||
| )); | ||
| } | ||
|
|
||
| public static function forInvalidTimezone(string $timezone, string $functionName): self | ||
| { | ||
| return new self(\sprintf( | ||
| 'Invalid timezone "%s" provided for %s. Must be a valid PHP timezone identifier.', | ||
| $timezone, | ||
| $functionName | ||
| )); | ||
| } | ||
| } |
48 changes: 48 additions & 0 deletions
48
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Traits/TimezoneValidationTrait.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Traits; | ||
|
|
||
| use Doctrine\ORM\Query\AST\Literal; | ||
| use Doctrine\ORM\Query\AST\Node; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTimezoneException; | ||
|
|
||
| /** | ||
| * Provides timezone validation functionality for functions that use valid PHP timezones. | ||
| * | ||
| * @since 3.1 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| trait TimezoneValidationTrait | ||
| { | ||
| /** | ||
| * Validates that the given node represents a valid PHP timezone. | ||
| * | ||
| * @throws InvalidTimezoneException If the timezone is invalid | ||
| */ | ||
| protected function validateTimezone(Node $node, string $functionName): void | ||
| { | ||
| if (!$node instanceof Literal || !\is_string($node->value)) { | ||
| throw InvalidTimezoneException::forNonLiteralNode($node::class, $functionName); | ||
| } | ||
|
|
||
| $timezone = \trim((string) $node->value, "'\""); | ||
|
|
||
| if (!$this->isValidTimezone($timezone)) { | ||
| throw InvalidTimezoneException::forInvalidTimezone($timezone, $functionName); | ||
| } | ||
| } | ||
|
|
||
| private function isValidTimezone(string $timezone): bool | ||
| { | ||
| try { | ||
| new \DateTimeZone($timezone); | ||
|
|
||
| return true; | ||
| } catch (\Exception) { | ||
| return false; | ||
| } | ||
| } | ||
| } |
46 changes: 46 additions & 0 deletions
46
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAddTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsDates; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateAdd; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTimezoneException; | ||
|
|
||
| class DateAddTest extends TestCase | ||
| { | ||
| protected function getStringFunctions(): array | ||
| { | ||
| return [ | ||
| 'DATE_ADD' => DateAdd::class, | ||
| ]; | ||
| } | ||
|
|
||
| protected function getExpectedSqlStatements(): array | ||
| { | ||
| return [ | ||
| 'adds 1 day with timezone' => "SELECT date_add(c0_.datetimetz1, '1 day', 'Europe/Sofia') AS sclr_0 FROM ContainsDates c0_", | ||
| 'adds 2 hours with timezone' => "SELECT date_add(c0_.datetimetz1, '2 hours', 'UTC') AS sclr_0 FROM ContainsDates c0_", | ||
| 'adds 3 days without timezone' => "SELECT date_add(c0_.datetimetz1, '3 days') AS sclr_0 FROM ContainsDates c0_", | ||
| ]; | ||
| } | ||
|
|
||
| protected function getDqlStatements(): array | ||
| { | ||
| return [ | ||
| 'adds 1 day with timezone' => \sprintf("SELECT DATE_ADD(e.datetimetz1, '1 day', 'Europe/Sofia') FROM %s e", ContainsDates::class), | ||
| 'adds 2 hours with timezone' => \sprintf("SELECT DATE_ADD(e.datetimetz1, '2 hours', 'UTC') FROM %s e", ContainsDates::class), | ||
| 'adds 3 days without timezone' => \sprintf("SELECT DATE_ADD(e.datetimetz1, '3 days') FROM %s e", ContainsDates::class), | ||
| ]; | ||
| } | ||
|
|
||
| public function test_invalid_timezone_throws_exception(): void | ||
| { | ||
| $this->expectException(InvalidTimezoneException::class); | ||
| $this->expectExceptionMessage('Invalid timezone "Invalid/Timezone" provided for DATE_ADD'); | ||
|
|
||
| $dql = \sprintf("SELECT DATE_ADD(e.datetimetz1, '1 day', 'Invalid/Timezone') FROM %s e", ContainsDates::class); | ||
| $this->buildEntityManager()->createQuery($dql)->getSQL(); | ||
| } | ||
| } |
34 changes: 34 additions & 0 deletions
34
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateBinTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsDates; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateBin; | ||
|
|
||
| class DateBinTest extends TestCase | ||
| { | ||
| protected function getStringFunctions(): array | ||
| { | ||
| return [ | ||
| 'DATE_BIN' => DateBin::class, | ||
| ]; | ||
| } | ||
|
|
||
| protected function getExpectedSqlStatements(): array | ||
| { | ||
| return [ | ||
| 'bins by 15 minutes' => "SELECT date_bin('15 minutes', c0_.datetime1, '2001-02-16 20:05:00') AS sclr_0 FROM ContainsDates c0_", | ||
| 'bins by 1 hour' => "SELECT date_bin('1 hour', c0_.datetime1, '2001-02-16 00:00:00') AS sclr_0 FROM ContainsDates c0_", | ||
| ]; | ||
| } | ||
|
|
||
| protected function getDqlStatements(): array | ||
| { | ||
| return [ | ||
| 'bins by 15 minutes' => \sprintf("SELECT DATE_BIN('15 minutes', e.datetime1, '2001-02-16 20:05:00') FROM %s e", ContainsDates::class), | ||
| 'bins by 1 hour' => \sprintf("SELECT DATE_BIN('1 hour', e.datetime1, '2001-02-16 00:00:00') FROM %s e", ContainsDates::class), | ||
| ]; | ||
| } | ||
| } | ||
46 changes: 46 additions & 0 deletions
46
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateSubtractTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Tests\MartinGeorgiev\Doctrine\ORM\Query\AST\Functions; | ||
|
|
||
| use Fixtures\MartinGeorgiev\Doctrine\Entity\ContainsDates; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DateSubtract; | ||
| use MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Exception\InvalidTimezoneException; | ||
|
|
||
| class DateSubtractTest extends TestCase | ||
| { | ||
| protected function getStringFunctions(): array | ||
| { | ||
| return [ | ||
| 'DATE_SUBTRACT' => DateSubtract::class, | ||
| ]; | ||
| } | ||
|
|
||
| protected function getExpectedSqlStatements(): array | ||
| { | ||
| return [ | ||
| 'subtracts 1 day with timezone' => "SELECT date_subtract(c0_.datetimetz1, '1 day', 'Europe/Sofia') AS sclr_0 FROM ContainsDates c0_", | ||
| 'subtracts 2 hours with timezone' => "SELECT date_subtract(c0_.datetimetz1, '2 hours', 'UTC') AS sclr_0 FROM ContainsDates c0_", | ||
| 'subtracts 3 days without timezone' => "SELECT date_subtract(c0_.datetimetz1, '3 days') AS sclr_0 FROM ContainsDates c0_", | ||
| ]; | ||
| } | ||
|
|
||
| protected function getDqlStatements(): array | ||
| { | ||
| return [ | ||
| 'subtracts 1 day with timezone' => \sprintf("SELECT DATE_SUBTRACT(e.datetimetz1, '1 day', 'Europe/Sofia') FROM %s e", ContainsDates::class), | ||
| 'subtracts 2 hours with timezone' => \sprintf("SELECT DATE_SUBTRACT(e.datetimetz1, '2 hours', 'UTC') FROM %s e", ContainsDates::class), | ||
| 'subtracts 3 days without timezone' => \sprintf("SELECT DATE_SUBTRACT(e.datetimetz1, '3 days') FROM %s e", ContainsDates::class), | ||
| ]; | ||
| } | ||
|
|
||
| public function test_invalid_timezone_throws_exception(): void | ||
| { | ||
| $this->expectException(InvalidTimezoneException::class); | ||
| $this->expectExceptionMessage('Invalid timezone "Invalid/Timezone" provided for DATE_SUBTRACT'); | ||
|
|
||
| $dql = \sprintf("SELECT DATE_SUBTRACT(e.datetimetz1, '1 day', 'Invalid/Timezone') FROM %s e", ContainsDates::class); | ||
| $this->buildEntityManager()->createQuery($dql)->getSQL(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🛠️ Refactor suggestion
Test coverage should include additional use cases based on PostgreSQL documentation.
The test covers basic functionality for the
DATE_BIN()function with two test cases (15 minutes and 1 hour binning), but would benefit from more comprehensive coverage based on PostgreSQL documentation.Consider adding the following test cases to ensure comprehensive coverage:
According to the PostgreSQL documentation,
date_bin()has specific behavior when changing the origin parameter, which affects where the bins are anchored. Adding these test cases would ensure the function behaves as expected across various scenarios.protected function getExpectedSqlStatements(): array { return [ 'bins by 15 minutes' => "SELECT date_bin('15 minutes', c0_.datetime1, '2001-02-16 20:05:00') AS sclr_0 FROM ContainsDates c0_", 'bins by 1 hour' => "SELECT date_bin('1 hour', c0_.datetime1, '2001-02-16 00:00:00') AS sclr_0 FROM ContainsDates c0_", + 'bins by 30 seconds with timestamp literal' => "SELECT date_bin('30 seconds', '2023-03-10 12:34:56', '2000-01-01 00:00:00') AS sclr_0 FROM ContainsDates c0_", + 'bins with different origin' => "SELECT date_bin('1 day', c0_.datetime1, '2023-01-01 12:00:00') AS sclr_0 FROM ContainsDates c0_", ]; } protected function getDqlStatements(): array { return [ 'bins by 15 minutes' => \sprintf("SELECT DATE_BIN('15 minutes', e.datetime1, '2001-02-16 20:05:00') FROM %s e", ContainsDates::class), 'bins by 1 hour' => \sprintf("SELECT DATE_BIN('1 hour', e.datetime1, '2001-02-16 00:00:00') FROM %s e", ContainsDates::class), + 'bins by 30 seconds with timestamp literal' => \sprintf("SELECT DATE_BIN('30 seconds', '2023-03-10 12:34:56', '2000-01-01 00:00:00') FROM %s e", ContainsDates::class), + 'bins with different origin' => \sprintf("SELECT DATE_BIN('1 day', e.datetime1, '2023-01-01 12:00:00') FROM %s e", ContainsDates::class), ]; }📝 Committable suggestion
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 these really relevant test scenarios?
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.
@martin-georgiev Looking at the PostgreSQL documentation more carefully, I think the most relevant additional test case would be the one with a different origin value, as the origin parameter significantly affects where bin boundaries are created.
The timestamp literal test is less critical since the function is primarily translating DQL to SQL correctly, which is already demonstrated with field references. The smaller interval test (30 seconds) doesn't really test anything fundamentally different from the existing 15-minute test.
The existing tests already cover the basic functionality of the DATE_BIN() implementation. If you want to keep the test focused on the most essential scenarios, just adding a test with a different origin value would be the most valuable addition.
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.
How is "bins by 1 hour" any different from "bins with different origin"?
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.
@martin-georgiev You're absolutely right. Looking more closely at the existing test cases, there's no fundamental difference in what's being tested. Both cases already test the function's ability to handle different interval values and different origin timestamps:
The existing test cases already adequately cover the functionality by testing different intervals ('15 minutes' and '1 hour') and different origins. My suggested addition doesn't add any meaningful test coverage that isn't already present in your implementation.