You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/09/30 09:35:16 UTC
[GitHub] [hive] asolimando commented on a diff in pull request #3628: HIVE-26320: Deserialize Parquet VARCHAR and CHAR types appropriately
asolimando commented on code in PR #3628:
URL: https://github.com/apache/hive/pull/3628#discussion_r984400218
##########
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java:
##########
@@ -212,23 +216,59 @@ public void testGetInt64NanosAdjustedToUTCTimestampConverter() throws Exception
}
@Test
- public void testGetTextConverter() throws Exception {
+ public void testGetTextConverterForString() throws Exception {
PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
- .as(LogicalTypeAnnotation.stringType()).named("value");
- Writable writable = getWritableFromBinaryConverter(new VarcharTypeInfo(), primitiveType,
- Binary.fromString("this_is_a_value"));
- Text textWritable = (Text) writable;
- assertEquals("this_is_a_value", textWritable.toString());
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+ Text textWritable = (Text) getWritableFromBinaryConverter(stringTypeInfo, primitiveType,
+ Binary.fromString(value));
+ // we should get what we put in
+ assertEquals(value, textWritable.toString());
+ }
+
+ @Test
+ public void testGetTextConverterForChar() throws Exception {
Review Comment:
Nit: same here, `throws Exception` is redundant
```suggestion
public void testGetTextConverterForChar() {
```
##########
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java:
##########
@@ -212,23 +216,59 @@ public void testGetInt64NanosAdjustedToUTCTimestampConverter() throws Exception
}
@Test
- public void testGetTextConverter() throws Exception {
+ public void testGetTextConverterForString() throws Exception {
PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
- .as(LogicalTypeAnnotation.stringType()).named("value");
- Writable writable = getWritableFromBinaryConverter(new VarcharTypeInfo(), primitiveType,
- Binary.fromString("this_is_a_value"));
- Text textWritable = (Text) writable;
- assertEquals("this_is_a_value", textWritable.toString());
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+ Text textWritable = (Text) getWritableFromBinaryConverter(stringTypeInfo, primitiveType,
+ Binary.fromString(value));
+ // we should get what we put in
+ assertEquals(value, textWritable.toString());
+ }
+
+ @Test
+ public void testGetTextConverterForChar() throws Exception {
+ PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+
+ HiveCharWritable textWritable = (HiveCharWritable) getWritableFromBinaryConverter(
+ new CharTypeInfo(value.length() + 2), primitiveType, Binary.fromString(value));
+ // check that it enforces the length (it should be padded)
+ assertEquals(value + " ", textWritable.toString());
+
+ textWritable = (HiveCharWritable) getWritableFromBinaryConverter(
+ new CharTypeInfo(6), primitiveType, Binary.fromString(value));
+ // check that it enforces the length (it should truncate)
+ assertEquals(value.substring(0, 6), textWritable.toString());
+ }
+
+ @Test
+ public void testGetTextConverterForVarchar() throws Exception {
Review Comment:
```suggestion
public void testGetTextConverterForVarchar() {
```
##########
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java:
##########
@@ -212,23 +216,59 @@ public void testGetInt64NanosAdjustedToUTCTimestampConverter() throws Exception
}
@Test
- public void testGetTextConverter() throws Exception {
+ public void testGetTextConverterForString() throws Exception {
Review Comment:
Nit: the "throws Exception" bit is redundant and can be safely removed
```suggestion
public void testGetTextConverterForString() {
```
##########
ql/src/test/org/apache/hadoop/hive/ql/io/parquet/convert/TestETypeConverter.java:
##########
@@ -212,23 +216,59 @@ public void testGetInt64NanosAdjustedToUTCTimestampConverter() throws Exception
}
@Test
- public void testGetTextConverter() throws Exception {
+ public void testGetTextConverterForString() throws Exception {
PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
- .as(LogicalTypeAnnotation.stringType()).named("value");
- Writable writable = getWritableFromBinaryConverter(new VarcharTypeInfo(), primitiveType,
- Binary.fromString("this_is_a_value"));
- Text textWritable = (Text) writable;
- assertEquals("this_is_a_value", textWritable.toString());
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+ Text textWritable = (Text) getWritableFromBinaryConverter(stringTypeInfo, primitiveType,
+ Binary.fromString(value));
+ // we should get what we put in
+ assertEquals(value, textWritable.toString());
+ }
+
+ @Test
+ public void testGetTextConverterForChar() throws Exception {
+ PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+
+ HiveCharWritable textWritable = (HiveCharWritable) getWritableFromBinaryConverter(
+ new CharTypeInfo(value.length() + 2), primitiveType, Binary.fromString(value));
+ // check that it enforces the length (it should be padded)
+ assertEquals(value + " ", textWritable.toString());
+
+ textWritable = (HiveCharWritable) getWritableFromBinaryConverter(
+ new CharTypeInfo(6), primitiveType, Binary.fromString(value));
+ // check that it enforces the length (it should truncate)
+ assertEquals(value.substring(0, 6), textWritable.toString());
+ }
+
+ @Test
+ public void testGetTextConverterForVarchar() throws Exception {
+ PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
+ .as(LogicalTypeAnnotation.stringType()).named("value");
+ String value = "this_is_a_value";
+
+ HiveVarcharWritable textWritable = (HiveVarcharWritable) getWritableFromBinaryConverter(
+ new VarcharTypeInfo(6), primitiveType, Binary.fromString(value));
+ // check that it enforces the length (it should truncate)
+ assertEquals(value.substring(0, 6), textWritable.toString());
+
+ textWritable = (HiveVarcharWritable) getWritableFromBinaryConverter(
+ new VarcharTypeInfo(34), primitiveType, Binary.fromString(value));
+ // check that it does not pad
+ assertEquals(value, textWritable.toString());
}
@Test
public void testGetTextConverterNoHiveTypeInfo() throws Exception {
PrimitiveType primitiveType = Types.optional(PrimitiveTypeName.BINARY)
.as(LogicalTypeAnnotation.stringType()).named("value");
- Writable writable =
- getWritableFromBinaryConverter(null, primitiveType, Binary.fromString("this_is_a_value"));
- Text textWritable = (Text) writable;
- assertEquals("this_is_a_value", textWritable.toString());
+ String value = "this_is_a_value";
Review Comment:
Nit: since you are modifying the method, you could remove the extra `throws Exception` here too :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org