asyncer-io / r2dbc-mysql

Reactive Relational Database Connectivity for MySQL. The official successor to mirromutth/r2dbc-mysql(dev.miku:r2dbc-mysql).
https://r2dbc.io
Apache License 2.0
195 stars 21 forks source link

[bug] Reserved column name can't be used when selected by native query #170

Closed ytkachenkokinoa closed 8 months ago

ytkachenkokinoa commented 8 months ago

io.asyncer.r2dbc.mysql.ColumnNameSet int findIndex(String name) { return MySqlNames.nameSearch(this.sortedNames, name); } Returns -1 when entity columns described like @Column("``system``") with apostrophes (here and further apostrophes are doubled just due to not to break formatting) and ReactiveCrudRepository has method annotated for use native query like this @Query(" SELECT ... ``system`` as '``system``' ... That's why Entity could not be mapped if reserved word used as a column name in both ways: as a standart entity for common ReactiveCrudRepository methods, and as an entity which used by native queries methods

Steps to reproduce the behavior:

  1. Using springboot data, create app with ReactiveCrudRepository.
  2. Create table with column named with reserved word like "system".
  3. Create entity that uses that table using @Table and @Column("``system``") annotations.
  4. Try to save via proposed by lib "save" method.
  5. Try to get entity by native query.

Expected behavior Entity could be saved and got via ReactiveCrudRepository

Actual behavior Column name '``system``' does not exist when @Column("``system``") OR bad SQL grammar [INSERT INTO ... (..., system when @Column("system")

mirromutth commented 8 months ago

Hi,

If the table column name is system, it should be code>@Column("\`system\`")</code and is case-sensitive. So, if your data column is named SYSTEM, it should be code>@Column("\`SYSTEM\`")</code.

Additionally, it's case-sensitive because the backtick ` means "exact match". See also r2dbc-mssql, r2dbc-postgresql does.

ytkachenkokinoa commented 8 months ago

Thank you for your fast response. Actually I guess problem is not in your code. I tried to used one of two query parts

    @Query("""                          
            SELECT
                se.`id`,
                1 as 'system',

and

    @Query("""                          
            SELECT
                se.`id`,
                1 as '`system`',

Second one can't be find. But with first one I've doublechecked with debugger and found out that RowDocument contained value and it had lost in the converters processing via Spring data, because BasicRelationalPersistentProperty that represented entity has columnName value "`system`", but RowDocument has column named "system". That is why Spring data fails with mapping that to entity.

mirromutth commented 8 months ago

I think it should be:

@Query("""                          
            SELECT
                se.`id`,
                1 as `system`,
ytkachenkokinoa commented 8 months ago

RowDocument in that case have the same name of field "system". Just checked. And entity still ignores it

mirromutth commented 8 months ago

I see.

Well, I believe it should not be a driver issue.

When the native SQL statement is executed, spring-data-r2dbc knows columns as id and system. Then, the RowDocument makes the Map like {"id":1, "system":1}.

So if the system property has been annotated as code>@Column("\`system\`")</code, RowDocument will not try to map system property (in Map) as `system` property (in annotation).

ytkachenkokinoa commented 8 months ago

Yeah. And for saving entity, system property in annotation (without apostrophes) leads to wrong Insert/update statement. As a fix could be done wrapping of all column names with apostrophes, which is totally OK for MySQL.

mirromutth commented 8 months ago

That is also not a driver issue. Drivers should not silently rewrite statements passed from ORM/QueryBuilder, which may cause more problems. Usually this kind of feature should be implemented in ORM/QueryBuilder.

ytkachenkokinoa commented 8 months ago

Is that not the driver responsibility to write Insert/Update statements according to Database requerements? E.g. PostgreSQL has different rules of escaping. So @Column("anything") annotation should be escaped according to DB requerements. What the reason though of usage Spring data if to migrate your app to other DB you need not to just switch driver?

Anyway. Using this driver what the way to annotate entity or name returned column to use it for both: getting by query and saving by repository common methods?

mirromutth commented 8 months ago

Yes, it is. Or you can try to use a JDBC native driver and see how it works.

Driver layer does not, and should never care the statement comes from an entity repository or a query builder. It is driver, not DAO. Which means drivers does not know "What is @Column", "What is entity".

For example, Spring Data JPA implements this kind as spring.jpa.properties.hibernate.dialect. AFAIK, it is not providered by mysql-connector-j.

See also https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java#L1000

ytkachenkokinoa commented 8 months ago

I mean if DAO sends column name just as system without any apostrophes and special signs (that corresponds @Column("system") annotation) to be used inside INSERT statement, that is a responsibility of driver to escape or not reserved/special words. But driver produces INSERT INTO foo (id, name, system) VALUES (1, 'Bar', 1) instead of INSERT INTO foo (id, name, `system`) VALUES (1, 'Bar', 1)

mirromutth commented 8 months ago

I knew.

However, this feature is not provided by any other driver. That's what I'm saying, it's not a driver issue.

Another example: SELECT CURRENT_TIMESTAMP FROM user, driver can not know what is user actually want: CURRENT_TIMESTAMP expression, or `CURRENT_TIMESTAMP` column in user?