Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateAdd.php
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 src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DateBin.php
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');
}
}
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');
}
}
}
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
));
}
}
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;
}
}
}
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();
}
}
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),
];
}
}
Copy link

@coderabbitai coderabbitai bot Apr 12, 2025

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:

  1. Binning with a timestamp literal instead of a field
  2. Edge cases like binning with small intervals (e.g., seconds)
  3. Test with different origin values and their effects on binning
  4. Different interval formats accepted by PostgreSQL

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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<?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),
];
}
}
<?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_",
'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),
];
}
}

Copy link
Owner Author

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?

Copy link

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.

Copy link
Owner Author

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"?

Copy link

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:

  • "bins by 1 hour": Uses '1 hour' interval with origin '2001-02-16 00:00:00'
  • "bins with different origin" (suggested): Uses '1 day' interval with origin '2023-01-01 12:00:00'

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.

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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ protected function assertSqlFromDql(string $expectedSql, string $dql, string $me
self::assertEquals($expectedSql, $query->getSQL(), $message);
}

private function buildEntityManager(): EntityManager
protected function buildEntityManager(): EntityManager
{
return new EntityManager(DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true], $this->configuration), $this->configuration);
}
Expand Down
Loading