Skip to content

feat: add the ability to specify range bounds #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Closed
janklan wants to merge 2 commits into from
Closed

feat: add the ability to specify range bounds #371

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4099ba3
Add the ability to specify range bounds
janklan May 7, 2025
532f94d
Merge branch 'main' into feat/range-bounds
martin-georgiev May 17, 2025
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
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ This package provides comprehensive Doctrine support for PostgreSQL features:
- Array aggregation (`array_agg`)
- Array manipulation (`array_append`, `array_prepend`, `array_remove`, `array_replace`, `array_shuffle`)
- Array dimensions and length
- **Range functions**
- Arithmetic ranges (`int4range`, `int8range`, `numrange`)
- Date ranges (`daterange`, `tsrange`, `tstzrange`)
- **JSON Functions**
- JSON construction (`json_build_object`, `jsonb_build_object`)
- JSON manipulation and transformation
Expand Down
33 changes: 33 additions & 0 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php
View file
Open in desktop
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;

/**
* @see https://www.postgresql.org/docs/17/rangetypes.html
* @since 3.1
Copy link
Owner

@martin-georgiev martin-georgiev May 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @since 3.1
* @since 3.2

*
* @author Jan Klan <jan@klan.com.au>
*/
abstract class BaseRangeFunction extends BaseVariadicFunction
{
protected function getNodeMappingPattern(): array
{
return [
'ArithmeticPrimary',
'ArithmeticPrimary',
'StringPrimary',
Comment on lines +18 to +20
Copy link
Owner

@martin-georgiev martin-georgiev May 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'ArithmeticPrimary',
'ArithmeticPrimary',
'StringPrimary',
'ArithmeticPrimary',
'StringPrimary',

];
}

protected function getMinArgumentCount(): int
{
return 2;
}

protected function getMaxArgumentCount(): int
{
return 3;
}
}
17 changes: 12 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,19 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Daterange extends BaseFunction
class Daterange extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('daterange(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'daterange';
}

protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
Comment on lines +25 to +27
Copy link
Owner

@martin-georgiev martin-georgiev May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'StringPrimary',
'StringPrimary',
'StringPrimary',
'StringPrimary',

];
}
}
8 changes: 3 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Int4range extends BaseFunction
class Int4range extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('int4range(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'int4range';
}
}
8 changes: 3 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Int8range extends BaseFunction
class Int8range extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('int8range(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'int8range';
}
}
8 changes: 3 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Numrange extends BaseFunction
class Numrange extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('numrange(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'numrange';
}
}
17 changes: 12 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,19 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Tsrange extends BaseFunction
class Tsrange extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('tsrange(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'tsrange';
}

protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
Comment on lines +25 to +27
Copy link
Owner

@martin-georgiev martin-georgiev May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai - does this look alright to you? Analyse the usage of getNodeMappingPattern and cross-check with Postgresql's function signature. You may need to revise similar changes in the current PR.

Copy link

@coderabbitai coderabbitai bot May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@martin-georgiev I've analyzed the usage of getNodeMappingPattern and cross-checked with PostgreSQL's function signatures, and found a potential issue.

The code in Tsrange.php correctly overrides getNodeMappingPattern() to use three StringPrimary nodes, which is appropriate for timestamp ranges where all parameters (including bounds) are passed as strings.

However, I noticed an inconsistency:

  • BaseRangeFunction defines the default pattern as [ArithmeticPrimary, ArithmeticPrimary, StringPrimary]
  • Tsrange and Tstzrange override it to use three StringPrimary nodes
  • Daterange also uses string literals in the tests, but doesn't override the pattern

This means Daterange inherits the default pattern with ArithmeticPrimary nodes, but is actually used with string literals in tests. For consistency, Daterange should also override getNodeMappingPattern() similar to Tsrange and Tstzrange.

Would you like me to provide a code suggestion for the Daterange class?

(_/)
(•ㅅ•)
/ づ ♥ ~thinking...

