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

Add support for double to varchar coercion in hive tables #18832

Conversation

Praveen2112
Copy link
Member

Description

Allows coerce a double type to varchar in hive tables.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Hive
* Add support for double to varchar coercion in hive tables 

@cla-bot cla-bot bot added the cla-signed label Aug 28, 2023
@Praveen2112 Praveen2112 force-pushed the praveen/double_to_varchar_coerver branch from 90d2186 to 5669331 Compare August 28, 2023 10:54
@github-actions github-actions bot added tests:hive hive Hive connector labels Aug 28, 2023
@Praveen2112 Praveen2112 force-pushed the praveen/double_to_varchar_coerver branch from 5669331 to 14abcb9 Compare August 28, 2023 11:42
@@ -321,6 +333,8 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
0.5,
-1.5))
.put("double_to_float", ImmutableList.of(0.5, -1.5))
.put("double_to_string", Arrays.asList("12345.12345", coercedNaN))
Copy link
Member

Choose a reason for hiding this comment

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

ImmutableList too?

Copy link
Member Author

Choose a reason for hiding this comment

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

coercedNaN can be null - so ImmutableList might fails.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, got it.

@@ -61,6 +61,8 @@ private static HiveTableDefinition.HiveTableDefinitionBuilder tableDefinitionBui
bigint_to_varchar BIGINT,
float_to_double FLOAT,
double_to_float DOUBLE,
double_to_string DOUBLE,
double_to_bounded_varchar DOUBLE,
Copy link
Member

Choose a reason for hiding this comment

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

What about a test for out of bounds?

Copy link
Member

@atanasenko atanasenko left a comment

Choose a reason for hiding this comment

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

LGTM % comments

@@ -241,6 +248,11 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
hiveValueForCaseChangeField = "\"LOWER2UPPERCASE\":2";
}

// For ORC when we coerce NaN to String, it returns coerced value as `null`
Copy link
Member

Choose a reason for hiding this comment

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

is this a bug that we should track?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is how ORC file behaves, I'm not sure if we need to track them in ORC project

@Praveen2112 Praveen2112 force-pushed the praveen/double_to_varchar_coerver branch from 14abcb9 to 2e89056 Compare September 5, 2023 06:21
@Praveen2112
Copy link
Member Author

@atanasenko AC

@Praveen2112
Copy link
Member Author

Overridden by #18930

@Praveen2112 Praveen2112 closed this Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed hive Hive connector
Development

Successfully merging this pull request may close these issues.

5 participants