Skip to content

Conversation

@seb-jean
Copy link
Contributor

@seb-jean seb-jean commented Apr 24, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a new DQL function for calculating earth distance between points, available for use in Doctrine ORM queries.
  • Documentation

    • Updated integration guides for Doctrine, Laravel, and Symfony to include instructions for registering the new distance calculation function.
  • Tests

    • Added unit tests to verify the correct behavior and SQL translation of the new distance function.
  • Chores

    • Added a sample entity for use in testing the new distance functionality.

@seb-jean seb-jean force-pushed the feat/distance-function-2 branch from 7da4f07 to ff82710 Compare April 24, 2025 06:20
@coderabbitai
Copy link

coderabbitai bot commented Apr 24, 2025

Walkthrough

This update introduces a new custom DQL function, DISTANCE, for Doctrine ORM, which leverages PostgreSQL's <@> earth distance operator. The change includes the implementation of the Distance function class, a corresponding test class, and a sample entity for testing. Documentation for integrating this function with Doctrine in Symfony, Laravel, and general usage has been updated with configuration examples. No changes were made to exported or public entity declarations outside of the new classes and documentation.

Changes

File(s) Change Summary
docs/INTEGRATING-WITH-DOCTRINE.md
docs/INTEGRATING-WITH-LARAVEL.md
docs/INTEGRATING-WITH-SYMFONY.md
Updated documentation to include configuration examples for registering the new custom DQL function DISTANCE.
fixtures/MartinGeorgiev/Doctrine/Entity/ContainsPoints.php Added new entity class ContainsPoints with two public Point properties for testing and demonstration.
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Distance.php Introduced new class Distance implementing the PostgreSQL <@> distance operator as a Doctrine DQL function.
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DistanceTest.php Added unit test class DistanceTest to verify the correct translation and execution of the DISTANCE DQL function.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Doctrine
    participant DistanceFunction
    participant PostgreSQL

    User->>Doctrine: Write DQL using DISTANCE()
    Doctrine->>DistanceFunction: Parse and translate DISTANCE()
    DistanceFunction->>PostgreSQL: Generate SQL with <@> operator
    PostgreSQL-->>Doctrine: Return query results
    Doctrine-->>User: Return processed results
Loading

Poem

🐇
A hop, a skip, a distance anew,
Now Doctrine knows what bunnies do—
Measure leaps with PostgreSQL flair,
Across the fields, from here to there!
With docs and tests, the journey's clear,
The DISTANCE function now is here!
🌍✨


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b922291 and 8a2c478.

📒 Files selected for processing (2)
  • fixtures/MartinGeorgiev/Doctrine/Entity/ContainsPoints.php (1 hunks)
  • tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DistanceTest.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • fixtures/MartinGeorgiev/Doctrine/Entity/ContainsPoints.php
  • tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DistanceTest.php
⏰ Context from checks skipped due to timeout of 90000ms (20)
  • GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.2 + Doctrine ORM latest + Doctrine Lexer latest
  • GitHub Check: PHP 8.4 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 1.2
  • GitHub Check: PHP 8.4 + Doctrine ORM latest + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.3 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.2 + Doctrine ORM 3.0 + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM 2.14 + Doctrine Lexer latest
  • GitHub Check: PHP 8.4 + Doctrine ORM 2.18 + Doctrine Lexer 3.0
  • GitHub Check: PHP 8.1 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.4 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.2 + Doctrine ORM 2.18 + Doctrine Lexer latest
  • GitHub Check: PHP 8.3 + Doctrine ORM latest + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer latest
  • GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer latest
  • GitHub Check: PHP 8.2 + Doctrine ORM 2.14 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.1 + Doctrine ORM 2.18 + Doctrine Lexer 2.1
  • GitHub Check: PHP 8.1 + Doctrine ORM 3.0 + Doctrine Lexer 3.0
  • GitHub Check: sloth
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@seb-jean seb-jean force-pushed the feat/distance-function-2 branch from b4edfb6 to 2440a02 Compare April 24, 2025 06:26
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 5

🧹 Nitpick comments (2)
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DistanceTest.php (2)

28-35: Fix code style issues and consider adding test cases for error conditions.

  1. The CI pipeline flagged the use of double quotes which should be replaced with single quotes.
  2. Consider adding test cases that verify error handling, such as invalid point formats.
    protected function getDqlStatements(): array
    {
        return [
-            \sprintf("SELECT DISTANCE(e.point, '(2.320041, 48.858889)') FROM %s e", ContainsPoints::class),
-            \sprintf("SELECT DISTANCE(e.point, e.point2) FROM %s e", ContainsPoints::class),
-            \sprintf("SELECT DISTANCE('(1.0, 1.0)', '(2.0, 2.0)') FROM %s e", ContainsPoints::class),
+            \sprintf('SELECT DISTANCE(e.point, \'(2.320041, 48.858889)\') FROM %s e', ContainsPoints::class),
+            \sprintf('SELECT DISTANCE(e.point, e.point2) FROM %s e', ContainsPoints::class),
+            \sprintf('SELECT DISTANCE(\'(1.0, 1.0)\', \'(2.0, 2.0)\') FROM %s e', ContainsPoints::class),
        ];
    }

1-36: Consider adding PHPDoc comments for better documentation.

Adding PHPDoc comments to the class and its methods would improve the documentation and better describe the purpose of this test class and what each method is testing.

Example:

