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
209 stars 21 forks source link

[bug]align with`RowMetaData#getColumnMetadata(String)` specification #244

Closed jchrys closed 8 months ago

jchrys commented 9 months ago

Describe the bug The specification states

@param name the name of the column.  
Column names are case-insensitive.  
When a get method contains several columns with same name, 
then the value of the >> first matching column << will be returned

However, the current behavior does not align with the specification.

To Reproduce Steps to reproduce the behavior:

connection.createStatement(SELECT 1 as t, 2 as t, 3 as t).execute()
                .flatMap(result -> Mono.from(result.map((row, metadata) -> row.get("t", Integer.class))))
                .doOnNext(System.out::println);

Prints 2

Expected behavior It should print 1

Screenshots

Additional context

mirromutth commented 9 months ago

It is caused by MySqlNames.binarySearch and Sort Comparator

The searching should do something like C++ std::lower_bound does.

The sorting should consider about index.

jchrys commented 8 months ago

@mirromutth I found that in our current implementation, enclosing a name in backticks causes case-sensitive mode. However, I'm considering removing this feature, since queries like SELECT `t` from (SELECT 1 as `T`) c work in a case-insensitive manner(+mariadb-r2dbc seems not support backtick). What are your thoughts on this?

mirromutth commented 8 months ago

Yep, we can just remove case-sensitive mode.

It was a feature for RowMetadata.contains

Implementations may allow escape characters to enforce a particular mode of comparison when querying for presence/absence of a column.

But only the javadoc for contains describes it, I don't see any description about this in the specification: https://r2dbc.io/spec/1.0.0.RELEASE/spec/html/#columnmetadata