🏁 Script executed:

#!/bin/bash
# First, let's examine the BaseRangeFunction class
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php

# Let's also see if there are any other range functions that have been modified
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php

# Let's check some of the tests to understand the usage
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php

Length of output: 2443

🏁 Script executed:

#!/bin/bash
# Let's check the other range functions to see if they properly override getNodeMappingPattern
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4range.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8range.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php

Length of output: 2125

🏁 Script executed:

#!/bin/bash
# Let's check the Daterange tests to see how dates are passed
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DaterangeTest.php

Length of output: 853

🏁 Script executed:

#!/bin/bash
# Look at how these range functions are used in the tests, specifically in DQL statements
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DaterangeTest.php
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php
grep -A 3 "getDqlStatements" tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php

Length of output: 822

🌐 Web query:

PostgreSQL tsrange function signature and parameters

💡 Result:

PostgreSQL's tsrange is a built-in range type that represents a range of timestamp values without time zone. This range type is useful for working with time periods in PostgreSQL databases.

Function Signature and Parameters

The tsrange constructor function can be used with two different parameter patterns:

  1. Two-parameter form: Creates a range with inclusive lower bound and exclusive upper bound (normal form)

    tsrange(lower_bound timestamp, upper_bound timestamp)

  2. Four-parameter form: Creates a range with explicitly specified bound inclusivity

    tsrange(lower_bound timestamp, upper_bound timestamp, lower_inclusive boolean, upper_inclusive boolean)

Where:

  • lower_bound: The starting timestamp of the range
  • upper_bound: The ending timestamp of the range
  • lower_inclusive: Boolean indicating whether the lower bound is inclusive (true) or exclusive (false)
  • upper_inclusive: Boolean indicating whether the upper bound is inclusive (true) or exclusive (false)

Usage Examples

The tsrange type is especially useful for scenarios involving time periods, such as reservations, events, or availability windows:

-- Create a reservation table using tsrange
CREATE TABLE reservation (
    room int, 
    during tsrange
);

-- Insert a reservation for a specific time period
INSERT INTO reservation VALUES (1108, '[2010-01-01 14:30, 2010-01-01 15:30)');

You can also query records that fall within a specific time range:

-- Select orders created within a specific time range
SELECT * FROM orders
WHERE created_at <@ tsrange('2023-03-14 04:00:00', '2023-03-14 10:00:00');

PostgreSQL also provides a corresponding multirange type called tsmultirange, which represents an ordered list of non-contiguous, non-empty, non-null timestamp ranges[1].

Citations:

@martin-georgiev I've analyzed the usage of getNodeMappingPattern in the Tsrange class and cross-checked with PostgreSQL's function signatures.

The implementation looks correct for the following reasons:

  1. Tsrange overrides getNodeMappingPattern() to use three StringPrimary nodes, which is appropriate since PostgreSQL timestamp ranges expect string parameters for both timestamps and bounds.

  2. Other date/time range functions follow the same pattern:

    • Tstzrange uses three StringPrimary nodes
    • Daterange also correctly overrides the pattern with three StringPrimary nodes

  3. The default pattern in BaseRangeFunction ([ArithmeticPrimary, ArithmeticPrimary, StringPrimary]) is suited for numeric ranges like int4range, int8range, and numrange, where the first two parameters are numbers.

  4. All the tests confirm this design, showing that:

    • Date/time ranges are used with string literals
    • Numeric ranges are used with numeric literals

This implementation correctly handles PostgreSQL's range function signatures, including the optional third parameter for specifying bounds. No revisions appear necessary for the current PR.

🧬 Code Graph Analysis Results

Relevant snippets for analyzing getNodeMappingPattern usage in Tsrange.php and cross-checking with PostgreSQL function signature:

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (lines 1-29)

<?php

declare(strict_types=1);

namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