/**
 * Test case for the DISTANCE DQL function that wraps PostgreSQL's <@> earth distance operator.
 */
class DistanceTest extends TestCase
{
    /**
     * Returns a mapping of DQL function names to their implementing classes.
     *
     * @return array<string, string>
     */
    protected function getStringFunctions(): array
    {
        // ...
    }
    
    // Similar PHPDoc for other methods
}
🧰 Tools
🪛 GitHub Actions: CI

[error] 20-29: PHP CS Fixer: Use single quotes instead of double quotes (single_quote) and missing newline at end of file (single_blank_line_at_eof).

🛑 Comments failed to post (5)
docs/INTEGRATING-WITH-LARAVEL.md (1)

221-221: ⚠️ Potential issue

Incorrect class reference for DISTANCE function.
The DISTANCE mapping points to DistanceTest::class instead of the actual Distance function class. This will prevent Doctrine from loading the function correctly.

Apply this diff:

-        'DISTANCE' => MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DistanceTest::class,
+        'DISTANCE' => MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Distance::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.

        'DISTANCE' => MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Distance::class,
docs/INTEGRATING-WITH-DOCTRINE.md (1)

166-166: ⚠️ Potential issue

Fix class name for DISTANCE custom function.
The example registers DISTANCE with the DistanceTest class, but it should reference Distance::class to correctly map the DQL function.

Proposed change:

-$configuration->addCustomStringFunction('DISTANCE', MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\DistanceTest::class);
+$configuration->addCustomStringFunction('DISTANCE', MartinGeorgiev\Doctrine\ORM\Query\AST\Functions\Distance::class);
src/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/Distance.php (1)

23-23: ⚠️ Potential issue

Add missing newline at end of file.
PHP CS Fixer is failing due to a missing newline at EOF. Please ensure the file ends with a single blank line.

Patch:

 }
+
📝 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.

}
 
tests/Unit/MartinGeorgiev/Doctrine/ORM/Query/AST/Functions/DistanceTest.php (2)

36-36: ⚠️ Potential issue

Add newline at the end of file.

The CI pipeline flagged that there's a missing newline at the end of the file.

}
+
📝 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.

}
 

19-26: ⚠️ Potential issue

Fix code style issues as flagged by the pipeline.

The CI pipeline flagged code style issues with the use of double quotes. According to the project's coding standards, single quotes should be used instead.

    protected function getExpectedSqlStatements(): array
    {
        return [
-            "SELECT (c0_.point <@> '(2.320041, 48.858889)') AS sclr_0 FROM ContainsPoints c0_",
-            "SELECT (c0_.point <@> c0_.point2) AS sclr_0 FROM ContainsPoints c0_",
-            "SELECT ('(1.0, 1.0)' <@> '(2.0, 2.0)') AS sclr_0 FROM ContainsPoints c0_",
+            'SELECT (c0_.point <@> \'(2.320041, 48.858889)\') AS sclr_0 FROM ContainsPoints c0_',
+            'SELECT (c0_.point <@> c0_.point2) AS sclr_0 FROM ContainsPoints c0_',
+            'SELECT (\'(1.0, 1.0)\' <@> \'(2.0, 2.0)\') AS sclr_0 FROM ContainsPoints c0_',
        ];
    }
📝 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.

    protected function getExpectedSqlStatements(): array
    {
        return [
            'SELECT (c0_.point <@> \'(2.320041, 48.858889)\') AS sclr_0 FROM ContainsPoints c0_',
            'SELECT (c0_.point <@> c0_.point2) AS sclr_0 FROM ContainsPoints c0_',
            'SELECT (\'(1.0, 1.0)\' <@> \'(2.0, 2.0)\') AS sclr_0 FROM ContainsPoints c0_',
        ];
    }

@coveralls
Copy link

coveralls commented Apr 24, 2025

Coverage Status

coverage: 93.789% (+0.02%) from 93.774%
when pulling 8a2c478 on seb-jean:feat/distance-function-2
into 88635d7 on martin-georgiev:main.

@seb-jean seb-jean force-pushed the feat/distance-function-2 branch from 8a8df52 to ad7f2d5 Compare April 24, 2025 06:41
coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 24, 2025
@seb-jean seb-jean force-pushed the feat/distance-function-2 branch from 355b5d2 to d6332bb Compare April 24, 2025 09:46
coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 24, 2025
@seb-jean
Copy link
Contributor Author

@martin-georgiev I have an error with scrutinizer:

  Command "run-tests-with-clover" is not defined.  
                                                   
  Did you mean this?                               
      run-unit-tests-with-clover                   
                                                   

@martin-georgiev
Copy link
Owner

@martin-georgiev I have an error with scrutinizer:

  Command "run-tests-with-clover" is not defined.  
                                                   
  Did you mean this?                               
      run-unit-tests-with-clover                   
                                                   

Oopsie... My bad when I introduced integration tests with a real PostgreSQL server earlier today. It's ok to ignore Scrutinizer failure in this particular instance.

@martin-georgiev martin-georgiev changed the title feat: add support for distance operator <@> feat: add support for distance operator <@> Apr 24, 2025
@martin-georgiev martin-georgiev enabled auto-merge (squash) April 24, 2025 11:09
@martin-georgiev martin-georgiev merged commit 8dbbf8c into martin-georgiev:main Apr 24, 2025
54 of 55 checks passed
@github-actions github-actions bot mentioned this pull request Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants