sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Getting `setting the PIPES_AS_CONCAT sql_mode is unsupported`

Open marcustut opened this issue 2 years ago • 0 comments

I'm using a planetscale database, which uses vitess under the hood. it was working fine for a past few days, but when I try to build my code today this error occured:

setting the PIPES_AS_CONCAT sql_mode is unsupported

    sqlx::query_as!(
        ExpenseQueryResponse,
        r#"
select
    e. `id`,
    e. `name`,
    e. `amount`,
    e. `date`,
    ec. `name` as `category`,
    e. `comment`,
    group_concat(t. `name` separator ', ') as `tags`
from
    `expenses` e
    inner join `expenses_categories` ec on e. `expenses_categories_id` = ec. `id`
    left join `tags_expenses` te on e. `id` = te. `expenses_id`
    left join `tags` t on t. `id` = te. `tags_id`
group by
    e. `id`,
    e. `name`,
    e. `amount`,
    e. `date`,
    ec. `name`,
    e. `comment`
order by
    e. `date` desc,
    e. `id` desc;
        "#,
    )
    .fetch_all(db)
    .await

I searched the code and I found that when sqlx connect to the db, it tries to set certain sql_mode on the connection. It can be found in sqlx-core/src/mysql/options/connect.rs

I queried for the current sql_mode set on the db, this is the result:

marcustut/|âš  main âš |> show variables like '%sql_mode%';
+---------------+-----------------------------------------------------------------------------------------------------------------------+
| Variable_name | Value                                                                                                                 |
+---------------+-----------------------------------------------------------------------------------------------------------------------+
| sql_mode      | ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION |
+---------------+-----------------------------------------------------------------------------------------------------------------------+

It can be clearly see that PIPES_AS_CONCAT is not set. So I suspect that planetscale doesn't allow setting sql_mode but sqlx is trying to set the variable. Is there a way to override this behavior?

marcustut avatar Aug 10 '22 04:08 marcustut

If this is something that only just started happening, it sounds like a regression in Planetside.

abonander avatar Aug 10 '22 07:08 abonander

If this is something that only just started happening, it sounds like a regression in Planetside.

Hmm that's weird. I've opened a discussion over there at the moment. https://github.com/planetscale/discussion/discussions/248

marcustut avatar Aug 10 '22 08:08 marcustut

~Definitely us and not a problem with sqlx. I was able to reproduce this with a specific Vitess version and we're going to look into it. This issue can be closed. :-)~

See below.

fooforge avatar Aug 10 '22 11:08 fooforge

Definitely us and not a problem with sqlx. I was able to reproduce this with a specific Vitess version and we're going to look into it. This issue can be closed. :-)

More details here from the PlanetScale side. The issue here is more nuanced than "this is a regression" because this breaking is something we actually consider a bug fix. The change was made specifically here:

https://github.com/vitessio/vitess/pull/10163/files#diff-9d2575386e98a47276e4d5c75e73cdefec7078a95ea4e159b50806a8c3099c1bR101

Vitess has the notion of certain SQL mode flags that are unsupported to be changed by the end user. PIPES_AS_CONCAT is included in that set of flags, but before that fix we didn't properly validate and error when clients end up setting unsupported flags.

Before this fix, what this can result in, is wrong results being returned. Vitess itself also parses the SQL queries and for example in the case of sharded setups has to aggregate and combine results from different MySQL instances. This means that we also have an SQL evaluation engine in Vitess.

That parser and evaluation engine only deal with the standard MySQL dialect. Options like PIPES_AS_CONCAT or ANSI_QUOTES change the actual parsing of SQL queries which is not something Vitess supports. This means that before when we didn't explicitly error on this, you would get silently potentially wrong results returned whenever an || was used in a query (depending on details like sharding and other factors of where this query was evaluated, in Vitess, MySQL or in both).

In order the avoid this silent broken behavior, what we needed to do was explicitly blocking these changes since we don't have any current plans to support modes like ANSI_QUOTES or PIPES_AS_CONCAT.

Is it possible that sqlx is willing to consider not setting / needing PIPES_AS_CONCAT? And instead for example use the CONCAT() function when two strings need to be concatenated?

dbussink avatar Aug 10 '22 12:08 dbussink

We chose to enable PIPES_AS_CONCAT by default as an ergonomic choice, and to get MySQL to behave at least somewhat more closely to SQLite and PostgreSQL. While || is generally understood to be the boolean OR operator, in the SQL standard it represents string concatenation, which makes this an annoying deviation from the standard from our perspective.

Similarly, setting PIPES_AS_CONCAT by default gives more consistent behavior when using the runtime-polymorphic Any driver to execute queries for applications that want to be database-agnostic.

I'd support a toggle in MySqlConnectOptions to turn it on or off, but changing the default would be a breaking behavior change for SQLx.

abonander avatar Aug 10 '22 23:08 abonander

We chose to enable PIPES_AS_CONCAT by default as an ergonomic choice, and to get MySQL to behave at least somewhat more closely to SQLite and PostgreSQL. While || is generally understood to be the boolean OR operator, in the SQL standard it represents string concatenation, which makes this an annoying deviation from the standard from our perspective.

Similarly, setting PIPES_AS_CONCAT by default gives more consistent behavior when using the runtime-polymorphic Any driver to execute queries for applications that want to be database-agnostic.

I'd support a toggle in MySqlConnectOptions to turn it on or off, but changing the default would be a breaking behavior change for SQLx.

Sounds great! I do agree on not disabling PIPES_AS_CONCAT as default since there might be application out there using it with sqlx at the moment.

marcustut avatar Aug 11 '22 00:08 marcustut