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

ARROW-17629: [Java] Bind DB column to Arrow Map type in JdbcToArrowUtils #14134

Merged
merged 4 commits into from
Sep 21, 2022
Merged

ARROW-17629: [Java] Bind DB column to Arrow Map type in JdbcToArrowUtils #14134

merged 4 commits into from
Sep 21, 2022

Conversation

igor-suhorukov
Copy link
Contributor

@igor-suhorukov igor-suhorukov commented Sep 15, 2022

This pull request allows support of mapping for map type (hstore translated by jdbc driver to java.util.Map or json text/varchar). Mapping for MapConsumer should be manually expressed in new JdbcToArrowConfigBuilder(...).setJdbcToArrowTypeConverter(USER_CUSTOM_jdbcToArrowTypeConverter). Now it possible as part of ARROW-17630 that allow user to distinguish columns by number and related external metadata.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@igor-suhorukov
Copy link
Contributor Author

@lidavidm lidavidm self-requested a review September 15, 2022 13:45
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay. Just a comment on the API changes.

@@ -216,7 +216,7 @@ public JdbcToArrowConfigBuilder setTargetBatchSize(int targetBatchSize) {
* Defaults to wrapping {@link JdbcToArrowUtils#getArrowTypeFromJdbcType(JdbcFieldInfo, Calendar)}.
*/
public JdbcToArrowConfigBuilder setJdbcToArrowTypeConverter(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add an overload to accept a Function still for backwards compatibility?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively - we could add the column index as a field in JdbcFieldInfo so that signatures do not have to change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for review @lidavidm
JdbcFieldInfo with additional column index is works for me

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ursabot
Copy link

ursabot commented Sep 21, 2022

Benchmark runs are scheduled for baseline = fc98c95 and contender = bd433c0. bd433c0 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.99% ⬆️0.14%] test-mac-arm
[Failed ⬇️2.82% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.46% ⬆️0.07%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] bd433c01 ec2-t3-xlarge-us-east-2
[Finished] bd433c01 test-mac-arm
[Failed] bd433c01 ursa-i9-9960x
[Finished] bd433c01 ursa-thinkcentre-m75q
[Finished] fc98c95b ec2-t3-xlarge-us-east-2
[Finished] fc98c95b test-mac-arm
[Failed] fc98c95b ursa-i9-9960x
[Finished] fc98c95b ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Sep 21, 2022

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…ils (apache#14134)

This pull request allows support of mapping for map type (hstore translated by jdbc driver to java.util.Map or json text/varchar). Mapping for MapConsumer should be manually expressed in new JdbcToArrowConfigBuilder(...).setJdbcToArrowTypeConverter(USER_CUSTOM_jdbcToArrowTypeConverter). Now it possible as part of [ARROW-17630](https://issues.apache.org/jira/browse/ARROW-17630) that allow user to distinguish columns by number and related external metadata.

Authored-by: igor.suhorukov <[email protected]>
Signed-off-by: David Li <[email protected]>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
…ils (apache#14134)

This pull request allows support of mapping for map type (hstore translated by jdbc driver to java.util.Map or json text/varchar). Mapping for MapConsumer should be manually expressed in new JdbcToArrowConfigBuilder(...).setJdbcToArrowTypeConverter(USER_CUSTOM_jdbcToArrowTypeConverter). Now it possible as part of [ARROW-17630](https://issues.apache.org/jira/browse/ARROW-17630) that allow user to distinguish columns by number and related external metadata.

Authored-by: igor.suhorukov <[email protected]>
Signed-off-by: David Li <[email protected]>
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.

3 participants