HIVE-27439: Support Decimal(precision, scale) format #4417
Conversation
| @@ -489,7 +489,7 @@ private TypeInfo parseType() { | |||
| } else if (params.length == 2) { | |||
| // New metadata always have two parameters. | |||
| precision = Integer.parseInt(params[0]); | |||
| scale = Integer.parseInt(params[1]); | |||
| scale = Integer.parseInt(params[1].trim()); | |||
There was a problem hiding this comment.
Thanks for fixing this issue. IMO, we can fix it in upstream invoking code, then we can avoid other potential type problems caused by the blank character. Like this:
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java
index b23e3cfc44..4788558d4b 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java
@@ -334,7 +334,7 @@ private static boolean isTypeChar(char c) {
|| !isTypeChar(typeInfoString.charAt(end))) {
Token t = new Token();
t.position = begin;
- t.text = typeInfoString.substring(begin, end);
+ t.text = typeInfoString.substring(begin, end).trim();
t.isType = isTypeChar(typeInfoString.charAt(begin));
tokens.add(t);
begin = end;
|
cc @nrg4878 |
|
cc @ayushtkn would you please take a look |
kgyrtkirk
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
@ayushtkn The tests have already passed |
|
@leesf The linked JIRA is HIVE-27439 not HIVE-27321. Please change the PR description. |
| "decimal(10,2)", | ||
| "decimal(10, 2)", | ||
| "decimal(10, 2 )", | ||
| "decimal( 10, 2 )" |
There was a problem hiding this comment.
HIVE-11476 added a special case that allowing spaces in some types. e.g struct<user id:int,user group: int> but it not added the test case. Could you please add this test case here to make sure we won't get a regression?
BTW, i have tested the case struct<user id:int,user group: int> with this change locally and it worked well. But i think it is necessary to add it explicitly. cc @ayushtkn
There was a problem hiding this comment.
thanks, added the case.
leesf
changed the title
done |
|
Kudos, SonarCloud Quality Gate passed!
|
…#4417). (leesf, reviewed by Ayush Saxena, Butao Zhang)
…#4417). (leesf, reviewed by Ayush Saxena, Butao Zhang)
What changes were proposed in this pull request?
Now, TypeInfoUtils#parseType only support Decimal(precision,scale) instead of Decimal(precision, scale), for example, support decimal(10,2) instead of decimal(10, 2) with a blank before 2, however, users may create decimal in this format decimal(10, 2) and it will throw exception in
we need fix this issue.
Why are the changes needed?
Fix the Decimal(precision, scale) parsing error.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Added UT