/**
 * Implementation of PostgreSQL TSRANGE().
 *
 * @see https://www.postgresql.org/docs/17/rangetypes.html
 * @since 2.9
 *
 * @author Martin Georgiev <martin.georgiev@gmail.com>
 */
class Tsrange extends BaseRangeFunction
{
    protected function getFunctionName(): string
    {
        return 'tsrange';
    }

    protected function getNodeMappingPattern(): array
    {
        return [
            'StringPrimary',
            'StringPrimary',
            'StringPrimary',
        ];
    }
}

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (lines 13-33)

abstract class BaseRangeFunction extends BaseVariadicFunction
{
    protected function getNodeMappingPattern(): array
    {
        return [
            'ArithmeticPrimary',
            'ArithmeticPrimary',
            'StringPrimary',
        ];
    }

    protected function getMinArgumentCount(): int
    {
        return 2;
    }

    protected function getMaxArgumentCount(): int
    {
        return 3;
    }
}
  • This base class defines a default getNodeMappingPattern with two ArithmeticPrimary and one StringPrimary nodes.
  • It also defines minimum and maximum argument counts (2 and 3 respectively).

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (lines 17-29)

protected function getFunctionName(): string
{
    return 'daterange';
}

protected function getNodeMappingPattern(): array
{
    return [
        'StringPrimary',
        'StringPrimary',
        'StringPrimary',
    ];
}
  • This function uses the same node mapping pattern as Tsrange.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php (lines 17-29)

protected function getFunctionName(): string
{
    return 'tstzrange';
}

protected function getNodeMappingPattern(): array
{
    return [
        'StringPrimary',
        'StringPrimary',
        'StringPrimary',
    ];
}
  • This function also uses the same node mapping pattern as Tsrange.

Summary of relevant context:

  • Tsrange overrides getNodeMappingPattern to return three 'StringPrimary' elements.
  • The base class BaseRangeFunction returns a different pattern: two 'ArithmeticPrimary' and one 'StringPrimary'.
  • Other range functions like Daterange and Tstzrange also use the three 'StringPrimary' pattern.
  • PostgreSQL's tsrange function signature (per official docs) accepts:
    • Two timestamp values (lower and upper bounds),
    • An optional text parameter for range bounds (e.g., '[)', '()').

This matches the three parameters represented as strings in the node mapping pattern.

Additional snippet for comparison (RegexpReplace.php) showing complex node mapping patterns:

protected function getNodeMappingPattern(): array
{
    /*
     * PostgreSQL overloads the 4th argument depending on its type:
     *   - if the 4th arg is a string, it’s taken as flags.
     *   - if the 4th arg is an integer, it’s taken as start position. This can be extended with the Nth argument.
     */
    return [
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary,StringPrimary', // with start, N and flags: regexp_replace(string, pattern, replacement, 3, 2, 'i')
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary', // with start and N: regexp_replace(string, pattern, replacement, 3, 2)
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary', // with start: regexp_replace(string, pattern, replacement, 3)
        'StringPrimary,StringPrimary,StringPrimary,StringPrimary', // with flags: regexp_replace(string, pattern, replacement, 'i')
        'StringPrimary,StringPrimary,StringPrimary', // basic replacement: regexp_replace(string, pattern, replacement)
    ];
}
  • Shows that getNodeMappingPattern can return multiple patterns to handle function overloading.
  • Tsrange returns a single pattern array with three 'StringPrimary' elements.

Summary for reviewer:

  • The Tsrange class overrides getNodeMappingPattern to expect three string-type nodes, consistent with the PostgreSQL tsrange function signature: two timestamps and an optional text parameter.
  • The base class BaseRangeFunction uses a different default pattern (two arithmetic and one string), but Tsrange and similar range functions (Daterange, Tstzrange) override it to three strings.
  • This pattern aligns with the PostgreSQL documentation for tsrange(lower timestamp, upper timestamp, text bounds).
  • Other range functions in the same namespace follow the same pattern, indicating consistency.
  • The getNodeMappingPattern method can return multiple patterns for overloaded functions, but Tsrange uses a single pattern.

