Active Record $this->db->join bug

[eluser]Shiro[/eluser]
Today I found out a bug in Active Record,
I am using mysql as my db.

CI tread the join first parameter always table id link with another table id
E.g

Code:

$this->db->join('comments', 'comments.id = blogs.id');

in this case, shouldn't be a problem, because CI will auto append ` to the condition
become:

JOIN comments ON `comments`.`id` = `blogs`.`id`

but today I got a case, which is I don't have any id linking with the table, (For a reason I do that).

Code:

$this->db->join('marketing', 'marketing.status = "active", 'left');

the output become

JOIN marketing ON `marketing`.`status` = "`active"`

which give me database error.

now I need to do

Code:

$this->db->join('marketing', 'marketing.status = marketing.status AND marketing.status = "active", 'left');

JOIN marketing ON `marketing`.`status` = `marketing`.`status` AND `marketing`.`status` = "active"

then this will solve my problem,
I think CI should detect first b4 add the `` or add another parameter at the end, allow user escape it



#2
Bug 2
I would like to code like this for better viewing, I put the AND condition next line

Code:

$this->db->join('comments', 'comments.id = blogs.id
                             AND comments.status = "active"');

in this case, it will have another bug. The outcome

JOIN comments ON `comments`.`id` = `blogs`.`id
` AND `comments`.`status` = "active"

as you can see, it become db error

`id <next line>
`

[eluser]WanWizard[/eluser]
The two joins you give as an example are not the same.

The first one doesn't contain a field to join on, therefore the result will be the product of the two tables (every record joined with every record from the second table). Altough it is a valid query, I don't think AR should consider this syntax as valid.

The second one can be easily fixed in the join() method with something like:

Code:

$cond = str_replace(array("\r\n", "\r", "\n", "\t"), ' ', $cond);

before the preg_match()...

[eluser]Shiro[/eluser]
ya, there are 2 bugs I am mentioned about it.

In the first case, for my situation I got one sql string that really don't have a contain field to join, for some reason, I only need the table for the condition purpose.
What I suggest is there should be a parameter for it, just in case there are a need for my case, just have an extra parameter to resolve it.

thx, for the second part solution. It will be good to mention which line of which file to include it

[eluser]WanWizard[/eluser]
Look in system/database/DB_active_rec.php for the function join(). About 20 lines in there is a preg_match() statement on $cond. Insert the line before that statement...

[eluser]WanWizard[/eluser]
We use this to work around the join condition limitation:

Code:

$this->db->select('cms_workflow_transitions.*');
$this->db->where('cms_workflow_arcs.wf_id', $workflow['wf_id']);
$this->db->where('cms_workflow_arcs.wf_places_id', $place['wf_places_id']);
$this->db->where('cms_workflow_arcs.wf_arcs_direction', 'I');
// work around a AR JOIN limitation
$this->db->ar_join[] = 'LEFT JOIN `cms_workflow_transitions` ON (`cms_workflow_transitions`.`wf_id` = `cms_workflow_arcs`.`wf_id` AND `cms_workflow_transitions`.`wf_transitions_id`= `cms_workflow_arcs`.`wf_transitions_id`)';
$query = $this->db->get('cms_workflow_arcs');

Don't forget to escape your join clause properly, CI doesn't check it anymore!

[eluser]WanWizard[/eluser]
SInce my previous post we've come across the need to extend the DB_active_rec.php library, so I decided to rewrite the join() method to get rid of this dirty hack.

For those who are interested:

Code:

/**
     * Join
     *
     * Generates the JOIN portion of the query
     *
     * Our version supports a more flexible condition syntax than the CI version
     *
     * @access    public
     * @param    string
     * @param    string    the join condition
     * @param    string    the type of join
     * @return    object
     */
    function join($table, $cond, $type = '')
    {
        if ($type != '')
        {
            $type = strtoupper(trim($type));

            if ( ! in_array($type, array('LEFT', 'RIGHT', 'OUTER', 'INNER', 'LEFT OUTER', 'RIGHT OUTER')))
            {
                $type = '';
            }
            else
            {
                $type .= ' ';
            }
        }

        // Extract any aliases that might exist.  We use this information
        // in the _protect_identifiers to know whether to add a table prefix
        $this->_track_aliases($table);

        // Strip apart the condition and protect the identifiers
        $keywords = preg_split( "/[\s,]*(\\\"[^\\\"]+\\\")[\s,]*|[\s,]+/", $cond, 0, PREG_SPLIT_DELIM_CAPTURE );

        // reset the condition
        $cond = '';

        // process the keywords
        foreach ( $keywords as $keyword )
        {
            // skip quoted strings and everything uppercase (assume they are sql keywords)
            if ( $keyword{0} != '"' && $keyword{0} != '\'' && $keyword != strtoupper($keyword) )
            {
                // protect the identifiers
                $keyword = $this->_protect_identifiers($keyword);
            }

            // add the keyword to the condition
            $cond .= ' ' . $keyword;
        }

        // Assemble the JOIN statement
        $join = $type.'JOIN '.$this->_protect_identifiers($table, TRUE, NULL, FALSE).' ON '.$cond;

        $this->ar_join[] = $join;
        if ($this->ar_caching === TRUE)
        {
            $this->ar_cache_join[] = $join;
            $this->ar_cache_exists[] = 'join';
        }

        return $this;
    }

[eluser]parvus[/eluser]
this is still a bug after 3 years. thanks for the sensitive dev-team!!!

query:

Code:

$this -> db -> join('predict', "predict.id_predict = portion.id_predict AND event.id_event = {$id_transaction}", 'left');

result:

LEFT JOIN `predict` ON `predict`.`id_predict` = `portion`.`id_predict` AND event.id_event = 5

BUT IT SHOULD BE ->

LEFT JOIN `predict` ON (`predict`.`id_predict` = `portion`.`id_predict` AND `event`.`id_event` = 5)

IS THERE ANY PLANS TO FIXING IT IN THE NEXT 25 YEARS ?

[eluser]Aken[/eluser]
Submit an issue on Github, without the attitude, please.

[eluser]jmadsen[/eluser]
[quote author="Aken" date="1360021867"]Submit an issue on Github, without the attitude, please.[/quote]

Or better yet, a Pull Request with the tested code.

It's open source - you are expected to contribute, not just demand & take

[eluser]Aken[/eluser]
I checked after replying, it's been fixed in 3.0 already.