Skip to content

Conversation

@aiguofer
Copy link
Contributor

@aiguofer aiguofer commented Aug 11, 2023

Rationale for this change

Existing info is not enough to make completely accurate conversion decisions.

What changes are included in this PR?

Include the type name reported by the database and the max number of characters as part of the JdbcFieldInfo.

@aiguofer
Copy link
Contributor Author

I tried to find all common fields across ResultSetMetaData and getColumns. These were the only 2 missing.

@lidavidm
Copy link
Member

I think you do need more fields, though. For example: in Postgres, you need it to figure out the timestamp precision (https://github.com/apache/arrow-adbc/blob/main/java/driver/jdbc/src/main/java/org/apache/arrow/adbc/driver/jdbc/adapter/JdbcToArrowTypeConverters.java). Or to figure out the precision/scale for NUMERIC/decimal.

@aiguofer
Copy link
Contributor Author

Both of those are already part of JdbcFieldInfo.

The remaining pieces of info seem to be harder to get in a standardized way.

For example, there's a bunch of extra columns from the getColumns ResultSet that don't have a respective ResultSetMetaData method call.

There's also a few ResultSetMetaData method calls that don't have a respective column in a getColumns row, but instead would be found as columns in a getTypeInfo.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Aug 11, 2023
@lidavidm lidavidm merged commit b97e526 into apache:main Aug 14, 2023
@lidavidm lidavidm removed the awaiting merge Awaiting merge label Aug 14, 2023
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit b97e526.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…pache#37123)

### Rationale for this change

Existing info is not enough to make completely accurate conversion decisions.

### What changes are included in this PR?

Include the type name reported by the database and the max number of characters as part of the `JdbcFieldInfo`.
* Closes: apache#35916

Authored-by: Diego Fernandez <aiguo.fernandez@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…pache#37123)

### Rationale for this change

Existing info is not enough to make completely accurate conversion decisions.

### What changes are included in this PR?

Include the type name reported by the database and the max number of characters as part of the `JdbcFieldInfo`.
* Closes: apache#35916

Authored-by: Diego Fernandez <aiguo.fernandez@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] JdbcFieldInfo is insufficient for proper type inference

2 participants