feat: extend support of REGEXP_LIKE(), REGEXP_MATCH() and REGEXP_REPLACE() while deprecating the legacy limited flagged variations of FlaggedRegexpLike, FlaggedRegexpMatch and FlaggedRegexpReplace
#357
Conversation
…precating the legacy limited flagged variations of `FlaggedRegexpLike`, `FlaggedRegexpMatch` and `FlaggedRegexpReplace`
WalkthroughThis update deprecates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DoctrineDQL
participant RegexpLike
participant RegexpMatch
User->>DoctrineDQL: Build DQL query with regexp_like() or regexp_match()
DoctrineDQL->>RegexpLike: Parse arguments (2 or 3)
RegexpLike->>DoctrineDQL: Validate argument count
DoctrineDQL->>RegexpLike: Map arguments to node pattern
RegexpLike->>DoctrineDQL: Generate SQL function call
Note over RegexpLike,RegexpMatch: Same flow applies for RegexpMatch
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.php (1)
19-24: 💡 Verification agent🧩 Analysis chain
Verify that the test coverage aligns with PostgreSQL documentation.
The test validates basic functionality of
regexp_likewith a pattern and flags, but could benefit from more comprehensive test cases that demonstrate the various flag options.
🏁 Script executed:
#!/bin/bash # Check if the test covers examples from PostgreSQL documentation echo "Checking for comprehensive test coverage based on PostgreSQL examples..." # Look for test cases for the non-deprecated RegexpLike rg -n "regexp_like" tests/Length of output: 1459
Enhance test coverage for all RegexpLike flag options
While the existing tests in
RegexpLikeTest.phpandFlaggedRegexpLikeTest.phpverify basic matching and the ignore‑case ('i') flag, PostgreSQL’sregexp_likesupports additional flags and flag combinations. Please add tests to cover:
- Single‑letter flags:
'c'(case‑sensitive)'m'(multi‑line mode)'n'(new‑line sensitive)'s'(dot‑all mode)'x'(extended mode)- Combined flags (e.g.,
'im','msx')- Edge cases where flags have no effect or conflict
Locations to update:
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLikeTest.php
- tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.php
This will ensure alignment with PostgreSQL’s documented behavior and catch regressions across all flag scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLike.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatch.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplace.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLike.php(1 hunks)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatch.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatchTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplaceTest.php(1 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLikeTest.php(2 hunks)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php`: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL que...
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/**/*.php: Use the PostgreSQL official documentation to verify that tests include comprehensive use cases and example SQL queries for the tested SQL functions and operators.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplaceTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatchTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLikeTest.phptests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php
🧠 Learnings (1)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLikeTest.php (2)
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#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.
Learnt from: martin-georgiev
PR: martin-georgiev/postgresql-for-doctrine#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`), not PHPUnit's TestCase, and therefore don't need an explicit import statement.
🧬 Code Graph Analysis (4)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLike.php (1)
FlaggedRegexpLike(17-26)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatchTest.php (1)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatch.php (1)
FlaggedRegexpMatch(17-26)
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php (5)
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsTexts.php (1)
ORM(10-18)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (1)
BaseVariadicFunction(20-135)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Exception/InvalidArgumentForVariadicFunctionException.php (1)
InvalidArgumentForVariadicFunctionException(7-56)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatch.php (1)
RegexpMatch(19-45)tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunctionTestCase.php (1)
BaseVariadicFunctionTestCase(14-42)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLike.php (3)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseVariadicFunction.php (5)
BaseVariadicFunction(20-135)getNodeMappingPattern(32-32)getFunctionName(27-27)getMinArgumentCount(34-34)getMaxArgumentCount(36-36)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatch.php (4)
getNodeMappingPattern(21-29)getFunctionName(31-34)getMinArgumentCount(36-39)getMaxArgumentCount(41-44)src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/BaseRegexpFunction.php (1)
getFunctionName(9-9)
🪛 markdownlint-cli2 (0.17.2)
docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md
85-85: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
86-86: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
87-87: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
🔇 Additional comments (28)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatch.php (1)
7-10: Appropriate deprecation notice with clear migration path.The deprecation notice clearly indicates when this function will be removed and what alternative to use instead. This gives users time to migrate to the recommended
RegexpMatchclass before v4.0.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpMatchTest.php (2)
15-15: Good use of static analysis suppression for deprecated code.Adding the
@phpstan-ignore-linecomment is appropriate for suppressing static analysis warnings on code that's intentionally using deprecated functionality. This allows the tests to continue validating the deprecated class during its deprecation period.
19-24:✅ Verification successful
Verify that the test coverage aligns with PostgreSQL documentation.
The test validates the basic functionality of
regexp_matchwith a pattern and flags, but could benefit from more comprehensive test cases.
🏁 Script executed:
#!/bin/bash # Check if the test covers examples from PostgreSQL documentation echo "Checking for comprehensive test coverage based on PostgreSQL examples..." # Look for additional test cases in other files rg -n "regexp_match" tests/Length of output: 1471
Test Coverage Is Already Comprehensive
The existing RegexpMatchTest covers:
- Basic pattern matching
- Matching with flags
- Matching with a start position
- Matching with both start position and flags
- Argument validation and exception cases
No additional test cases are required to align with the PostgreSQL documentation.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLikeTest.php (1)
15-15: Good use of static analysis suppression for deprecated code.Adding the
@phpstan-ignore-linecomment is appropriate for suppressing static analysis warnings on code that's intentionally using deprecated functionality. This maintains test coverage during the deprecation period.tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplaceTest.php (2)
15-15: Good use of static analysis suppression for deprecated code.Adding the
@phpstan-ignore-linecomment is appropriate for suppressing static analysis warnings on code that's intentionally using deprecated functionality. This allows the tests to continue validating the deprecated class during its deprecation period.
19-24:✅ Verification successful
Verify that the test coverage aligns with PostgreSQL documentation.
The test validates basic functionality of
regexp_replacewith a pattern, replacement, and flags, but could benefit from more comprehensive test cases that demonstrate various replacement scenarios.
🏁 Script executed:
#!/bin/bash # Check if the test covers examples from PostgreSQL documentation echo "Checking for comprehensive test coverage based on PostgreSQL examples..." # Look for test cases for the non-deprecated RegexpReplace rg -n "regexp_replace" tests/Length of output: 2070
🏁 Script executed:
#!/bin/bash sed -n '1,200p' tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplaceTest.phpLength of output: 994
No additional tests required for
FlaggedRegexpReplace.The suite already covers all major
regexp_replacescenarios (basic replacement, flags, start position, occurrence count) inRegexpReplaceTest.php. TheFlaggedRegexpReplaceTest.phpexercise—passing a flags-only invocation—is consistent with its purpose and PostgreSQL behavior.
– Comprehensive flag/use‐case combinations live in tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpReplaceTest.php
–FlaggedRegexpReplaceTest.phpcorrectly verifies the DQL function mapping with the flag argument only.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpReplace.php (1)
7-10: Appropriate deprecation notice with clear migration path.The deprecation notice clearly indicates when this function will be removed and what alternative to use instead. This gives users time to migrate to the recommended
RegexpReplaceclass before v4.0.src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/FlaggedRegexpLike.php (1)
8-10: Correctly marked as deprecatedThe deprecation notice is clear and helpful, guiding users to use
RegexpLikeinstead of this class. This aligns well with the PR objective of deprecating legacy limited flagged variations of regexp functions.docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md (1)
85-87: Documentation simplification looks goodThe documentation has been simplified by unifying regexp function entries without flag distinctions, which aligns with the code changes that consolidate these functions. This makes the documentation clearer and more maintainable.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
85-85: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe(MD055, table-pipe-style)
86-86: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe(MD055, table-pipe-style)
87-87: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe(MD055, table-pipe-style)
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatch.php (4)
10-13: Documentation update enhances clarityThe updated description clearly explains what the function does and links to the latest PostgreSQL documentation. The reference to PostgreSQL 17 shows forward-thinking documentation practices.
17-18: Helpful usage example addedAdding an example of how to use the function in DQL is very helpful for developers. This kind of practical documentation makes the library more user-friendly.
19-29: Well-structured variadic function implementationThe class now extends
BaseVariadicFunctionand properly defines the node mapping patterns for different argument combinations. The implementation supports various argument patterns in a clean and maintainable way.
36-44: Proper argument count constraintsThe implementation correctly defines the minimum (2) and maximum (4) argument counts, which aligns with PostgreSQL's regexp_match function capabilities. This provides clear boundaries for function usage.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php (5)
12-18: Good test class extension and fixture creationExtending
BaseVariadicFunctionTestCaseis appropriate for testing the variadic functionality. ThecreateFixture()method properly initializes the function for testing.
28-33: Comprehensive SQL test casesThe expected SQL statements cover all the supported patterns: basic match, with flags, with start position, and with both start position and flags. This ensures complete test coverage of the function's capabilities.
39-43: Well-structured DQL test statementsThe DQL statements match the expected SQL statements and provide good test coverage for all supported argument patterns. Using
ContainsTextsfixture makes the tests realistic.
46-53: Good validation test for minimum argumentsTesting that the function properly rejects calls with too few arguments ensures robust validation. The exception message is clear and matches the implementation's constraints.
55-62: Good validation test for maximum argumentsTesting that the function properly rejects calls with too many arguments completes the boundary testing. The exception message correctly communicates the acceptable argument range.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLikeTest.php (6)
7-9: Appropriate imports for new base class and exception.Good additions. The imports correctly reflect the new class inheritance and exception handling for argument validation tests.
12-18: Base class change and fixture setup properly implemented.The test class now correctly extends
BaseVariadicFunctionTestCaseinstead ofTestCase, which aligns with the implementation change. The newcreateFixture()method properly instantiates theRegexpLikeclass as aBaseVariadicFunction.
28-32: Well-structured test cases for SQL statements.The test cases are now well-organized with descriptive keys and cover all the function variations according to PostgreSQL documentation:
- Basic 2-parameter usage
- With flags (3 parameters)
- With start position (3 parameters)
- With both start position and flags (4 parameters)
This provides comprehensive coverage of all valid parameter combinations.
38-42: Matching DQL statements for all parameter combinations.The DQL statements correctly match the expected SQL output and cover all supported parameter combinations. The use of
sprintfwith the entity class name is a good practice for maintaining test flexibility.
46-53: Good exception testing for insufficient arguments.This test appropriately verifies that the function throws the correct exception with a meaningful message when called with fewer than the required minimum of 2 arguments.
55-62: Good exception testing for excessive arguments.This test properly validates that the function throws an exception when called with more than the maximum allowed 4 arguments. The error message is clear and helpful.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpLike.php (4)
7-18: Improved documentation with clear examples.The enhanced PHPDoc provides a more descriptive explanation of the function's behavior, includes an updated link to PostgreSQL 17 documentation, and adds a helpful example of usage in DQL. This makes the function more maintainable and easier to use.
19-20: Appropriate base class change for variadic support.Changing from
BaseRegexpFunctiontoBaseVariadicFunctioncorrectly implements the support for variable argument counts (2-4) as intended in this PR.
21-29: Well-defined node mapping patterns for all parameter combinations.The implementation provides mapping patterns for all valid parameter combinations:
- Basic 2-parameter usage: string, pattern
- 3-parameter with flags: string, pattern, flags
- 3-parameter with start position: string, pattern, position
- 4-parameter with both: string, pattern, position, flags
This correctly implements the PostgreSQL REGEXP_LIKE function's parameter variations.
36-44: Proper argument count constraints.The implementation correctly defines:
- Minimum argument count: 2 (string to search, pattern)
- Maximum argument count: 4 (string, pattern, position, flags)
These constraints match PostgreSQL's function signature and are properly enforced by the base class.
tests/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/RegexpMatchTest.php
Show resolved
Hide resolved
martin-georgiev
changed the title
REGEXP_LIKE() and REGEXP_MATCH() while deprecating the legacy limited flagged variations of FlaggedRegexpLike, FlaggedRegexpMatch and FlaggedRegexpReplaceREGEXP_LIKE(), REGEXP_MATCH() and REGEXP_REPLACE() while deprecating the legacy limited flagged variations of FlaggedRegexpLike, FlaggedRegexpMatch and FlaggedRegexpReplace
| { | ||
| protected function customizeFunction(): void | ||
| { | ||
| $this->setFunctionPrototype('regexp_like(%s, %s, 1, %s)'); |
There was a problem hiding this comment.
This was actually a bug that never worked as intended.
# Conflicts: # docs/AVAILABLE-FUNCTIONS-AND-OPERATORS.md
Summary by CodeRabbit
Deprecation Notices
Enhancements
Bug Fixes
Tests