This context should assist in verifying the correctness of getNodeMappingPattern usage in Tsrange.php and guide any necessary revisions in the current PR or similar functions.

Copy link
Owner

@martin-georgiev martin-georgiev May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? Explain why RegexpCount and RegexpInstr look different then.

Copy link

@coderabbitai coderabbitai bot May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Let's check the implementations of RegexpCount and RegexpInstr
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCount.php
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstr.php

# Let's also look at what base class they extend from
grep -A 1 "class RegexpCount" src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCount.php
grep -A 1 "class RegexpInstr" src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstr.php

# Compare with Tsrange implementation again for reference
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php

# Compare with BaseRangeFunction
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php

# Let's also check if there's a base class for regexp functions
ls -la src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRegexpFunction.php 2>/dev/null || echo "BaseRegexpFunction.php not found"

Length of output: 4544

🏁 Script executed:

#!/bin/bash
# Let's check the BaseVariadicFunction to understand how the node mapping pattern is used
cat src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php

# Also check the tests for RegexpCount and RegexpInstr to understand how they're used
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpCountTest.php 2>/dev/null || echo "RegexpCountTest.php not found"
cat tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpInstrTest.php 2>/dev/null || echo "RegexpInstrTest.php not found"

Length of output: 12031

@martin-georgiev I understand your question now. The difference between RegexpCount/RegexpInstr and Tsrange implementations is intentional and relates to how they handle function overloads in PostgreSQL:

Why They Look Different

  1. Different Function Complexity:

    • RegexpCount/RegexpInstr: Support multiple function signatures with varying parameter counts (2-4 and 2-7 parameters respectively)
    • Tsrange: Supports a simpler function with 2-3 parameters of consistent types

  2. Node Mapping Format:

    • RegexpCount/RegexpInstr use comma-separated strings in an array: 
      protected function getNodeMappingPattern(): array
{
    return [
        'StringPrimary,StringPrimary,ArithmeticPrimary,StringPrimary', // 4 params
        'StringPrimary,StringPrimary,ArithmeticPrimary', // 3 params
        'StringPrimary,StringPrimary', // 2 params
    ];
}
    • Tsrange uses individual array elements: 
      protected function getNodeMappingPattern(): array
{
    return [
        'StringPrimary',
        'StringPrimary',
        'StringPrimary',
    ];
}

  3. How BaseVariadicFunction Processes Them:

    • For RegexpCount/RegexpInstr: It tries each pattern string, splitting it by commas and matching arguments until one succeeds
    • For Tsrange: It uses the single pattern with consistent parameter types

Both implementations are correct but serve different purposes based on the complexity of the PostgreSQL functions they represent. The regexp functions need to handle multiple overloads with varying parameter counts, while range functions have a more consistent signature.

🧬 Code Graph Analysis Results

Relevant snippets for understanding Tsrange.php and the user comment about RegexpCount and RegexpInstr differences:

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php (lines 1-29)

<?php

declare(strict_types=1);

namespace MartinGeorgiev\Doctrine\ORM\Query\AST\Functions;

/**
 * Implementation of PostgreSQL TSRANGE().
 *
 * @see https://www.postgresql.org/docs/17/rangetypes.html
 * @since 2.9
 *
 * @author Martin Georgiev <martin.georgiev@gmail.com>
 */
class Tsrange extends BaseRangeFunction
{
    protected function getFunctionName(): string
    {
        return 'tsrange';
    }

    protected function getNodeMappingPattern(): array
    {
        return [
            'StringPrimary',
            'StringPrimary',
            'StringPrimary',
        ];
    }
}

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRangeFunction.php (lines 13-33)

abstract class BaseRangeFunction extends BaseVariadicFunction
{
    protected function getNodeMappingPattern(): array
    {
        return [
            'ArithmeticPrimary',
            'ArithmeticPrimary',
            'StringPrimary',
        ];
    }

