Ability to check exists through tables with "through" key.#30
Ability to check exists through tables with "through" key.#30RaitzeR wants to merge 1 commit intotimgws:masterfrom
Conversation
Checking exists changed to join, since it's insanely much faster (from tens or hundreds of seconds to milliseconds). Checking not exists is as before. Through works with unlimited number of pivot tables. Example: 'join2through' => array( 'from_table' => 'master2', 'from_col' => 'm2_col', 'to_table' => 'subtable2', 'to_col' => 's2_col', 'to_value_column' => 's2_value', 'not_exists' => true, 'through' => array( 'from_table' => 'subtable2', 'from_col' => 's2_col', 'to_table' => 'subtable3', 'to_col' => 's3_col', 'to_value_column' => 's3_value', ) ), Keep adding "through" if there's more tables in between.
|
The failed tests comes from the fact that the tests check for exists, instead of how it's done now. I didn't want to rewrite the tests, in case you're not going to accept this pull request. |
timgws
left a comment
timgws
left a comment
There was a problem hiding this comment.
Generally, this is looking good. There are a few changes that I would prefer to be performed before accepting the changes.
Also, please ensure that the tests pass (of course!)
🎉 Thanks for your first contribution to this project!
| $condition, | ||
| $not | ||
| ); | ||
| if ( $not ) { |
There was a problem hiding this comment.
One thing that I am cautious of is that this code looks like it will change the behavior for other users of this class.
Can we separate out the code inside the if() {...} block into two separate functions to ensure that the length of this function (buildSubclauseQuery) is not too long?
| } else { | ||
| // Create a join clause to join to the other table, and find results matching the criteria | ||
|
|
||
| $query = $query->join( $subclause["to_table"], $subclause['to_table'] . '.' . $subclause['to_col'], '=', $subclause['from_table'] . '.' . $subclause['from_col'] ); |
There was a problem hiding this comment.
Please try to keep lines < 80 characters long. This line of code wraps in my browser.
| $this->assertEquals(28, $bindings[1]->day); | ||
| } | ||
|
|
||
| public function testJoinNotExistsInThrough() |
There was a problem hiding this comment.
Also, with this project, we are trying to stick to PSR-2 styling (https://www.php-fig.org/psr/psr-2/). Could you please ensure that you are using spaces and not tabs to format code?
2 participants
Checking exists changed to join, since it's insanely much faster (from tens or hundreds of seconds to milliseconds). Checking not exists is as before.
Through works with unlimited number of pivot tables.
Example:
'join2through' => array(
'from_table' => 'master2',
'from_col' => 'm2_col',
'to_table' => 'subtable2',
'to_col' => 's2_col',
'to_value_column' => 's2_value',
'not_exists' => true,
'through' => array(
'from_table' => 'subtable2',
'from_col' => 's2_col',
'to_table' => 'subtable3',
'to_col' => 's3_col',
'to_value_column' => 's3_value',
)
),
Keep adding "through" if there's more tables in between.