Fix a bug in the join generator when same table is present multiple times #11690
Conversation
tsteur
added
Bug
…s when removing an item
| 'log_visit', | ||
| array('table' => 'log_conversion', 'joinOn' => 'log_visit.idvisit2 = log_conversion.idvisit2'), | ||
| 'log_conversion' | ||
| )); |
There was a problem hiding this comment.
Haven't had a closer look at the usage of the JoinGenerator, but the result might still be invalid if the order changes like this
$generator = $this->generate(array(
'log_visit',
'log_conversion',
array('table' => 'log_conversion', 'joinOn' => 'log_visit.idvisit2 = log_conversion.idvisit2')
));
That would still result in a duplicate LEFT JOIN. Is that something that could happen, or can we ignore that?
There was a problem hiding this comment.
I know, I tried to implement it but won't be trivial. We could rather create a new issue for it. Eg when array is like
array(
'log_visit',
'log_conversion',
'log_link_visit_action',
array('table' => 'log_conversion', 'joinOn' => 'log_link_visit_action.idvisit2 = log_conversion.idvisit2'),Then I can't simply use the join condition of the second log_conversion array on the first log_conversion table since it might reference a table that is only joined later. Also I cannot remove the first log_conversion since it might be used in a join before the other log_conversion comes.
There was a problem hiding this comment.
FYI: Just pushed an update to instead throw an exception when this happens with detailed information where it happens and why. As solving it won't be trivial and is easier to instead just re-structure the array order for now if / when needed
There was a problem hiding this comment.
+1 merging this fix now and creating a new issue for the rest.
|
Pushed some more changes re review feedback |
See the test below. Before this fix it would generate a join like
instead we want to have only
Because
log _conversionwas already joined we don't need to join it again on different logic because there was no specific join logic specified forlog_conversionso we can re-use an existing join and prevent an error message that eglog_conversion is not unique.