-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: extend build methods to accept DbDriverInterface and add MySQL alias and subquery join tests #29
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.
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
feat: extend build methods to accept DbDriverInterface and add MySQL alias and subquery join tests #29
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ | |
|
|
||
| namespace ByJG\MicroOrm; | ||
|
|
||
| use ByJG\AnyDataset\Db\DbDriverInterface; | ||
| use ByJG\AnyDataset\Db\DbFunctionsInterface; | ||
| use ByJG\AnyDataset\Db\SqlStatement; | ||
| use ByJG\MicroOrm\Exception\InvalidArgumentException; | ||
| use ByJG\MicroOrm\Interface\QueryBuilderInterface; | ||
| use ByJG\MicroOrm\Literal\Literal; | ||
|
|
@@ -65,34 +67,58 @@ public function setLiteral(string $field, mixed $value): UpdateQuery | |
| return $this; | ||
| } | ||
|
|
||
| protected function getJoinTables(DbFunctionsInterface $dbHelper = null): array | ||
| protected function getJoinTables(DbFunctionsInterface|DbDriverInterface $dbDriverOrHelper = null): array | ||
| { | ||
| $dbDriver = null; | ||
| if ($dbDriverOrHelper instanceof DbDriverInterface) { | ||
| $dbDriver = $dbDriverOrHelper; | ||
| $dbHelper = $dbDriverOrHelper->getDbHelper(); | ||
| } else { | ||
| $dbHelper = $dbDriverOrHelper; | ||
| } | ||
|
|
||
| if (is_null($dbHelper)) { | ||
| if (!empty($this->joinTables)) { | ||
| throw new InvalidArgumentException('You must specify a DbFunctionsInterface to use join tables'); | ||
| } | ||
| return ['sql' => '', 'position' => 'before_set']; | ||
| } | ||
| foreach ($this->joinTables as $key => $joinTable) { | ||
| if ($this->joinTables[$key]['table'] instanceof QueryBasic) { | ||
| if (is_null($dbDriver)) { | ||
| Throw new InvalidArgumentException('You must specify a DbDriverInterface to use Query join tables'); | ||
| } | ||
|
Comment on lines
+88
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect 'Throw' keyword capitalization
Tell me moreWhat is the issue?Inconsistent capitalization in the 'Throw' keyword - it should be lowercase 'throw' as per PHP standards. Why this mattersWhile the error is caught correctly, using inconsistent capitalization violates PHP coding standards and makes the code less maintainable. Suggested change ∙ Feature PreviewReplace 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. |
||
| $this->joinTables[$key]['table'] = new SqlStatement($this->joinTables[$key]['table']->build($dbDriver)->getSql()); | ||
| } | ||
| } | ||
|
|
||
| return $dbHelper->getJoinTablesUpdate($this->joinTables); | ||
| } | ||
|
|
||
| public function join(string $table, string $joinCondition): UpdateQuery | ||
| public function join(QueryBasic|string $table, string $joinCondition, ?string $alias = null): UpdateQuery | ||
| { | ||
| $this->joinTables[] = ["table" => $table, "condition" => $joinCondition]; | ||
| $this->joinTables[] = ["table" => $table, "condition" => $joinCondition, "alias" => $alias]; | ||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * @param DbFunctionsInterface|null $dbHelper | ||
| * @param DbDriverInterface|DbFunctionsInterface|null $dbDriverOrHelper | ||
| * @return SqlObject | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| public function build(DbFunctionsInterface $dbHelper = null): SqlObject | ||
| public function build(DbFunctionsInterface|DbDriverInterface|null $dbDriverOrHelper = null): SqlObject | ||
| { | ||
| if (empty($this->set)) { | ||
| throw new InvalidArgumentException('You must specify the fields for update'); | ||
| } | ||
|
|
||
| $dbDriver = null; | ||
| if ($dbDriverOrHelper instanceof DbDriverInterface) { | ||
| $dbHelper = $dbDriverOrHelper->getDbHelper(); | ||
| $dbDriver = $dbDriverOrHelper; | ||
| } else { | ||
| $dbHelper = $dbDriverOrHelper; | ||
| } | ||
|
|
||
| $fieldsStr = []; | ||
| $params = []; | ||
|
|
@@ -120,7 +146,7 @@ public function build(DbFunctionsInterface $dbHelper = null): SqlObject | |
| $tableName = $dbHelper->delimiterTable($tableName); | ||
| } | ||
|
|
||
| $joinTables = $this->getJoinTables($dbHelper); | ||
| $joinTables = $this->getJoinTables($dbDriverOrHelper); | ||
| $joinBeforeSet = $joinTables['position'] === 'before_set' ? $joinTables['sql'] : ''; | ||
| $joinAfterSet = $joinTables['position'] === 'after_set' ? $joinTables['sql'] : ''; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.