You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by parthchandra <gi...@git.apache.org> on 2017/11/23 02:57:48 UTC
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
GitHub user parthchandra opened a pull request:
https://github.com/apache/drill/pull/1049
DRILL-5971: Fix INT64, INT32 logical types in complex parquet reader
Added support for the INT64 and INT32 logical types in the complex parquet reader
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/parthchandra/drill DRILL-5971
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1049.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1049
----
commit a27f5e4cb9a71d6ce3901c700fd69850dbdf22b9
Author: Parth Chandra <pa...@apache.org>
Date: 2017-08-29T18:20:47Z
DRILL-5971: Fix INT64, INT32 logical types in complex parquet reader
----
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1049
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1049
@vvysotskyi please review.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r153735994
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ColumnReaderFactory.java ---
@@ -138,6 +138,8 @@
return new ParquetFixedWidthDictionaryReaders.DictionaryBigIntReader(recordReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, (BigIntVector) v, schemaElement);
--- End diff --
This comment had to be placed in line [117](https://github.com/apache/drill/pull/1049/files?diff=unified#diff-4a7ec07122bfb16e4ff696af256f56dcR117), but I could not add it there.
Should we also handle the case when `columnChunkMetaData.getType()` type is `INT32` and `convertedType` is `INT_32`?
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158703216
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
+ .baselineValues( mapOf("a","a","b","b") , -1L , -1 , -1 , -1 , -1L , -1 , -1 , -1 )
+ .baselineValues( mapOf("a","a","b","b") , 1L , 1 , 1 , 1 , -9223372036854775808L , 1 , 1 , 1 )
+ .baselineValues( mapOf("a","a","b","b") , 9223372036854775807L , 2147483647 , 65535 , 255 , 9223372036854775807L , -2147483648 , -32768 , -128 )
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes2() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
--- End diff --
Please move `Arrays.fill(bytesOnes, (byte)1);` to the new line and add space before 1.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158703375
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
--- End diff --
Please remove space before `mapOf` and remove all spaces before commas.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158703477
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
+ .baselineValues( mapOf("a","a","b","b") , -1L , -1 , -1 , -1 , -1L , -1 , -1 , -1 )
+ .baselineValues( mapOf("a","a","b","b") , 1L , 1 , 1 , 1 , -9223372036854775808L , 1 , 1 , 1 )
+ .baselineValues( mapOf("a","a","b","b") , 9223372036854775807L , 2147483647 , 65535 , 255 , 9223372036854775807L , -2147483648 , -32768 , -128 )
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes2() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
+ byte[] bytesZeros = new byte[12];
+ String query = String.format(
+ " select " +
+ " t.rowKey as rowKey, " +
+ " t.StringTypes._UTF8 as _UTF8, " +
+ " t.StringTypes._Enum as _Enum, " +
+ " t.NumericTypes.Int32._INT32_RAW as _INT32_RAW, " +
+ " t.NumericTypes.Int32._INT_8 as _INT_8, " +
+ " t.NumericTypes.Int32._INT_16 as _INT_16, " +
+ " t.NumericTypes.Int32._INT_32 as _INT_32, " +
+ " t.NumericTypes.Int32._UINT_8 as _UINT_8, " +
+ " t.NumericTypes.Int32._UINT_16 as _UINT_16, " +
+ " t.NumericTypes.Int32._UINT_32 as _UINT_32, " +
+ " t.NumericTypes.Int64._INT64_RAW as _INT64_RAW, " +
+ " t.NumericTypes.Int64._INT_64 as _INT_64, " +
+ " t.NumericTypes.Int64._UINT_64 as _UINT_64, " +
+ " t.NumericTypes.DateTimeTypes._DATE_int32 as _DATE_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIME_MILLIS_int32 as _TIME_MILLIS_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIMESTAMP_MILLIS_int64 as _TIMESTAMP_MILLIS_int64, " +
+ " t.NumericTypes.DateTimeTypes._INTERVAL_fixed_len_byte_array_12 as _INTERVAL_fixed_len_byte_array_12, " +
+ " t.NumericTypes.Int96._INT96_RAW as _INT96_RAW " +
+ " from " +
+ " cp.`store/parquet/complex/parquet_logical_types_complex.parquet` t " +
+ " order by t.rowKey "
+ );
+ String[] columns = {
+ "rowKey " ,
+ "_UTF8" ,
+ "_Enum" ,
+ "_INT32_RAW" ,
+ "_INT_8" ,
+ "_INT_16" ,
+ "_INT_32" ,
+ "_UINT_8" ,
+ "_UINT_16" ,
+ "_UINT_32" ,
+ "_INT64_RAW" ,
+ "_INT_64" ,
+ "_UINT_64" ,
+ "_DATE_int32" ,
+ "_TIME_MILLIS_int32" ,
+ "_TIMESTAMP_MILLIS_int64" ,
+ "_INTERVAL_fixed_len_byte_array_12" ,
+ "_INT96_RAW"
+
+ };
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
+ .baselineColumns(columns)
+ .baselineValues(1, "UTF8 string1", "RANDOM_VALUE", 1234567, 123, 12345, 1234567, 123, 1234, 1234567,
+ 1234567890123456L, 1234567890123456L, 1234567890123456L, new DateTime("5350-02-17"),
+ new DateTime(1234567, DateTimeZone.UTC).withZoneRetainFields(DateTimeZone.getDefault()),
+ new DateTime("1973-11-29T21:33:09.012"), new Period("P875770417M943142453DT1650536.505S"),
+ bytes12)
+ .baselineValues(2, "UTF8 string2", "MAX_VALUE", 2147483647, 127, 32767, 2147483647, 255, 65535, -1,
+ 9223372036854775807L, 9223372036854775807L, -1L, new DateTime("1969-12-31"),
+ new DateTime(0xFFFFFFFF, DateTimeZone.UTC).withZoneRetainFields(DateTimeZone.getDefault()),
+ new DateTime("2038-01-19T03:14:07.999"), new Period("P16843009M16843009DT16843.009S"),
+ bytesOnes)
+ .baselineValues(3, "UTF8 string3", "MIN_VALUE", -2147483648, -128, -32768, -2147483648, 0, 0, 0,
+ -9223372036854775808L, -9223372036854775808L, 0L, new DateTime("1970-01-01"),
+ new DateTime(0, DateTimeZone.UTC).withZoneRetainFields(DateTimeZone.getDefault()),
+ new DateTime("1970-01-01T00:00:00.0"), new Period("PT0S"), bytesZeros)
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes3() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
+ byte[] bytesZeros = new byte[12];
+ String query = String.format(
+ " select " +
+ " t.rowKey as rowKey, " +
+ " t.StringTypes._UTF8 as _UTF8, " +
+ " t.StringTypes._Enum as _Enum, " +
+ " t.NumericTypes.Int32._INT32_RAW as _INT32_RAW, " +
+ " t.NumericTypes.Int32._INT_8 as _INT_8, " +
+ " t.NumericTypes.Int32._INT_16 as _INT_16, " +
+ " t.NumericTypes.Int32._INT_32 as _INT_32, " +
+ " t.NumericTypes.Int32._UINT_8 as _UINT_8, " +
+ " t.NumericTypes.Int32._UINT_16 as _UINT_16, " +
+ " t.NumericTypes.Int32._UINT_32 as _UINT_32, " +
+ " t.NumericTypes.Int64._INT64_RAW as _INT64_RAW, " +
+ " t.NumericTypes.Int64._INT_64 as _INT_64, " +
+ " t.NumericTypes.Int64._UINT_64 as _UINT_64, " +
+ " t.NumericTypes.DateTimeTypes._DATE_int32 as _DATE_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIME_MILLIS_int32 as _TIME_MILLIS_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIMESTAMP_MILLIS_int64 as _TIMESTAMP_MILLIS_int64, " +
+ " t.NumericTypes.DateTimeTypes._INTERVAL_fixed_len_byte_array_12 as _INTERVAL_fixed_len_byte_array_12, " +
+ " t.NumericTypes.Int96._INT96_RAW as _INT96_RAW " +
+ " from " +
+ " cp.`store/parquet/complex/parquet_logical_types_complex_nullable.parquet` t " +
+ " order by t.rowKey "
+ );
+ String[] columns = {
+ "rowKey " ,
+ "_UTF8" ,
+ "_Enum" ,
+ "_INT32_RAW" ,
+ "_INT_8" ,
+ "_INT_16" ,
+ "_INT_32" ,
+ "_UINT_8" ,
+ "_UINT_16" ,
+ "_UINT_32" ,
+ "_INT64_RAW" ,
+ "_INT_64" ,
+ "_UINT_64" ,
+ "_DATE_int32" ,
+ "_TIME_MILLIS_int32" ,
+ "_TIMESTAMP_MILLIS_int64" ,
+ "_INTERVAL_fixed_len_byte_array_12" ,
+ "_INT96_RAW"
+
+ };
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
--- End diff --
unOrdered
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158703447
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
+ .baselineValues( mapOf("a","a","b","b") , -1L , -1 , -1 , -1 , -1L , -1 , -1 , -1 )
+ .baselineValues( mapOf("a","a","b","b") , 1L , 1 , 1 , 1 , -9223372036854775808L , 1 , 1 , 1 )
+ .baselineValues( mapOf("a","a","b","b") , 9223372036854775807L , 2147483647 , 65535 , 255 , 9223372036854775807L , -2147483648 , -32768 , -128 )
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes2() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
+ byte[] bytesZeros = new byte[12];
+ String query = String.format(
+ " select " +
+ " t.rowKey as rowKey, " +
+ " t.StringTypes._UTF8 as _UTF8, " +
+ " t.StringTypes._Enum as _Enum, " +
+ " t.NumericTypes.Int32._INT32_RAW as _INT32_RAW, " +
+ " t.NumericTypes.Int32._INT_8 as _INT_8, " +
+ " t.NumericTypes.Int32._INT_16 as _INT_16, " +
+ " t.NumericTypes.Int32._INT_32 as _INT_32, " +
+ " t.NumericTypes.Int32._UINT_8 as _UINT_8, " +
+ " t.NumericTypes.Int32._UINT_16 as _UINT_16, " +
+ " t.NumericTypes.Int32._UINT_32 as _UINT_32, " +
+ " t.NumericTypes.Int64._INT64_RAW as _INT64_RAW, " +
+ " t.NumericTypes.Int64._INT_64 as _INT_64, " +
+ " t.NumericTypes.Int64._UINT_64 as _UINT_64, " +
+ " t.NumericTypes.DateTimeTypes._DATE_int32 as _DATE_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIME_MILLIS_int32 as _TIME_MILLIS_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIMESTAMP_MILLIS_int64 as _TIMESTAMP_MILLIS_int64, " +
+ " t.NumericTypes.DateTimeTypes._INTERVAL_fixed_len_byte_array_12 as _INTERVAL_fixed_len_byte_array_12, " +
+ " t.NumericTypes.Int96._INT96_RAW as _INT96_RAW " +
+ " from " +
+ " cp.`store/parquet/complex/parquet_logical_types_complex.parquet` t " +
+ " order by t.rowKey "
+ );
+ String[] columns = {
+ "rowKey " ,
+ "_UTF8" ,
+ "_Enum" ,
+ "_INT32_RAW" ,
+ "_INT_8" ,
+ "_INT_16" ,
+ "_INT_32" ,
+ "_UINT_8" ,
+ "_UINT_16" ,
+ "_UINT_32" ,
+ "_INT64_RAW" ,
+ "_INT_64" ,
+ "_UINT_64" ,
+ "_DATE_int32" ,
+ "_TIME_MILLIS_int32" ,
+ "_TIMESTAMP_MILLIS_int64" ,
+ "_INTERVAL_fixed_len_byte_array_12" ,
+ "_INT96_RAW"
+
+ };
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
--- End diff --
Maybe `unOrdered`?
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:
https://github.com/apache/drill/pull/1049
@vvysotskyi please review -
added fixes for date time and interval types, fixed the reading of dictionary encoded fixed width array types (including int96), fixed object representation of interval types (used by unit tests and sqlline/jdbc), and added a new test file generator that generates data for all (primitive) logical types in either flat or hierarchical schemas. Added those files as unit tests for the logical types.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158985937
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ColumnReaderFactory.java ---
@@ -138,6 +138,8 @@
return new ParquetFixedWidthDictionaryReaders.DictionaryBigIntReader(recordReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, (BigIntVector) v, schemaElement);
--- End diff --
Hmm. Looks like I missed INT_32 here. Added it but I cannot seem to force the test generator to generate dictionary encoded values for the logical int types. I'll make a note to enhance it later. Note that the DATE type is (apparently) handled by the code on line 101.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158700125
--- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
@@ -394,9 +394,11 @@ public void get(int index, Nullable${minor.class}Holder holder){
final int offsetIndex = index * VALUE_WIDTH;
final int months = data.getInt(offsetIndex);
final int days = data.getInt(offsetIndex + ${minor.daysOffset});
- final int millis = data.getInt(offsetIndex + ${minor.millisecondsOffset});
+ int millis = data.getInt(offsetIndex + 8);
--- End diff --
I think we should revert this change. It is easier to change `daysOffset` and `millisecondsOffset` values in `ValueVectorTypes.tdd` rather find all usages if it will be needed.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r159003539
--- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
@@ -394,9 +394,11 @@ public void get(int index, Nullable${minor.class}Holder holder){
final int offsetIndex = index * VALUE_WIDTH;
final int months = data.getInt(offsetIndex);
final int days = data.getInt(offsetIndex + ${minor.daysOffset});
- final int millis = data.getInt(offsetIndex + ${minor.millisecondsOffset});
+ int millis = data.getInt(offsetIndex + 8);
--- End diff --
Hmm. The only reason for this change was to overcome a Joda problem that caused the unit tests to fail. I fixed the unit tests and undid the change.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r153909807
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ColumnReaderFactory.java ---
@@ -138,6 +138,8 @@
return new ParquetFixedWidthDictionaryReaders.DictionaryBigIntReader(recordReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, (BigIntVector) v, schemaElement);
--- End diff --
OK I'll try to add these. BTW, I realized that the test files that I added for the unit tests are not annotated, so I'll need to fix those as well!
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:
https://github.com/apache/drill/pull/1049
Updated the PR to include all the remaining logical int types. While generating the test file for the unit test, found that some of the types were missing from the fast parquet reader so added those as well.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158702957
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java ---
@@ -636,8 +636,10 @@ public void testCTASWithIntervalTypes() throws Exception {
"interval '-1000000000 20:12:23.999' day(10) to second col2 " +
"from cp.`employee.json` limit 2", tableName);
- Period row1Col1 = new Period(0, 0, 0, 10, 0, 0, 0, 73840123);
- Period row1Col2 = new Period(0, 0, 0, -1000000000, 0, 0, 0, -72743999);
+// Period row1Col1 = new Period(0, 0, 0, 10, 0, 0, 0, 73840123);
+// Period row1Col2 = new Period(0, 0, 0, -1000000000, 0, 0, 0, -72743999);
--- End diff --
Please remove these comment lines.
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1049
@parthchandra, thanks for the pull request! LGTM, +1.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r153739725
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ColumnReaderFactory.java ---
@@ -138,6 +138,8 @@
return new ParquetFixedWidthDictionaryReaders.DictionaryBigIntReader(recordReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, (BigIntVector) v, schemaElement);
--- End diff --
`DATE` logical type also encoded as the `INT32` physical type [1], so could you please also add its support?
[1] https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#date
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1049
LGTM, +1. Except `INT_32` and `INT_64` parquet can contain other logical numeric types: `UINT_8`, `UINT_16`, `UINT_32`, `UINT_64`, `INT_8`, `INT_16`[1]. It would be great to add support for these logical types in this pull request or create another Jira for this.
[1] https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#numeric-types
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158702875
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ColumnReaderFactory.java ---
@@ -138,6 +138,8 @@
return new ParquetFixedWidthDictionaryReaders.DictionaryBigIntReader(recordReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, (BigIntVector) v, schemaElement);
--- End diff --
Sorry, I didn't notice a check for `DATE`, but what about `INT_32`?
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158986798
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
+ .baselineValues( mapOf("a","a","b","b") , -1L , -1 , -1 , -1 , -1L , -1 , -1 , -1 )
+ .baselineValues( mapOf("a","a","b","b") , 1L , 1 , 1 , 1 , -9223372036854775808L , 1 , 1 , 1 )
+ .baselineValues( mapOf("a","a","b","b") , 9223372036854775807L , 2147483647 , 65535 , 255 , 9223372036854775807L , -2147483648 , -32768 , -128 )
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes2() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
+ byte[] bytesZeros = new byte[12];
+ String query = String.format(
+ " select " +
+ " t.rowKey as rowKey, " +
+ " t.StringTypes._UTF8 as _UTF8, " +
+ " t.StringTypes._Enum as _Enum, " +
+ " t.NumericTypes.Int32._INT32_RAW as _INT32_RAW, " +
+ " t.NumericTypes.Int32._INT_8 as _INT_8, " +
+ " t.NumericTypes.Int32._INT_16 as _INT_16, " +
+ " t.NumericTypes.Int32._INT_32 as _INT_32, " +
+ " t.NumericTypes.Int32._UINT_8 as _UINT_8, " +
+ " t.NumericTypes.Int32._UINT_16 as _UINT_16, " +
+ " t.NumericTypes.Int32._UINT_32 as _UINT_32, " +
+ " t.NumericTypes.Int64._INT64_RAW as _INT64_RAW, " +
+ " t.NumericTypes.Int64._INT_64 as _INT_64, " +
+ " t.NumericTypes.Int64._UINT_64 as _UINT_64, " +
+ " t.NumericTypes.DateTimeTypes._DATE_int32 as _DATE_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIME_MILLIS_int32 as _TIME_MILLIS_int32, " +
+ " t.NumericTypes.DateTimeTypes._TIMESTAMP_MILLIS_int64 as _TIMESTAMP_MILLIS_int64, " +
+ " t.NumericTypes.DateTimeTypes._INTERVAL_fixed_len_byte_array_12 as _INTERVAL_fixed_len_byte_array_12, " +
+ " t.NumericTypes.Int96._INT96_RAW as _INT96_RAW " +
+ " from " +
+ " cp.`store/parquet/complex/parquet_logical_types_complex.parquet` t " +
+ " order by t.rowKey "
+ );
+ String[] columns = {
+ "rowKey " ,
+ "_UTF8" ,
+ "_Enum" ,
+ "_INT32_RAW" ,
+ "_INT_8" ,
+ "_INT_16" ,
+ "_INT_32" ,
+ "_UINT_8" ,
+ "_UINT_16" ,
+ "_UINT_32" ,
+ "_INT64_RAW" ,
+ "_INT_64" ,
+ "_UINT_64" ,
+ "_DATE_int32" ,
+ "_TIME_MILLIS_int32" ,
+ "_TIMESTAMP_MILLIS_int64" ,
+ "_INTERVAL_fixed_len_byte_array_12" ,
+ "_INT96_RAW"
+
+ };
+ testBuilder()
+ .sqlQuery(query)
+ .ordered()
--- End diff --
In these tests the result of the query is always ordered since there is only one fragment. Ordered tests run sightly faster and if/when the test fails, ordered tests are _much_ easier to debug.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/1049#discussion_r158986526
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetComplex.java ---
@@ -193,4 +200,168 @@ public void notxistsField() throws Exception {
.run();
}
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes() throws Exception {
+ String query = String.format("select t.complextype as complextype, " +
+ "t.uint_64 as uint_64, t.uint_32 as uint_32, t.uint_16 as uint_16, t.uint_8 as uint_8, " +
+ "t.int_64 as int_64, t.int_32 as int_32, t.int_16 as int_16, t.int_8 as int_8 " +
+ "from cp.`store/parquet/complex/logical_int_complex.parquet` t" );
+ String[] columns = {"complextype", "uint_64", "uint_32", "uint_16", "uint_8", "int_64", "int_32", "int_16", "int_8" };
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns(columns)
+ .baselineValues( mapOf("a","a","b","b") , 0L , 0 , 0 , 0 , 0L , 0 , 0 ,0 )
+ .baselineValues( mapOf("a","a","b","b") , -1L , -1 , -1 , -1 , -1L , -1 , -1 , -1 )
+ .baselineValues( mapOf("a","a","b","b") , 1L , 1 , 1 , 1 , -9223372036854775808L , 1 , 1 , 1 )
+ .baselineValues( mapOf("a","a","b","b") , 9223372036854775807L , 2147483647 , 65535 , 255 , 9223372036854775807L , -2147483648 , -32768 , -128 )
+ .build()
+ .run();
+ }
+
+ @Test //DRILL-5971
+ public void testComplexLogicalIntTypes2() throws Exception {
+ byte[] bytes12 = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0', 'a', 'b' };
+ byte[] bytesOnes = new byte[12]; Arrays.fill(bytesOnes, (byte)1);
--- End diff --
Can't see how that makes things more readable. This way, the declaration and initialization is together and all the declarations are clearly readable. Changed it anyway since you asked :)
---