October: SQL syntax error when using concat and parentheses in backend list

[leoquijano]

Hi,

I'm trying to use the following column definitions for a backend model in a PGSQL site:

columns:
id:
label: ID
searchable: true
amount_due:
label: Amount Due
currency_code:
label: Currency
description:
label: Description
payments:
label: Payments
relation: payments
select: 'concat("#", id, " - ", amount, "(", currency_code, ")")'
...

Please note the use case, I'd like to display the currency_code using parentheses. Like this: 500 (USD). However, the SQL parser is not getting it correctly. This is what the above select generates:

"#" || id || " - " || amount || "(" || currency_code || "")

I noticed that this in an old issue, as referenced in #883. However, testing the above string in the parseConcat function, directly, is removing my last closing parentheses:

public function parseConcat($sql)
{
    return preg_replace_callback('/(?:group_)?concat\(([^)]+)\)(?R)?/i', function($matches){
        if (!isset($matches[1])) {
            return $matches[0];
        }

        // This is a group_concat() so ignore it
        if (strpos($matches[0], 'group_') === 0) {
            return $matches[0];
        }

        $concatFields = array_map('trim', explode(',', $matches[1]));

        switch ($this->driver) {
            default:
            case 'mysql':
                return $matches[0];

            case 'pgsql':
            case 'postgis':
            case 'sqlite':
                return implode(' || ', $concatFields);
        }
    }, $sql);
}

I tested the above function using phptester.net, and yes, it is eating the last closing ")".

[Patroklo]

Why you don't make in this case a "false" attribute called whatever and return that data from the php model?

Like:

    public function getWhateverAttribute()
    {
        return '('.$this->getAttribute('id').') - '.$this->getAttribute('amount')....;
    }

It should work for lists and it's sql agnostic! 8D

[leoquijano]

It is a good workaround. I actually replaced CONCAT with ||.

But I think the problem is more general: the parser is eating the last ")". It's not concat-specific.

[Patroklo]

edit: everything here it's being tried with mysql, maybe the problem it's only with the postgre driver.

edit 2: if you still have problems after trying the non string solution, please, post your exact error.

A question about your defined concat... I have just seen that you are using an string instead of a function there. Why is that?

edit: I made a film script about this because I'm dumb. It doesn't work because the regex that extracts the data inside the concat only gets the first ending bracket ) and ignores the rest. Since in mysql october uses the concat without regex extraction, it works fine, but in the rest of db drivers it will fail.

[leoquijano]

1. I'm using PostgreSQL, not MySQL

2. I did run a test on the regular expression (parseConcat function) and it doesn't pick it right. Try it yourself:

   /(?:group_)?concat\(([^)]+)\)(?R)?/i
   

   Either October CMS supports SQL or it doesn't. Using it as a function is valid SQL, so why wouldn't October be able to handle it?

[Patroklo]

Problem it's that your example is working for me (at least my version, probably I have something different). What is exactly your error?

[leoquijano]

Take a look:

https://regex101.com/r/gGbWZi/1

[Patroklo]

All right, now I see the problem and why it works only for MySQL 8D It's using the concat without regex and the non mysql drivers are using the other one that's the reason I didn't have any errors. It seems to be a regex thingie, I'll try to remember my very very old times using those.

In fact it only gets the first parenthesis, concat("#", id, " - ", amount,")" ,"(", currency_code, ")", ")") will get the first one.

[Patroklo]

Le fu~ this has a lot of inside complications and I'm not a skilled regex developer. Right now I have this that somewhat (aka, have tried only two examples) works ((?:group_)?concat)?(\(((?:[^\(\)]++|(?R))*)\)) but it's not usable because probably will fail with another string.

[leoquijano]

It would probably be better to use an actual SQL parser, as regexps have a limit when strings get more complex.

[Patroklo]

Well... I know this sql parser, but it seems overkill here... https://github.com/greenlion/PHP-SQL-Parser

[leoquijano]

Yeah, and even then, it's only for MySQL.

Why was October doing that parseConcat, anyway?

[daftspunk]

This will be fixed in October Build 381+