    protected function getMinArgumentCount(): int
    {
        return 2;
    }

    protected function getMaxArgumentCount(): int
    {
        return 3;
    }
}
  • Abstract base class for range functions.
  • Defines a default node mapping pattern expecting two ArithmeticPrimary and one StringPrimary arguments.
  • Defines minimum and maximum argument counts (2 to 3).

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Numrange.php (lines 17-20)

protected function getFunctionName(): string
{
    return 'numrange';
}
  • Example of another range function overriding the function name.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php (lines 17-29)

protected function getFunctionName(): string
{
    return 'daterange';
}

protected function getNodeMappingPattern(): array
{
    return [
        'StringPrimary',
        'StringPrimary',
        'StringPrimary',
    ];
}
  • Shows a range function overriding the node mapping pattern to three StringPrimary arguments, similar to Tsrange.

File: src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpReplace.php (lines 22-41)

protected function getNodeMappingPattern(): array
{
    /*
     * PostgreSQL overloads the 4th argument depending on its type:
     *   - if the 4th arg is a string, it’s taken as flags.
     *   - if the 4th arg is an integer, it’s taken as start position. This can be extended with the Nth argument.
     */
    return [
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary,StringPrimary', // with start, N and flags: regexp_replace(string, pattern, replacement, 3, 2, 'i')
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary,ArithmeticPrimary', // with start and N: regexp_replace(string, pattern, replacement, 3, 2)
        'StringPrimary,StringPrimary,StringPrimary,ArithmeticPrimary', // with start: regexp_replace(string, pattern, replacement, 3)
        'StringPrimary,StringPrimary,StringPrimary,StringPrimary', // with flags: regexp_replace(string, pattern, replacement, 'i')
        'StringPrimary,StringPrimary,StringPrimary', // basic replacement: regexp_replace(string, pattern, replacement)
    ];
}

protected function getFunctionName(): string
{
    return 'regexp_replace';
}
  • Shows a complex node mapping pattern with multiple overloads depending on argument types.
  • Demonstrates why some functions like RegexpReplace have more complex patterns compared to simpler range functions.

Summary of relevance:

  • Tsrange.php overrides getNodeMappingPattern() to expect three StringPrimary arguments, differing from the base class BaseRangeFunction which expects two ArithmeticPrimary and one StringPrimary.
  • Other range functions like Daterange also override the node mapping pattern similarly.
  • The RegexpReplace function shows a more complex and varied node mapping pattern due to PostgreSQL's function overloading and argument type variations.
  • This contrast explains why RegexpCount and RegexpInstr might have different node mapping patterns compared to simpler range functions like Tsrange.

These snippets provide the factual context needed to understand the differences in node mapping patterns and function implementations.

Copy link

@coderabbitai coderabbitai bot May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Owner

@martin-georgiev martin-georgiev May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Now revise your approval of the PR and request changes where they are relevant!

Copy link

@coderabbitai coderabbitai bot May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A summary of the edits CodeRabbit can apply:

Request changes to Tsrange.php so that getNodeMappingPattern() returns a single-element array ['StringPrimary'], fixing the mapping-bug in BaseVariadicFunction.

Request changes to Daterange.php so that getNodeMappingPattern() returns ['StringPrimary'] only, avoiding the three-entry bug.

Request changes to Tstzrange.php so that getNodeMappingPattern() returns ['StringPrimary'] only, correcting the pattern application bug.

Modify Daterange.php so getNodeMappingPattern returns a single 'StringPrimary' element.

Modify Tsrange.php so getNodeMappingPattern returns a single 'StringPrimary' element.

Modify Tstzrange.php so getNodeMappingPattern returns a single 'StringPrimary' element.

The Tstzrange.php file will be updated to use a single-element node mapping pattern ['StringPrimary'] so that all parameters are correctly treated as StringPrimary.

I will request changes in Tsrange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.

I will request changes in Daterange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.

