Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug]align withRowMetaData#getColumnMetadata(String) specification #244

Closed
jchrys opened this issue Feb 22, 2024 · 3 comments · Fixed by #257
Closed

[bug]align withRowMetaData#getColumnMetadata(String) specification #244

jchrys opened this issue Feb 22, 2024 · 3 comments · Fixed by #257
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jchrys
Copy link
Collaborator

jchrys commented Feb 22, 2024

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

@jchrys jchrys added the bug Something isn't working label Feb 22, 2024
@jchrys jchrys self-assigned this Feb 22, 2024
@jchrys jchrys added this to the 1.1.3 milestone Feb 22, 2024
@jchrys jchrys assigned jchrys and unassigned jchrys Feb 22, 2024
@mirromutth
Copy link
Contributor

mirromutth commented Feb 23, 2024

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
Copy link
Collaborator Author

jchrys commented Mar 5, 2024

@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
Copy link
Contributor

mirromutth commented Mar 6, 2024

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

@jchrys jchrys linked a pull request Mar 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants