feat: extend build methods to accept DbDriverInterface and add MySQL alias and subquery join tests #29
Conversation
…alias and subquery join tests
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Incorrect 'Throw' keyword capitalization ▹ view | |
 |
Incorrect NULL Value Handling ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| src/Interface/UpdateBuilderInterface.php | ✅ |
| src/DeleteQuery.php | ✅ |
| src/InsertSelectQuery.php | ✅ |
| src/InsertQuery.php | ✅ |
| src/InsertBulkQuery.php | ✅ |
| src/UpdateQuery.php | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| if (is_null($dbDriver)) { | ||
| Throw new InvalidArgumentException('You must specify a DbDriverInterface to use Query join tables'); | ||
| } |
There was a problem hiding this comment.
Incorrect 'Throw' keyword capitalization 
Tell me more
What is the issue?
Inconsistent capitalization in the 'Throw' keyword - it should be lowercase 'throw' as per PHP standards.
Why this matters
While the error is caught correctly, using inconsistent capitalization violates PHP coding standards and makes the code less maintainable.
Suggested change ∙ Feature Preview
Replace the capitalized 'Throw' with lowercase 'throw':
if (is_null($dbDriver)) {
throw new InvalidArgumentException('You must specify a DbDriverInterface to use Query join tables');
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| if (!is_numeric($value)) { | ||
| $value = $dbHelper?->delimiterField($value) ?? "'{$value}'"; | ||
| $value = $dbDriverOrHelper?->delimiterField($value) ?? "'{$value}'"; |
There was a problem hiding this comment.
Incorrect NULL Value Handling 
Tell me more
What is the issue?
The code assumes non-numeric values need field delimiters or quotes, which may not handle null values correctly.
Why this matters
NULL values could be incorrectly wrapped in quotes, causing them to be treated as strings rather than NULL in the database.
Suggested change ∙ Feature Preview
Add a specific check for NULL values before the numeric check:
if ($value === null) {
$value = 'NULL';
} elseif (!is_numeric($value)) {
$value = $dbDriverOrHelper?->delimiterField($value) ?? "'{$value}'";
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
2 participants
The pull request introduces the following changes:
DbDriverInterface.Please review the changes and provide feedback.
Description by Korbit AI
What change is being made?
Extend the build methods across various query classes to accept both
DbDriverInterfaceandDbFunctionsInterface, and add tests for MySQL alias and subquery joins in theUpdateQueryTest.Why are these changes being made?
The changes increase compatibility and flexibility by supporting both interfaces for providing database helpers. The added MySQL tests ensure that aliasing and subquery joins work correctly, addressing advanced query scenarios that can arise in complex database interactions.