I will request changes in Tstzrange.php to simplify getNodeMappingPattern() to a single ['StringPrimary'] element.

The getNodeMappingPattern in Tsrange.php will be simplified to a single 'StringPrimary'.

The getNodeMappingPattern in Tstzrange.php will be simplified to a single 'StringPrimary'.

The getNodeMappingPattern in Daterange.php will be simplified to a single 'StringPrimary'.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tsrange.php

Remove the existing multi-line return array in getNodeMappingPattern():

return [
    'StringPrimary',
    'StringPrimary',
    'StringPrimary',
];

Replace it with a single-element return immediately after the method signature:

return ['StringPrimary'];

Save Tsrange.php and run the TsrangeTest suite to verify that both 2-argument and 3-argument forms of tsrange now parse without errors.

✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Daterange.php

Remove the existing multi-line return array in getNodeMappingPattern():

return [
    'StringPrimary',
    'StringPrimary',
    'StringPrimary',
];

Replace it with:

return ['StringPrimary'];

Save Daterange.php and run the DaterangeTest suite to confirm both 2-argument and 3-argument usages of daterange parse correctly.

✏️ src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php

Remove the existing multi-line return array in getNodeMappingPattern():

return [
    'StringPrimary',
    'StringPrimary',
    'StringPrimary',
];

Replace it with:

return ['StringPrimary'];

Save Tstzrange.php and run the TstzrangeTest suite to ensure both 2-argument and 3-argument invocations of tstzrange parse correctly.

A summary of the context that CodeRabbit is considering across the codebase:

We need to locate the BaseVariadicFunction definition and see how it handles a single-element node mapping pattern to confirm that returning ['StringPrimary'] will correctly apply that type to all arguments.

Locate every subclass of BaseRangeFunction that overrides getNodeMappingPattern, so we know which files need updating.

Inspect Daterange.php to determine the exact lines to replace.

Inspect Tsrange.php to determine the exact lines to replace.

Inspect Tstzrange.php to determine the exact lines to replace.

Confirm that BaseRangeFunction defines a minimum of 2 and a maximum of 3 arguments, so a single‐element pattern will correctly apply to both two‐ and three‐parameter invocations.

Identify all date/time range function classes to ensure consistent updates across Tsrange, Daterange, and Tstzrange.

Locate the existing override of getNodeMappingPattern() in Tsrange.php to determine the exact lines to replace.

Locate the existing override of getNodeMappingPattern() in Daterange.php to determine the exact lines to replace.

Locate the existing override of getNodeMappingPattern() in Tstzrange.php to determine the exact lines to replace.

Identify all range‐function classes that override getNodeMappingPattern so we can update their patterns to a single 'StringPrimary'.

  • Created local commit - (🔄 Check again to try again)

Copy link

@coderabbitai coderabbitai bot May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

CodeRabbit has failed to create a local commit with your edits.

Copy link

@coderabbitai coderabbitai bot May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

CodeRabbit has failed to create a local commit with your edits.

Copy link

@coderabbitai coderabbitai bot May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

CodeRabbit has failed to create a local commit with your edits.

Comment on lines +25 to +27
Copy link
Owner

@martin-georgiev martin-georgiev May 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'StringPrimary',
'StringPrimary',
'StringPrimary',
'StringPrimary',

];
}
}
17 changes: 12 additions & 5 deletions src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Tstzrange.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,19 @@
*
* @author Martin Georgiev <martin.georgiev@gmail.com>
*/
class Tstzrange extends BaseFunction
class Tstzrange extends BaseRangeFunction
{
protected function customizeFunction(): void
protected function getFunctionName(): string
{
$this->setFunctionPrototype('tstzrange(%s, %s)');
$this->addNodeMapping('StringPrimary');
$this->addNodeMapping('StringPrimary');
return 'tstzrange';
}

protected function getNodeMappingPattern(): array
{
return [
'StringPrimary',
'StringPrimary',
'StringPrimary',
Comment on lines +25 to +27
Copy link
Owner

@martin-georgiev martin-georgiev May 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'StringPrimary',
'StringPrimary',
'StringPrimary',
'StringPrimary',

];
}
}
2 changes: 2 additions & 0 deletions tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int4rangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ protected function getExpectedSqlStatements(): array
{
return [
'SELECT int4range(c0_.integer1, c0_.integer2) AS sclr_0 FROM ContainsIntegers c0_',
'SELECT int4range(c0_.integer1, c0_.integer2, \'[)\') AS sclr_0 FROM ContainsIntegers c0_',
];
}

