ActiveRecord Select - constant string with a comma changes

[uzsolt]

I want concat two fields with comma:

$this->db->select("CONCAT(nev,',',id_tanar)",false)->from("tanar");

And when I use $this->db->get() and after that $this->db->last_query(), it shows:

SELECT CONCAT(nev, ', ', id_tanar) FROM (`tanar`)

(check the plus space character near after comma)

How can I disable this behavior? I don't want extra space...

[zwilias]

You can use something like this instead:

$this->db->select(array('CONCAT(nev,', "','", 'id_tanar)', false)->from("tanar");

Which is a hack, but will do what you want, I think.

The culprit here is the somewhat shortsighted $select = explode(',', $select); on line 94 of DB_active_rec.php. However, making that a little more intelligent is somewhat harder than one would expect. The goal is to obtain all fields that are being selected, however, when you're selecting something like CONCAT(nev,',',id_tanar), it will split that into CONCAT(nev, ', ' and id_tanar), which are obviously not fieldnames.

One option would be to use a regex, and only split outside of brackets.

[zwilias]

Thinking about this some more, there are a fair number of cases where the current code would produce unwanted results. Including, but not limited to, various scalar functions with multiple arguments, case constructs containing a comma, whatever really.

One option I can think of would be to create a select_field($select, $alias) function which wouldn't do much post-processing but simple add the field and its alias to the internal $this->ar_select array.

In fact, it's the only non-convoluted option I can think of that wouldn't rely on actually parsing the input (as well as keeping in mind the various databases around).

[uzsolt]

I've began write a simple replacement and I've wrote similar as you said: a function which "says" to library that this is a table/field (select_field($table,$field,$alias="")). I know this will need more typing from user but I think it's safer and I hope it has fewer bugs ;)

[zwilias]

I'd think a more "futureproof" version would leave the $table parameter out. In fact, it would probably be a good idea to make the $alias param mandatory, since this would be (mainly) used for less-trivial field-expressions, ranging from ',' to subqueries/case blocks/functions which would all need an $alias to be useful.

[uzsolt]

Yes, this isn't trivial question. What is the most important:

• simple coding, less typing but the special cases maybe don't work
• safer solution, special cases works too, but complicated coding, more typing.

Maybe there can be two ways: simplier functions but sometimes doesn't work well and specialized functions wich work everytime but needs more coding but works in all cases.

[zwilias]

I don't have a CodeIgniter install around to play with, but perhaps something like the following could work?

    public function select_field($field, $alias = '', $escape = NULL)
    {
        $field = trim($field);

        if ($alias != '')
        {
            $field .= ' AS '.$alias;
        }

        $this->ar_select[] = $field;
        $this->ar_no_escape[] = $escape;

        if ($this->ar_caching === TRUE)
        {
            $this->ar_cache_select[] = $field;
            $this->ar_cache_exists[] = 'select';
            $this->ar_cache_no_escape[] = $escape;
        }

        return $this;
    }

[uzsolt]

I've similar function in my library ;)

[zwilias]

Oh, please test it, make it pretty, make sure it conforms to the styleguides and make a pull request :)

[AkenRoberts]

@uzsolt @zwilias @alexbilbie

First, the explode is only performed if the select($select) parameter is a string. You can pass it an array and this problem is avoided entirely, if you need a solution right away.

I've tried a solution that involves temporarily replacing commas inside of functions before performing the explode, then returning them afterwards. It works properly in a couple test scenarios, even without specifying the FALSE escape parameter. You can view the modified select() here: https://gist.github.com/4371198

The regular expression used to find the functions could be improved to prevent a couple bugs that I can think of (such as using a ) in your function). But this might be a good start to finding a possible solution. I'm not diverse enough to test multiple scenarios, DBs, etc, so give this a shot and let me know what you find.

[AkenRoberts]

Also, sorry @alexbilbie I'm not sure why I tagged you in this issue thread. :frog:

[jim-parry]

Any traction on this? Is this a bug? worth fixing?

[AkenRoberts]

Definitely a bug still, definitely worth fixing.

[carlotrimarchi]

I'm still experiencing this but in Codeigniter 2. Has this been fixed in Codeigniter 3?

[narfbg]

Issue is still open, so no.

I think I remember making it not add spaces, but it does try to escape identifiers and everything separated by commas is treated as one.

[ngekoding]

I found the issue in 2021. I want to remove a trailing comma, but because CI add extra space, so the code doesn't work.

$this->db->select("TRIM(TRAILING ',' FROM name)) AS username")

Any solution?

[ngekoding]

Okay, finally for temporary solution we can directly change _compile_select method in system/database/DB_query_builder.php.

From:

$sql .= implode(', ', $this->qb_select);

To:

$sql .= implode(',', $this->qb_select); // without space

Hope this can help everyone who have the same problem.