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

Implement RowMetadata.getColumnNames() #73

Closed
mp911de opened this issue Apr 29, 2019 · 5 comments
Closed

Implement RowMetadata.getColumnNames() #73

mp911de opened this issue Apr 29, 2019 · 5 comments
Labels
type: spec A change mandated by the specification
Milestone

Comments

@mp911de
Copy link
Member

mp911de commented Apr 29, 2019

See r2dbc/r2dbc-spi#58

@mp911de mp911de added the type: spec A change mandated by the specification label Apr 29, 2019
@mp911de mp911de added this to the 1.0.0.M8 milestone Apr 29, 2019
@katzyn
Copy link

katzyn commented Apr 29, 2019

SELECT N AS L FROM TEST;

Please note that JDBC historically has two similar methods in ResultSetMetaData:

  1. getColumnLabel() that should return an alias (L in the above sample), if any, or column name otherwise;
  2. getColumnName() that should return a real column name (N) or some replacement for it.

Because many databases don't distinguish these names their JDBC drivers return aliases (L) from both methods. H2 follows the JDBC specification strictly in this area by default and returns different names from these methods, but it can return the alias from both methods if a compatibility mode or a separate compatibility flag was set. Many people unaware about getColumnLabel() and incorrectly use getColumnName() instead of it.

io.r2dbc.spi.ColumnMetadata, however, has only one getName() method that looks like a good idea, but I cannot find in its javadoc is it equivalent of getColumnName() or getColumnLabel(). Perhaps it should really work like getColumnLabel() but it's name can create an impression that it is an equivalent of getColumnName().

io.r2dbc.h2.H2ColumnMetadata.toColumnMetadata() uses ResultInterface.getColumnName() as a source for getName() method. ResultInterface.getColumnName() returns a real column name, I think that you should use ResultInterface.getAlias() instead.

@mp911de
Copy link
Member Author

mp911de commented Apr 29, 2019

Thanks for having a look. I wasn't even aware of the distinction between labels and names. We intend to expose columns as they are represented in the result and that would be always the alias. We also need to make sure that we consistently use labels/aliases with our H2 wrapper.

@katzyn can you leave a comment at r2dbc/r2dbc-spi#58 so we can improve Javadocs before merging the PR?

Real column names seem to me only useful if the consumer is going to analyze results to rewrite a query or to find the original column names.

@gregturn
Copy link
Contributor

Based on @katzyn 's feedback, I switched to using getAlias() as of de99a01.

@gregturn
Copy link
Contributor

If we have need for BOTH name and alias, then we can offer that as an extension to H2 R2DBC types.

@mp911de
Copy link
Member Author

mp911de commented Apr 29, 2019

I updated Javadocs in SPI to reflect we’re using aliases/result labels and not real column names if columns are aliased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: spec A change mandated by the specification
Projects
None yet
Development

No branches or pull requests

3 participants