protected function getDqlStatements(): array
{
return [
\sprintf('SELECT INT4RANGE(e.integer1, e.integer2) FROM %s e', ContainsIntegers::class),
\sprintf('SELECT INT4RANGE(e.integer1, e.integer2, \'[)\') FROM %s e', ContainsIntegers::class),
];
}
}
2 changes: 2 additions & 0 deletions tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Int8rangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ protected function getExpectedSqlStatements(): array
{
return [
'SELECT int8range(c0_.integer1, c0_.integer2) AS sclr_0 FROM ContainsIntegers c0_',
'SELECT int8range(c0_.integer1, c0_.integer2, \'[)\') AS sclr_0 FROM ContainsIntegers c0_',
];
}

protected function getDqlStatements(): array
{
return [
\sprintf('SELECT INT8RANGE(e.integer1, e.integer2) FROM %s e', ContainsIntegers::class),
\sprintf('SELECT INT8RANGE(e.integer1, e.integer2, \'[)\') FROM %s e', ContainsIntegers::class),
];
}
}
2 changes: 2 additions & 0 deletions tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/NumrangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ protected function getExpectedSqlStatements(): array
{
return [
'SELECT numrange(c0_.decimal1, c0_.decimal2) AS sclr_0 FROM ContainsDecimals c0_',
'SELECT numrange(c0_.decimal1, c0_.decimal2, \'[)\') AS sclr_0 FROM ContainsDecimals c0_',
];
}

protected function getDqlStatements(): array
{
return [
\sprintf('SELECT NUMRANGE(e.decimal1, e.decimal2) FROM %s e', ContainsDecimals::class),
\sprintf('SELECT NUMRANGE(e.decimal1, e.decimal2, \'[)\') FROM %s e', ContainsDecimals::class),
];
}
}
2 changes: 2 additions & 0 deletions tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TsrangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ protected function getExpectedSqlStatements(): array
{
return [
'SELECT tsrange(c0_.datetime1, c0_.datetime2) AS sclr_0 FROM ContainsDates c0_',
'SELECT tsrange(c0_.datetime1, c0_.datetime2, \'[)\') AS sclr_0 FROM ContainsDates c0_',
];
}

protected function getDqlStatements(): array
{
return [
\sprintf('SELECT TSRANGE(e.datetime1, e.datetime2) FROM %s e', ContainsDates::class),
\sprintf('SELECT TSRANGE(e.datetime1, e.datetime2, \'[)\') FROM %s e', ContainsDates::class),
];
}
}
2 changes: 2 additions & 0 deletions tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/TstzrangeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ protected function getExpectedSqlStatements(): array
{
return [
'SELECT tstzrange(c0_.datetimetz1, c0_.datetimetz2) AS sclr_0 FROM ContainsDates c0_',
'SELECT tstzrange(c0_.datetimetz1, c0_.datetimetz2, \'[)\') AS sclr_0 FROM ContainsDates c0_',
];
}

protected function getDqlStatements(): array
{
return [
\sprintf('SELECT TSTZRANGE(e.datetimetz1, e.datetimetz2) FROM %s e', ContainsDates::class),
\sprintf('SELECT TSTZRANGE(e.datetimetz1, e.datetimetz2, \'[)\') FROM %s e', ContainsDates::class),
];
}
}
Loading