[GitHub] drill pull request #1066: DRILL-3993: Changes to support Calcite 1.15
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/1066#discussion_r158263816 --- Diff: exec/jdbc-all/pom.xml --- @@ -572,7 +572,7 @@ This is likely due to you adding new dependencies to a java-exec and not updating the excludes in this module. This is important as it minimizes the size of the dependency of Drill application users. -2900 +3100 --- End diff -- We didn't have problems on our local machines or test cluster connected with `maxsize` after changing it to 3100. Which version of Maven did you use? Anyway, I have increased it to 3200. ---
[GitHub] drill pull request #1066: DRILL-3993: Changes to support Calcite 1.15
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1066#discussion_r158261430
--- Diff: exec/java-exec/src/test/resources/record/test_recorditerator.json
---
@@ -60,7 +60,7 @@
@id:2,
child:1,
pop:"project",
-exprs:[ { ref : "`*`", expr : "`*`"} ]
+exprs:[ { ref : "`**`", expr : "`**`"} ]
--- End diff --
This change connected with commit `[CALCITE-1150] Add dynamic record type
and dynamic star for schema-on-read table`. `**` is used as the dynamic star
column name prefix. It is equivalent to `*` in our Drill-Calcite 1.4.0.
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1068 @MitchelLabonte, thanks for the pull request, +1 ---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1068 Sorry, I read your removed comment and made a suggestion that this test fails. Did you run all unit tests to see that this change does not break anything? ---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1068 @MitchelLabonte, I think this NPE is just a consequence of the bug that should be fixed. Please investigate why Drill is trying to use child `PathSegment` when a value has VarChar type. ---
[GitHub] drill pull request #1068: DRILL-6020 Fix NullPointerException when querying ...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1068#discussion_r156621897
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/FieldIdUtil.java
---
@@ -131,7 +131,7 @@ public static TypedFieldId
getFieldIdIfMatches(ValueVector vector, TypedFieldId.
} else if(v instanceof ListVector) {
ListVector list = (ListVector) v;
return getFieldIdIfMatches(list, builder, addToBreadCrumb,
seg.getChild());
-} else if (v instanceof UnionVector) {
+} else if (v instanceof UnionVector && !seg.isLastPath()) {
--- End diff --
Good catch!
I think we should add check for nulls into method
`getFieldIdIfMatchesUnion()` as it was done for `getFieldIdIfMatches()`. Also
please add a unit test for this change. You may use
[testFieldWithDots()](https://github.com/apache/drill/blob/acc5ed927e1fa4011ac1c3724d15197484b9f45b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java#L705)
as an example.
But one more point: with this change error is not thrown, but only nulls
are returned. I think we should also fix this issue on the borders of this pull
request.
---
[GitHub] drill pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
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 pull request #1049: DRILL-5971: Fix INT64, INT32 logical types in comp...
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 issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
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 #1046: DRILL-5986: Update jackson-databind version to 2.7...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/1046 DRILL-5986: Update jackson-databind version to 2.7.9.1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5986 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1046.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 #1046 commit b5dd2af5272cfd08237122cf21fc2fee5ec8a6df Author: Volodymyr Vysotskyi <vvo...@gmail.com> Date: 2017-11-22T15:45:42Z DRILL-5986: Update jackson-databind version to 2.7.9.1 ---
[GitHub] drill issue #904: DRILL-5717: change some date time test cases with specific...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/904 @weijietong, thanks for the pull request, +1 ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149616688
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -120,4 +124,27 @@ protected LogicalExpression parseExpr(String expr)
throws RecognitionException {
return ret.e;
}
+ /**
+ * mock current locale to US
+ */
+ public static void mockUSLocale() {
+new MockUp() {
+ @Mock
+ public DateFormatSymbols getDateFormatSymbols(Locale locale) {
+return new DateFormatSymbols(Locale.US);
+ }
+};
+ }
+
+ /**
+ * mock current timezone to UTC
--- End diff --
Please start the sentence with a capital letter and add a description about
the method which is mocked.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149615891
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -120,4 +124,27 @@ protected LogicalExpression parseExpr(String expr)
throws RecognitionException {
return ret.e;
}
+ /**
+ * mock current locale to US
--- End diff --
Please start the sentence with a capital letter and add a description about
the method which is mocked.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149612076
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java
---
@@ -33,128 +33,140 @@
import org.apache.drill.exec.server.Drillbit;
import org.apache.drill.exec.server.RemoteServiceSet;
import org.apache.drill.exec.vector.ValueVector;
+import org.joda.time.DateTimeUtils;
import org.joda.time.LocalDate;
-import org.joda.time.LocalTime;
import org.joda.time.LocalDateTime;
+import org.joda.time.LocalTime;
import org.junit.Ignore;
import org.junit.Test;
-
-import com.google.common.base.Charsets;
-import com.google.common.io.Files;
import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import java.text.DateFormatSymbols;
+import java.util.List;
+import java.util.Locale;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+@RunWith(JMockit.class)
@Category({UnlikelyTest.class, SqlFunctionTest.class})
public class TestDateFunctions extends PopUnitTestBase {
-static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TestDateFunctions.class);
+ static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TestDateFunctions.class);
-public void testCommon(String[] expectedResults, String physicalPlan,
String resourceFile) throws Exception {
-try (RemoteServiceSet serviceSet =
RemoteServiceSet.getLocalServiceSet();
- Drillbit bit = new Drillbit(CONFIG, serviceSet);
- DrillClient client = new DrillClient(CONFIG,
serviceSet.getCoordinator())) {
-// run query.
-bit.run();
-client.connect();
-List results =
client.runQuery(org.apache.drill.exec.proto.UserBitShared.QueryType.PHYSICAL,
-
Files.toString(FileUtils.getResourceAsFile(physicalPlan), Charsets.UTF_8)
-.replace("#{TEST_FILE}", resourceFile));
+ public void testCommon(String[] expectedResults, String physicalPlan,
String resourceFile) throws Exception {
+try (RemoteServiceSet serviceSet =
RemoteServiceSet.getLocalServiceSet();
+ Drillbit bit = new Drillbit(CONFIG, serviceSet);
+ DrillClient client = new DrillClient(CONFIG,
serviceSet.getCoordinator())) {
-RecordBatchLoader batchLoader = new
RecordBatchLoader(bit.getContext().getAllocator());
+ // run query.
+ bit.run();
+ client.connect();
+ List results =
client.runQuery(org.apache.drill.exec.proto.UserBitShared.QueryType.PHYSICAL,
+Files.toString(FileUtils.getResourceAsFile(physicalPlan),
Charsets.UTF_8)
+ .replace("#{TEST_FILE}", resourceFile));
-QueryDataBatch batch = results.get(0);
-assertTrue(batchLoader.load(batch.getHeader().getDef(),
batch.getData()));
+ RecordBatchLoader batchLoader = new
RecordBatchLoader(bit.getContext().getAllocator());
+ QueryDataBatch batch = results.get(0);
+ assertTrue(batchLoader.load(batch.getHeader().getDef(),
batch.getData()));
-int i = 0;
-for (VectorWrapper v : batchLoader) {
-ValueVector.Accessor accessor =
v.getValueVector().getAccessor();
-System.out.println(accessor.getObject(0));
-assertEquals( expectedResults[i++],
accessor.getObject(0).toString());
-}
-
-batchLoader.clear();
-for(QueryDataBatch b : results){
-b.release();
-}
-}
-}
-
-@Test
-@Ignore("relies on particular timezone")
-public void testDateIntervalArithmetic() throws Exception {
-String expectedResults[] = {"2009-02-23T00:00:00.000-08:00",
-"2008-02-24T00:00:00.000-08:00",
-"1970-01-01T13:20:33.000-08:00",
-"2008-02-24T12:00:00.000-08:00",
-"2009-04-23T12:00:00.000-07:00",
-"2008-02-24T12:00:00.000-08:00",
-"2009-04-23T12:00:00.000-07:00",
-"2009-02-23T00:00:00.000-08:00",
-"2008-02-24T00:00:00.000-08:00",
-"1970-01-01T13:20:33.000-08:00",
-"2008-02-24T12:00:00.
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149614134
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -120,4 +124,27 @@ protected LogicalExpression parseExpr(String expr)
throws RecognitionException {
return ret.e;
}
+ /**
+ * mock current locale to US
+ */
+ public static void mockUSLocale() {
--- End diff --
Please rename this method to mockUsDateFormatSymbols
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149613241
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -120,4 +124,27 @@ protected LogicalExpression parseExpr(String expr)
throws RecognitionException {
return ret.e;
}
+ /**
+ * mock current locale to US
+ */
+ public static void mockUSLocale() {
+new MockUp() {
+ @Mock
+ public DateFormatSymbols getDateFormatSymbols(Locale locale) {
+return new DateFormatSymbols(Locale.US);
+ }
+};
+ }
+
+ /**
+ * mock current timezone to UTC
+ */
+ public static void mockUTCTimezone() {
--- End diff --
Please rename this method to mockUtcDateTimeZone
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149440034
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java
---
@@ -117,6 +123,13 @@ public void createFiles(int smallFileLines, int
bigFileLines) throws Exception{
@Test
public void testConstantFolding_allTypes() throws Exception {
+new MockUp() {
--- End diff --
I meant to use the same method in all tests where Locale should be mocked.
Please replace it.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149440814
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -77,16 +82,23 @@ public void testCastByConstantFolding() throws
Exception {
@Test // DRILL-3769
public void testToDateForTimeStamp() throws Exception {
-final String query = "select to_date(to_timestamp(-1)) as col \n" +
-"from (values(1))";
+new MockUp() {
--- End diff --
This mock is used twice. So let's also move the code into the separate
method.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r149440392
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/testing/TestDateConversions.java
---
@@ -225,4 +243,16 @@ public void testPostgresDateFormatError() throws
Exception {
throw e;
}
}
+
+ /**
+ * mock current locale to US
+ */
+ private void mockUSLocale() {
--- End diff --
Please move this method into ExecTest class and make it public and static.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r148002267
--- Diff:
logical/src/test/java/org/apache/drill/common/expression/fn/JodaDateValidatorTest.java
---
@@ -119,13 +118,14 @@ public void testDateDayOfYearYearFormat() {
@Test
public void testTimeHoursMinutesSecondsFormat() {
+Locale.setDefault(Locale.US);
--- End diff --
Not sure that this change is necessary.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r147997514 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java --- @@ -26,14 +26,14 @@ import org.junit.Test; import org.junit.experimental.categories.Category; -import java.sql.Date; --- End diff -- When you removed unused import, two empty lines stayed. Could you please also remove one of them? ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r147996953
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java
---
@@ -26,14 +26,14 @@
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import java.sql.Date;
@Category({UnlikelyTest.class, SqlFunctionTest.class})
public class TestNewDateFunctions extends BaseTestQuery {
DateTime date;
DateTimeFormatter formatter;
long unixTimeStamp = -1;
+
--- End diff --
Please revert the addition of empty line there.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r148000393
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/testing/TestDateConversions.java
---
@@ -104,6 +110,13 @@ public void testPostgresDate() throws Exception {
@Test
public void testJodaTime() throws Exception {
+new MockUp() {
--- End diff --
Since the same block of code is used in many places, we should move it to
the separate method (and add Javadoc).
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r148000718
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/testing/TestDateConversions.java
---
@@ -117,10 +130,18 @@ public void testJodaTime() throws Exception {
.baselineValues(true, true)
.baselineValues(false, true)
.go();
+
--- End diff --
Please revert the addition of empty line.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r147581757
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -77,16 +83,21 @@ public void testCastByConstantFolding() throws
Exception {
@Test // DRILL-3769
public void testToDateForTimeStamp() throws Exception {
-final String query = "select to_date(to_timestamp(-1)) as col \n" +
-"from (values(1))";
-
+final String query = "select to_date(to_timestamp(-1)) as col \n"
+ + "from (values(1))";
+new MockUp() {
--- End diff --
Please move this mock before string declaration and add an empty line after
the mock.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r147582599 --- Diff: logical/src/test/java/org/apache/drill/common/expression/fn/JodaDateValidatorTest.java --- @@ -24,6 +24,7 @@ import org.junit.BeforeClass; import org.junit.Test; +import java.util.Locale; --- End diff -- Could you please revert changes, made in this class to the version, where you just added `.withLocale(Locale.US)` in `parseDateFromPostgres` method. My earlier comment was connected only with change, made in UDF function, but here, in the method that just used for unit tests, we can do that. ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r147581841 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java --- @@ -17,11 +17,10 @@ */ package org.apache.drill.exec.fn.impl; + --- End diff -- Please remove this extra empty line (and in other places, where you have added it). ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r147582266
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java
---
@@ -34,19 +33,22 @@
import org.apache.drill.exec.server.RemoteServiceSet;
import org.apache.drill.exec.vector.ValueVector;
import org.joda.time.LocalDate;
-import org.joda.time.LocalTime;
import org.joda.time.LocalDateTime;
+import org.joda.time.LocalTime;
import org.junit.Ignore;
import org.junit.Test;
-
import com.google.common.base.Charsets;
import com.google.common.io.Files;
import org.junit.experimental.categories.Category;
+import java.util.Locale;
+
+
@Category({UnlikelyTest.class, SqlFunctionTest.class})
public class TestDateFunctions extends PopUnitTestBase {
--- End diff --
This class has 4 space indentation, but according to our convention, it
should be with 2 spaces. Since you have made changes here, could you please
make 2 spaces indentation?
---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1012 @vrozov, sorry for the misunderstanding. Currently, it is also not supported. ---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1012 @vrozov, If you meant [geojson](http://geojson.org) when saying about JSON format support, then my answer - not, it is not supported yet. ---
[GitHub] drill issue #1012: DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1012 @vrozov As I understand from the [release notes](https://github.com/Esri/geometry-api-java/releases/tag/v2.0.0), the main reason for the major version change was changing the interface due to the removing org.json dependency. Gis storage plugin provides UDF functions, and they are covered by the existing unit tests, so since all tests are passed this change does not break drill-gis. I think this change should be merged in the current release, but I don't know when other PR will be merged. Also, another pull request isn't affected by changing esri-geometry-api library, ie this change does not break changes made there. ---
[GitHub] drill pull request #1012: DRILL-5911: Upgrade esri-geometry-api version to 2...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/1012 DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to avoid depen⦠â¦dency on org.json library Please see [DRILL-5911](https://issues.apache.org/jira/browse/DRILL-5911) for details. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5911 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1012.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 #1012 commit 87e892515bfd731a088a3e03ffdc0bef184cf29c Author: Volodymyr Vysotskyi <vvo...@gmail.com> Date: 2017-10-27T14:45:17Z DRILL-5911: Upgrade esri-geometry-api version to 2.0.0 to avoid dependency on org.json library ---
[GitHub] drill issue #904: DRILL-5717: change some date time test cases with specific...
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/904
@weijietong thanks for the explanation of your problem. I was able to
reproduce it, but also I found working solution. This mock works correctly. The
problem appears when unit test is run with other tests:
[DateTimeZone](https://github.com/JodaOrg/joda-time/blob/ba95735daf79d00ce0928f30701d691f5e029d81/src/main/java/org/joda/time/DateTimeZone.java)
class contains static field
[cDefault](https://github.com/JodaOrg/joda-time/blob/ba95735daf79d00ce0928f30701d691f5e029d81/src/main/java/org/joda/time/DateTimeZone.java#L128)
which is used to receive timezone in `testToDateForTimeStamp()` and in other
tests. When timezone does not set to this field, but method
[getDefault()](https://github.com/JodaOrg/joda-time/blob/ba95735daf79d00ce0928f30701d691f5e029d81/src/main/java/org/joda/time/DateTimeZone.java#L149)
is called, value is taken from `System.getProperty()`.
When the first test that calls this method does not have a mock, it sets a
real value of timezone from `System.getProperty()`. Therefore our test uses a
unmocked value from `cDefault`.
So lets mock `DateTimeZone.getDefault()` method:
```
new MockUp() {
@Mock
public DateTimeZone getDefault() {
return DateTimeZone.UTC;
}
};
```
---
[GitHub] drill issue #904: DRILL-5717: change some date time test cases with specific...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/904 @weijietong which problems did you have when tried to mock `System.getProperty()` method? As the example of nice mock of this method, you may use [this test](https://github.com/apache/drill/pull/936/files#diff-b053496903bff0dc154e42604b93b9efR60). Also besides the timezone, property with locale should be mocked. ---
[GitHub] drill issue #972: DRILL-5838: Fix MaprDB filter pushdown for the case of nes...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/972 @paul-rogers, Thanks for the so informative comments, I completely agree with the convention that you have proposed. The problem which this PR is addressed, partially connected to the third point: > When calling back out to the external system, the interface (plugin) must convert from Drill format back out to the external system format. MaprDB uses `FieldPath` class to represent the field. Since the string, that is received in the method `JsonConditionBuilder.createJsonScanSpec()` from `SchemaPath` instance will be parsed using `org.ojai.FieldPath.parseFrom()` method, to avoid any ambiguousness, I converted `SchemaPath` to `FieldPath` and then called `FieldPath.asPathString()`. ---
[GitHub] drill pull request #980: DRILL-5857: Fix NumberFormatException in Hive unit ...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/980 DRILL-5857: Fix NumberFormatException in Hive unit tests There is no unit test since with or without this change tests are passes and query plan does not change. The exception that has been appeared caught and wrote into the logs. After the check on [this line](https://github.com/apache/drill/blob/3e8b01d5b0d3013e3811913f0fd6028b22c1ac3f/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveMetadataProvider.java#L90), correct stats was created calling [getStatsEstimateFromInputSplits()](https://github.com/apache/drill/blob/3e8b01d5b0d3013e3811913f0fd6028b22c1ac3f/contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveMetadataProvider.java#L95) method. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5857 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/980.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 #980 commit 24be497469a976fb51c16c0474774229e44fcc71 Author: Volodymyr Vysotskyi <vvo...@gmail.com> Date: 2017-10-09T14:28:42Z DRILL-5857: Fix NumberFormatException in Hive unit tests ---
[GitHub] drill pull request #972: DRILL-5838: Fix MaprDB filter pushdown for the case...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/972 DRILL-5838: Fix MaprDB filter pushdown for the case of nested field (reg. of DRILL-4264) Please see [DRILL-4264|https://issues.apache.org/jira/browse/DRILL-5838] for the problem description. `FieldPath.asPathString()` returns string in which field names may be quoted if `FieldPath` instance was created from the string, in which these fields were quoted. `SchemaPath.toExpr()` returns string were all field names are quoted. - Created new method in `SchemaPath` which quotes only field names which contain dots. - Fixed existing MaprDB unit tests to handle this case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5838 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/972.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 #972 ---
[GitHub] drill issue #510: DRILL-4139: Add missing BIT support for Parquet partition ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/510 @pmaciolek, could you please close this pull request, since it was merged in 1ea191fa351b29847e2358f5777982d602cf5ec3 ---
[GitHub] drill issue #805: Drill-4139: Exception while trying to prune partition. jav...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/805 This my bad habit to regenerate metadata left from the times before fix DRILL-5660. These functional test failures are the result of the mistake that I have made when resolving merge conflicts with DRILL-4264. After I made rebase, older 3.2 version still used in `ParquetReaderUtility` instead of 3.3. I fixed this bug and functional tests are pass without regenerating parquet metadata cache. Could you please take a look at this change in the last commit? ---
[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/905 Since Drill does not have enough information about tables in the planning time to avoid this OOM and since without using the `getMaxRowCount()` for `ScanPrel`, current approach could not be used and it would be better to defer the fix for this issue until the table statistics is implemented. ---
[GitHub] drill pull request #905: DRILL-1162: Fix OOM for hash join operator when the...
Github user vvysotskyi closed the pull request at: https://github.com/apache/drill/pull/905 ---
[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/905 @jinfengni it was an inflated example, but considering the case of multiple joins and when tables have several repeated values, the result will be the same. ---
[GitHub] drill issue #805: Drill-4139: Exception while trying to prune partition. jav...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/805 @paul-rogers, test data should be regenerated (using the Drill version with this fix) before running functional tests since these tests use metadata cache files. Regarding unit test failures: I rebased onto the master and run tests on my machine and on the test cluster and for both cases, tests finished without errors. So, perhaps it was a random test failure. ---
[GitHub] drill pull request #905: DRILL-1162: Fix OOM for hash join operator when the...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/905#discussion_r140730762
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdMaxRowCount.java
---
@@ -0,0 +1,71 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements. See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to you under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License. You may obtain a copy of the License at
+*
+* http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.apache.drill.exec.planner.cost;
+
+import org.apache.calcite.plan.volcano.RelSubset;
+import org.apache.calcite.rel.SingleRel;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.metadata.ReflectiveRelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMdMaxRowCount;
+import org.apache.calcite.rel.metadata.RelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.BuiltInMethod;
+import org.apache.drill.exec.planner.physical.AbstractPrel;
+import org.apache.drill.exec.planner.physical.ScanPrel;
+
+/**
+ * DrillRelMdMaxRowCount supplies a specific implementation of
+ * {@link RelMetadataQuery#getMaxRowCount} for Drill.
+ */
+public class DrillRelMdMaxRowCount extends RelMdMaxRowCount {
+
+ private static final DrillRelMdMaxRowCount INSTANCE = new
DrillRelMdMaxRowCount();
+
+ public static final RelMetadataProvider SOURCE =
ReflectiveRelMetadataProvider.reflectiveSource(BuiltInMethod.MAX_ROW_COUNT.method,
INSTANCE);
+
+ public Double getMaxRowCount(ScanPrel rel, RelMetadataQuery mq) {
+// the actual row count is known so returns its value
+return rel.estimateRowCount(mq);
--- End diff --
Taking into account that `ScanPrel` may return estimated value, I
completely agree with you that this change will cause problems during planning
time.
Thanks for pointing this.
---
[GitHub] drill pull request #942: DRILL-5781: Fix unit test failures to use tests con...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/942#discussion_r140213645 --- Diff: contrib/storage-hbase/src/test/resources/hbase-site.xml --- @@ -66,15 +66,13 @@ Default is 10. -
[GitHub] drill pull request #942: DRILL-5781: Fix unit test failures to use tests con...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/942#discussion_r140215549
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -100,6 +101,14 @@ public void run() {
return dir.getAbsolutePath() + File.separator + dirName;
}
+ /**
+ * Sets zookeeper server and client SASL test config properties.
+ */
+ public static void setZookeeperSaslTestConfigProps() {
+System.setProperty(ZooKeeperSaslServer.LOGIN_CONTEXT_NAME_KEY,
"Test_server");
--- End diff --
Thanks, replaced.
---
[GitHub] drill pull request #942: DRILL-5781: Fix unit test failures to use tests con...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/942#discussion_r140209669
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/PathUtils.java ---
@@ -70,4 +72,14 @@ public static final String normalize(final String path) {
return builder.toString();
}
+ /**
+ * Creates and returns path with the protocol at the beginning from
specified {@code url}.
+ */
--- End diff --
Thanks, done.
---
[GitHub] drill pull request #942: DRILL-5781: Fix unit test failures to use tests con...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/942#discussion_r140214189
--- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java
---
@@ -100,6 +101,14 @@ public void run() {
return dir.getAbsolutePath() + File.separator + dirName;
}
+ /**
+ * Sets zookeeper server and client SASL test config properties.
+ */
+ public static void setZookeeperSaslTestConfigProps() {
--- End diff --
Thanks, it looks better with these changes
---
[GitHub] drill pull request #805: Drill-4139: Exception while trying to prune partiti...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/805#discussion_r140057495
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/Metadata.java
---
@@ -1054,8 +1057,36 @@ public void setMax(Object max) {
return nulls;
}
-@Override public boolean hasSingleValue() {
- return (max != null && min != null && max.equals(min));
+/**
+ * Checks that the column chunk has a single value.
+ * Returns {@code true} if {@code min} and {@code max} are the same
but not null
+ * and nulls count is 0 or equal to the rows count.
+ *
+ * Returns {@code true} if {@code min} and {@code max} are null and
the number of null values
+ * in the column chunk is equal to the rows count.
+ *
+ * Comparison of nulls and rows count is needed for the cases:
+ *
+ * column with primitive type has single value and null values
+ *
+ * column with primitive type has only null values, min/max
couldn't be null,
+ * but column has single value
+ *
+ *
+ * @param rowCount rows count in column chunk
+ * @return true if column has single value
+ */
+@Override
+public boolean hasSingleValue(long rowCount) {
+ if (nulls != null) {
+if (min != null) {
+ // Objects.deepEquals() is used here, since min and max may be
byte arrays
+ return Objects.deepEquals(min, max) && (nulls == 0 || nulls ==
rowCount);
--- End diff --
Statistics [1] for most parquet types use java primitive types to store min
and max values, so min/max can not be null even if the table has null values.
[1]
https://github.com/apache/parquet-mr/tree/e54ca615f213f5db6d34d9163c97eec98920d7a7/parquet-column/src/main/java/org/apache/parquet/column/statistics
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r137591528
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java
---
@@ -135,11 +136,16 @@ public void testIntervalArithmetic() throws Exception
{
@Test
public void testToChar() throws Exception {
-
-String expectedResults[] = {(new LocalDate(2008, 2,
23)).toString("-MMM-dd"),
+Locale defaultLocale = Locale.getDefault();
+try{
--- End diff --
Please add spaces in all places where required.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r137590193 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java --- @@ -32,17 +29,21 @@ import org.apache.drill.exec.server.RemoteServiceSet; import org.apache.drill.exec.vector.ValueVector; import org.joda.time.LocalDate; -import org.joda.time.LocalTime; import org.joda.time.LocalDateTime; +import org.joda.time.LocalTime; import org.junit.Ignore; import org.junit.Test; -import com.google.common.base.Charsets; -import com.google.common.io.Files; +import java.util.List; --- End diff -- Please remove unused imports in all classes where you have made changes. ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r13824
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/testing/TestDateConversions.java
---
@@ -99,106 +100,142 @@ public void testPostgresDate() throws Exception {
@Test
public void testJodaTime() throws Exception {
-String query = String.format("SELECT to_time(time1, 'H:m:ss') = "
- + "to_time(time2, 'h:m:ssa') as col1, "
- + "to_time(time1, 'H:m:ss') = "
- + "to_time(time3, 'ssmha') as col2 "
- + "from dfs_test.`%s/joda_postgres_time.json`", TEMP_DIR);
+Locale defaultLocale = Locale.getDefault();
+try {
+ Locale.setDefault(new Locale("en", "US"));
--- End diff --
Please replace all occurrences of `new Locale("en", "US")` by `Locale.US`
in all your changes.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi closed the pull request at: https://github.com/apache/drill/pull/930 ---
[GitHub] drill pull request #942: DRILL-5781: Fix unit test failures to use tests con...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/942 DRILL-5781: Fix unit test failures to use tests config even if default config is available Please see [DRILL-5781](https://issues.apache.org/jira/browse/DRILL-5781) for details. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5781 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/942.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 #942 commit b47b4d760adc62e1625d23e80aae611a54ea9e28 Author: Volodymyr Vysotskyi <vvo...@gmail.com> Date: 2017-09-07T18:01:12Z DRILL-5781: Fix unit test failures to use tests config even if default config is available ---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/930 @paul-rogers, @vrozov, thanks for the review. Squashed commits and rebased onto master. ---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138620064
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,92 @@
+
+
+
+
+
+
+
+true
+1
+true
+${LILITH_HOSTNAME:-localhost}
+
+
+
+
+
+
+
+ %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n
+
+
+
+
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138622881
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+
+
+
+
+
+
+
+true
+1
+true
+${LILITH_HOSTNAME:-localhost}
+
+
+
+
+
+
+
+ %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n
+
+
+
+
+
+
+
--- End diff --
Without comment with file appender yes, it can. Thanks, done.
---
[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/905 @jinfengni, the idea is very interesting for me. More appropriate memory cost estimation will cause to choosing better plans, that will cause the less time execution and memory costs. Writing about the comparing the ratio of column count vs ratio of row count I meant that the multiplying of row count and column count does not help to choose which input needs more memory in the case when the ratio of **actual** row count is much greater than the ratio of column count. `getCumulativeMemCost` implementation will not help for this case. To show that, let's consider an example. We have three tables: - a(has 5 columns and 5 rows with the same values); - b(has 5 columns and 10 rows with the same values as in the table a); - c(has 5 columns and 35 rows with the same values as in the table a). For the query ``` select count(*) from a inner join b on a.col1=b.col1 inner join c on b.col1=c.col1; ``` Drill will build plan ``` HahJoin[1] /\ c HahJoin[2] /\ b a ``` `getCumulativeMemCost` for HahJoin[1] will return a value proportional to `(HahJoin[2] row count) * (HahJoin[2] column count) + HahJoin[2].getCumulativeMemCost() = Max(aRowCount, bRowCount) * (aColumnCount + bColumnCount) + aRowCount * aColumnCount = 5 * (5 + 5) + 5 * 5 = 75`. Actual row count for build side of HahJoin[1] in this case is 50. For the plan, that will be more suitable for this particular case: ``` HahJoin[1] / \ HahJoin[2] c /\ b a ``` `getCumulativeMemCost` for HahJoin[1] will return a value proportional to `cRowCount * cColumnCount = 35 * 5 = 175`. Actual row count for build side of HahJoin[1] in this case is 35. Smal information about this pull request. This pull request addresses only the case of large row count. It checks that OOM may happen and if swap allows avoiding this potential OOM, the swap will happen. ---
[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/905 @jinfengni thanks for looking into this. Completely agree with you that it would be better to consider both row and column count. Unfortunately, it does not help to fix this issue, since the ratio of column count of tables very often much smaller than the ratio of row count (actual row count). (`c1/c2 << r2/r1`) So I guess it makes sense to implement your proposal to consider the column count together with the row count in another pull request. ---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138102613
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+
+
+
+
+
+
+
+true
+1
+true
+${LILITH_HOSTNAME:-localhost}
+
+
+
+
+
+
+
+ %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
--- End diff --
Initially, I assumed that it would be useful for those, who work with the
mapr-format-plugin. I agree with you that we should delete this logger.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138103031
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+
+
+
+
+
+
+
+true
+1
+true
+${LILITH_HOSTNAME:-localhost}
+
+
+
+
+
+
+
+ %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
--- End diff --
Thanks for pointing this, already removed.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r138100617
--- Diff: common/src/test/resources/logback-test.xml ---
@@ -0,0 +1,111 @@
+
+
+
+
+
+
+
+true
+1
+true
+${LILITH_HOSTNAME:-localhost}
+
+
+
+
+
+
+
+ %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n
+
+
+
+
+
+
+
--- End diff --
This logger contains only Lilith appender, and since we are using Lilith
only for debugging separate tests, I think it would be better to left logging
level here at `DEBUG`. In this case, everyone who uses Lilith won't need to
change logback file. Console appender is used in the root logger and it has
`ERROR` logging level.
---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/930#discussion_r137252146
--- Diff: common/src/test/resources/logback.xml ---
@@ -16,17 +16,22 @@
1
true
${LILITH_HOSTNAME:-localhost}
+
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi closed the pull request at: https://github.com/apache/drill/pull/909 ---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/930 I have implemented the proposal of @vrozov to conditionally enable Lilith appender. I have used `ThresholdFilter` in Lilith appender, so it was not needed to change all places where it is used. An example of how to enable Lilith was added to the Jira description. @jinfengni, @vrozov could you please take a look? ---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/930 Yes, we can change hiveserver2 port #, but I think that it would be better to disable Lilith by default since hiveserver2 port number may be used in the configs of other applications and it would be cumbersome to change all that configs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/930 @priteshm, as I have mentioned in the Jira [DRILL-5761](https://issues.apache.org/jira/browse/DRILL-5761), port in Lilith UI cannot be changed (https://github.com/huxi/lilith/issues/10). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender ...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/930 Unit tests hang when Lilith application is not running, but when `ClassicMultiplexSocketAppender` in `logback.xml` is defined and used in the loggers. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...
GitHub user vvysotskyi opened a pull request: https://github.com/apache/drill/pull/930 DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender by default You can merge this pull request into a Git repository by running: $ git pull https://github.com/vvysotskyi/drill DRILL-5761 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/930.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 #930 commit be17b285747b0ca867b6a3733c17c1b8187c7a6d Author: Volodymyr Vysotskyi <vvo...@gmail.com> Date: 2017-08-31T12:52:43Z DRILL-5761: Disable Lilith ClassicMultiplexSocketAppender by default --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #927: DRILL-5751: Fix unit tests to use local file system even i...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/927 LGTM! +1. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #927: DRILL-5751: Fix unit tests to use local file system...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/927#discussion_r136373928
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java ---
@@ -120,8 +132,11 @@ public void testDisableDynamicSupport() throws
Exception {
@Test
public void testAbsentBinaryInStaging() throws Exception {
Path staging =
getDrillbitContext().getRemoteFunctionRegistry().getStagingArea();
+FileSystem fs =
getDrillbitContext().getRemoteFunctionRegistry().getFs();
+copyJar(fs, jars, staging, default_binary_name);
--- End diff --
It seems that a source file should be copied here instead of the binary
file, since test checks that binary file is missing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #927: DRILL-5751: Fix unit tests to use local file system...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/927#discussion_r136384386
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTestBase.java
---
@@ -64,7 +63,9 @@
@BeforeClass
public static void setup() throws IOException {
// initialize file system
-fs = FileSystem.get(new Configuration());
+Configuration configuration = new Configuration();
+configuration.set(FileSystem.FS_DEFAULT_NAME_KEY,
FileSystem.DEFAULT_FS);
+fs = FileSystem.get(configuration);
--- End diff --
These three lines appear several times in the code, so may be it would be
better to create a static method which returns `FileSystem` instance.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r136016798
--- Diff:
logical/src/main/java/org/apache/drill/common/expression/SchemaPath.java ---
@@ -115,6 +112,33 @@ public static SchemaPath create(NamePart namePart) {
}
/**
+ * Parses input string and returns {@code SchemaPath} instance.
+ *
+ * @param expr input string to be parsed
+ * @return {@code SchemaPath} instance
+ */
+ public static SchemaPath parseFromString(String expr) {
--- End diff --
Done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r135848539 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java --- @@ -22,14 +22,15 @@ import org.apache.drill.BaseTestQuery; import org.apache.drill.common.util.FileUtils; import org.joda.time.DateTime; +import org.junit.BeforeClass; --- End diff -- Please remove unused imports. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r135849992
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -73,16 +74,23 @@ public void testCastByConstantFolding() throws
Exception {
@Test // DRILL-3769
public void testToDateForTimeStamp() throws Exception {
-final String query = "select to_date(to_timestamp(-1)) as col \n" +
-"from (values(1))";
+String defaultTimezone = System.getProperty("user.timezone");
+try {
+ System.setProperty("user.timezone","Etc/GMT");
--- End diff --
Please add space after the comma.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r135850298
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -73,16 +74,23 @@ public void testCastByConstantFolding() throws
Exception {
@Test // DRILL-3769
public void testToDateForTimeStamp() throws Exception {
-final String query = "select to_date(to_timestamp(-1)) as col \n" +
-"from (values(1))";
+String defaultTimezone = System.getProperty("user.timezone");
+try {
+ System.setProperty("user.timezone","Etc/GMT");
+ final String query = "select to_date(to_timestamp(-1)) as col \n"
++ "from (values(1))";
-testBuilder()
+ testBuilder()
.sqlQuery(query)
.ordered()
.baselineColumns("col")
.baselineValues(new DateTime(1969, 12, 31, 0, 0))
.build()
.run();
+}finally {
+ System.setProperty("user.timezone",defaultTimezone);
--- End diff --
space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r135850258
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -73,16 +74,23 @@ public void testCastByConstantFolding() throws
Exception {
@Test // DRILL-3769
public void testToDateForTimeStamp() throws Exception {
-final String query = "select to_date(to_timestamp(-1)) as col \n" +
-"from (values(1))";
+String defaultTimezone = System.getProperty("user.timezone");
+try {
+ System.setProperty("user.timezone","Etc/GMT");
+ final String query = "select to_date(to_timestamp(-1)) as col \n"
++ "from (values(1))";
-testBuilder()
+ testBuilder()
.sqlQuery(query)
.ordered()
.baselineColumns("col")
.baselineValues(new DateTime(1969, 12, 31, 0, 0))
.build()
.run();
+}finally {
--- End diff --
please add space: `} finally {`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/904#discussion_r135850825 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java --- @@ -20,7 +20,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.io.IOException; --- End diff -- unused imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #904: DRILL-5717: change some date time test cases with s...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r135851159
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java
---
@@ -135,11 +139,16 @@ public void testIntervalArithmetic() throws Exception
{
@Test
public void testToChar() throws Exception {
-
-String expectedResults[] = {(new LocalDate(2008, 2,
23)).toString("-MMM-dd"),
+Locale defaultLocale = Locale.getDefault();
+try{
+ Locale.setDefault(new Locale("en", "US"));
--- End diff --
Please replace `new Locale("en", "US")` by `Locale.US`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r135753493
--- Diff:
contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/binary/CompareFunctionsProcessor.java
---
@@ -82,466 +45,37 @@ public static CompareFunctionsProcessor
process(FunctionCall call, boolean nullC
LogicalExpression swapArg = valueArg;
valueArg = nameArg;
nameArg = swapArg;
-evaluator.functionName =
COMPARE_FUNCTIONS_TRANSPOSE_MAP.get(functionName);
+
evaluator.setFunctionName(COMPARE_FUNCTIONS_TRANSPOSE_MAP.get(functionName));
}
- evaluator.success = nameArg.accept(evaluator, valueArg);
+ evaluator.setSuccess(nameArg.accept(evaluator, valueArg));
} else if (nullComparatorSupported && call.args.get(0) instanceof
SchemaPath) {
- evaluator.success = true;
- evaluator.path = (SchemaPath) nameArg;
+ evaluator.setSuccess(true);
+ evaluator.setPath((SchemaPath) nameArg);
}
return evaluator;
}
- public CompareFunctionsProcessor(String functionName) {
-this.success = false;
-this.functionName = functionName;
-this.isEqualityFn =
COMPARE_FUNCTIONS_TRANSPOSE_MAP.containsKey(functionName)
-&&
COMPARE_FUNCTIONS_TRANSPOSE_MAP.get(functionName).equals(functionName);
-this.isRowKeyPrefixComparison = false;
-this.sortOrderAscending = true;
- }
-
- public byte[] getValue() {
-return value;
- }
-
- public boolean isSuccess() {
-return success;
- }
-
- public SchemaPath getPath() {
-return path;
- }
-
- public String getFunctionName() {
-return functionName;
- }
-
- public boolean isRowKeyPrefixComparison() {
- return isRowKeyPrefixComparison;
- }
-
- public byte[] getRowKeyPrefixStartRow() {
-return rowKeyPrefixStartRow;
- }
-
- public byte[] getRowKeyPrefixStopRow() {
- return rowKeyPrefixStopRow;
- }
-
- public Filter getRowKeyPrefixFilter() {
- return rowKeyPrefixFilter;
- }
-
- public boolean isSortOrderAscending() {
-return sortOrderAscending;
- }
-
@Override
- public Boolean visitCastExpression(CastExpression e, LogicalExpression
valueArg) throws RuntimeException {
-if (e.getInput() instanceof CastExpression || e.getInput() instanceof
SchemaPath) {
- return e.getInput().accept(this, valueArg);
-}
-return false;
- }
-
- @Override
- public Boolean visitConvertExpression(ConvertExpression e,
LogicalExpression valueArg) throws RuntimeException {
-if (e.getConvertFunction() == ConvertExpression.CONVERT_FROM) {
-
- String encodingType = e.getEncodingType();
- int prefixLength= 0;
-
- // Handle scan pruning in the following scenario:
- // The row-key is a composite key and the CONVERT_FROM() function
has byte_substr() as input function which is
- // querying for the first few bytes of the row-key(start-offset 1)
- // Example WHERE clause:
- // CONVERT_FROM(BYTE_SUBSTR(row_key, 1, 8), 'DATE_EPOCH_BE') < DATE
'2015-06-17'
- if (e.getInput() instanceof FunctionCall) {
-
-// We can prune scan range only for big-endian encoded data
-if (encodingType.endsWith("_BE") == false) {
- return false;
-}
-
-FunctionCall call = (FunctionCall)e.getInput();
-String functionName = call.getName();
-if (!functionName.equalsIgnoreCase("byte_substr")) {
- return false;
-}
-
-LogicalExpression nameArg = call.args.get(0);
-LogicalExpression valueArg1 = call.args.size() >= 2 ?
call.args.get(1) : null;
-LogicalExpression valueArg2 = call.args.size() >= 3 ?
call.args.get(2) : null;
-
-if (((nameArg instanceof SchemaPath) == false) ||
- (valueArg1 == null) || ((valueArg1 instanceof IntExpression)
== false) ||
- (valueArg2 == null) || ((valueArg2 instanceof IntExpression)
== false)) {
- return false;
-}
-
-boolean isRowKey =
((SchemaPath)nameArg).getAsUnescapedPath().equals(DrillHBaseConstants.ROW_KEY);
-int offset = ((IntExpression)valueArg1).getInt();
-
-if (!isRowKey || (offset != 1)) {
- return false;
-}
-
-this.path= (SchemaPath)nameArg;
-prefixLength = ((IntExpression)valueArg2).getInt();
-this.isRowKeyPrefixComparison = true;
-return visitRowKeyPrefi
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134778939
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java
---
@@ -93,4 +93,23 @@ public void testProjectionMultipleFiles() throws
Exception {
.go();
}
+ @Test //DRILL-4264
+ public void testFieldNameWithDot() throws Exception {
--- End diff --
Added more tests
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134788919
--- Diff:
exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/TupleAccessor.java
---
@@ -48,9 +48,21 @@
MaterializedField column(int index);
-MaterializedField column(String name);
+/**
+ * Returns {@code MaterializedField} instance from schema using the
name path specified in param.
+ *
+ * @param name full name path of the column in the schema
+ * @return {@code MaterializedField} instance
+ */
+MaterializedField column(String[] name);
--- End diff --
Thanks, reverted my changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134539810
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
---
@@ -359,30 +361,109 @@ public Mutator(OperatorExecContext oContext,
BufferAllocator allocator, VectorCo
public T addField(MaterializedField field,
Class clazz) throws
SchemaChangeException {
// Check if the field exists.
- ValueVector v = fieldVectorMap.get(field.getPath());
- if (v == null || v.getClass() != clazz) {
+ ValueVector vector = fieldVectorMap.get(field.getName());
+ ValueVector childVector = vector;
+ // if a vector does not exist yet, creates value vector, or if it
exists and has map type, omit this code
+ if (vector == null || (vector.getClass() != clazz
+&& (vector.getField().getType().getMinorType() != MinorType.MAP
+|| field.getType().getMinorType() != MinorType.MAP))) {
// Field does not exist--add it to the map and the output
container.
-v = TypeHelper.getNewVector(field, allocator, callBack);
-if (!clazz.isAssignableFrom(v.getClass())) {
+vector = TypeHelper.getNewVector(field, allocator, callBack);
+childVector = vector;
+// gets inner field if the map was created the first time
+if (field.getType().getMinorType() == MinorType.MAP) {
+ childVector = getChildVectorByField(vector, field);
+} else if (!clazz.isAssignableFrom(vector.getClass())) {
throw new SchemaChangeException(
String.format(
"The class that was provided, %s, does not correspond to
the "
+ "expected vector type of %s.",
- clazz.getSimpleName(), v.getClass().getSimpleName()));
+ clazz.getSimpleName(),
vector.getClass().getSimpleName()));
}
-final ValueVector old = fieldVectorMap.put(field.getPath(), v);
+final ValueVector old = fieldVectorMap.put(field.getName(),
vector);
if (old != null) {
old.clear();
container.remove(old);
}
-container.add(v);
+container.add(vector);
// Added new vectors to the container--mark that the schema has
changed.
schemaChanged = true;
}
+ // otherwise, checks that field and existing vector have a map type
--- End diff --
I was suggesting that the work here may be produced on the nested fields
thru the map.
I agree with you that it would be correct to deal with the desired field.
So thanks for pointing this, I reverted the changes in this method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134686740
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
---
@@ -359,30 +361,109 @@ public Mutator(OperatorExecContext oContext,
BufferAllocator allocator, VectorCo
public T addField(MaterializedField field,
Class clazz) throws
SchemaChangeException {
// Check if the field exists.
- ValueVector v = fieldVectorMap.get(field.getPath());
- if (v == null || v.getClass() != clazz) {
+ ValueVector vector = fieldVectorMap.get(field.getName());
+ ValueVector childVector = vector;
+ // if a vector does not exist yet, creates value vector, or if it
exists and has map type, omit this code
+ if (vector == null || (vector.getClass() != clazz
+&& (vector.getField().getType().getMinorType() != MinorType.MAP
+|| field.getType().getMinorType() != MinorType.MAP))) {
// Field does not exist--add it to the map and the output
container.
-v = TypeHelper.getNewVector(field, allocator, callBack);
-if (!clazz.isAssignableFrom(v.getClass())) {
+vector = TypeHelper.getNewVector(field, allocator, callBack);
+childVector = vector;
+// gets inner field if the map was created the first time
+if (field.getType().getMinorType() == MinorType.MAP) {
+ childVector = getChildVectorByField(vector, field);
+} else if (!clazz.isAssignableFrom(vector.getClass())) {
throw new SchemaChangeException(
String.format(
"The class that was provided, %s, does not correspond to
the "
+ "expected vector type of %s.",
- clazz.getSimpleName(), v.getClass().getSimpleName()));
+ clazz.getSimpleName(),
vector.getClass().getSimpleName()));
}
-final ValueVector old = fieldVectorMap.put(field.getPath(), v);
+final ValueVector old = fieldVectorMap.put(field.getName(),
vector);
if (old != null) {
old.clear();
container.remove(old);
}
-container.add(v);
+container.add(vector);
// Added new vectors to the container--mark that the schema has
changed.
schemaChanged = true;
}
+ // otherwise, checks that field and existing vector have a map type
+ // and adds child fields from the field to the vector
+ else if (field.getType().getMinorType() == MinorType.MAP
+ && vector.getField().getType().getMinorType() ==
MinorType.MAP
+ && !field.getChildren().isEmpty()) {
+// an incoming field contains only single child since it determines
+// full name path of the field in the schema
+childVector = addNestedChildToMap((MapVector) vector,
Iterables.getLast(field.getChildren()));
+schemaChanged = true;
+ }
- return clazz.cast(v);
+ return clazz.cast(childVector);
+}
+
+/**
+ * Finds and returns value vector which path corresponds to the
specified field.
+ * If required vector is nested in the map, gets and returns this
vector from the map.
+ *
+ * @param valueVector vector that should be checked
+ * @param field field that corresponds to required vector
+ * @return value vector whose path corresponds to the specified field
+ *
+ * @throws SchemaChangeException if the field does not correspond to
the found vector
+ */
+private ValueVector getChildVectorByField(ValueVector valueVector,
+ MaterializedField field)
throws SchemaChangeException {
+ if (field.getChildren().isEmpty()) {
+if (valueVector.getField().equals(field)) {
+ return valueVector;
+} else {
+ throw new SchemaChangeException(
+String.format(
+ "The field that was provided, %s, does not correspond to the
"
++ "expected vector type of %s.",
+ field, valueVector.getClass().getSimpleName()));
+}
+ } else {
+// an incoming field contains only single child since it determines
+// full name path of the field in the schema
+MaterializedField childField =
Iterables.getLast(field.getChildren());
+return getChildVectorByField(((MapVector)
valueVector).getChild(childField.getName()), childField);
+ }
+}
+
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/909#discussion_r134688368 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java --- @@ -293,7 +294,7 @@ private StreamingAggregator createAggregatorInternal() throws SchemaChangeExcept continue; } keyExprs[i] = expr; - final MaterializedField outputField = MaterializedField.create(ne.getRef().getAsUnescapedPath(), expr.getMajorType()); + final MaterializedField outputField = SchemaPathUtil.getMaterializedFieldFromSchemaPath(ne.getRef(), expr.getMajorType()); --- End diff -- Yes, it should. But `MaterializedField` class is in the `vector` module, `SchemaPath` class is in the `drill-logical` module and `vector` module does not have the dependency on the `drill-logical` module. Replaced this code by the `MaterializedField.create(ne.getRef().getLastSegment().getNameSegment().getPath(), expr.getMajorType());` since simple name path is used here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134764401
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/SchemaPathUtil.java
---
@@ -0,0 +1,59 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements. See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to you under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License. You may obtain a copy of the License at
+*
+* http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.apache.drill.exec.vector.complex;
+
+import org.apache.drill.common.expression.PathSegment;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.types.TypeProtos;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.record.MaterializedField;
+
+public class SchemaPathUtil {
--- End diff --
Removed this class, since all code where it was used, uses simple name path
so it is not needed anymore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134782398
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java
---
@@ -84,4 +89,22 @@ public void testClone() {
}
+ @Test // DRILL-4264
+ public void testSchemaPathToMaterializedFieldConverting() {
--- End diff --
This test was designed to check the
`SchemaPathUtil.getMaterializedFieldFromSchemaPath()` method. Since this method
removed, I removed this test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134787569
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/test/rowSet/RowSetSchema.java ---
@@ -83,7 +85,13 @@ private void updateStructure(int index, PhysicalSchema
children) {
public boolean isMap() { return mapSchema != null; }
public PhysicalSchema mapSchema() { return mapSchema; }
public MaterializedField field() { return field; }
-public String fullName() { return fullName; }
+
+/**
+ * Returns full name path of the column.
--- End diff --
I reverted these changes. Also, I commented out the test where this code is
used with the map fields.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134540459
--- Diff:
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/CompareFunctionsProcessor.java
---
@@ -147,10 +147,10 @@ public Boolean visitCastExpression(CastExpression e,
LogicalExpression valueArg)
@Override
public Boolean visitConvertExpression(ConvertExpression e,
LogicalExpression valueArg) throws RuntimeException {
-if (e.getConvertFunction() == ConvertExpression.CONVERT_FROM) {
+if (ConvertExpression.CONVERT_FROM.equals(e.getConvertFunction())) {
--- End diff --
Since both these classes almost the same, I moved mutual code to the
abstract class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134788014
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/test/rowSet/RowSetSchema.java ---
@@ -94,12 +102,20 @@ private void updateStructure(int index, PhysicalSchema
children) {
*/
public static class NameSpace {
-private final Map<String,Integer> nameSpace = new HashMap<>();
+private final Map<String, Integer> nameSpace = new HashMap<>();
private final List columns = new ArrayList<>();
-public int add(String key, T value) {
+/**
+ * Adds column path with specified value to the columns list
+ * and returns the index of the column in the list.
+ *
+ * @param key full name path of the column in the schema
+ * @param value value to be added to the list
+ * @return index of the column in the list
+ */
+public int add(String[] key, T value) {
int index = columns.size();
- nameSpace.put(key, index);
+ nameSpace.put(SchemaPath.getCompoundPath(key).toExpr(), index);
--- End diff --
Thanks for the explanation, reverted my changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134795741
--- Diff:
logical/src/main/java/org/apache/drill/common/expression/SchemaPath.java ---
@@ -115,6 +112,33 @@ public static SchemaPath create(NamePart namePart) {
}
/**
+ * Parses input string and returns {@code SchemaPath} instance.
+ *
+ * @param expr input string to be parsed
+ * @return {@code SchemaPath} instance
+ */
+ public static SchemaPath parseFromString(String expr) {
--- End diff --
It parses a string using the same rules which are used for the field in the
query. If a string contains dot outside backticks, or there are no backticks in
the string, will be created `SchemaPath` with the `NameSegment` which contains
one else `NameSegment`, etc. If a string contains [] then `ArraySegment` will
be created.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #909: DRILL-4264: Allow field names to include dots
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/909#discussion_r134689741
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
---
@@ -362,16 +363,16 @@ protected boolean setupNewSchema() throws
SchemaChangeException {
final TransferPair tp = vvIn.makeTransferPair(vvOut);
transfers.add(tp);
}
- } else if (value != null && value.intValue() > 1) { //
subsequent wildcards should do a copy of incoming valuevectors
+ } else if (value != null && value > 1) { // subsequent wildcards
should do a copy of incoming valuevectors
int k = 0;
for (final VectorWrapper wrapper : incoming) {
final ValueVector vvIn = wrapper.getValueVector();
- final SchemaPath originalPath =
SchemaPath.getSimplePath(vvIn.getField().getPath());
- if (k > result.outputNames.size()-1) {
+ final SchemaPath originalPath =
SchemaPath.getSimplePath(vvIn.getField().getName());
+ if (k > result.outputNames.size() - 1) {
assert false;
}
final String name = result.outputNames.get(k++); // get the
renamed column names
- if (name == EMPTY_STRING) {
+ if (EMPTY_STRING.equals(name)) {
--- End diff --
Thanks, replaced by `name.isEmpty()`, but `EMPTY_STRING` is used in other
places, so left it in the class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: let some date time test cases be Local ...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r134262565
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestCastFunctions.java
---
@@ -30,6 +31,11 @@
public class TestCastFunctions extends BaseTestQuery {
+ @BeforeClass
+ public static void setupLocal() {
+System.setProperty("user.timezone","Etc/GMT");
--- End diff --
We should change the timezone in the same way as I proposed for the locale.
Also since we fix test failures connected with the timezone, please update
Jira and pull request titles.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: let some date time test cases be Local ...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r134260296
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewDateFunctions.java
---
@@ -51,9 +51,9 @@ public void testIsDate() throws Exception {
.sqlQuery("select case when isdate(date1) then cast(date1 as date)
else null end res1 from " + dateValues)
.unOrdered()
.baselineColumns("res1")
-.baselineValues(new DateTime(Date.valueOf("1900-01-01").getTime()))
-.baselineValues(new DateTime(Date.valueOf("3500-01-01").getTime()))
-.baselineValues(new DateTime(Date.valueOf("2000-12-31").getTime()))
+.baselineValues(new DateTime(1900,1,1,0,0))
+.baselineValues(new DateTime(3500,1,1,0,0))
+.baselineValues(new DateTime(2000,12,31,0,0))
--- End diff --
Please add spaces after the comma.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #904: DRILL-5717: let some date time test cases be Local ...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/904#discussion_r134259055
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java
---
@@ -43,6 +46,11 @@
public class TestDateFunctions extends PopUnitTestBase {
static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(TestDateFunctions.class);
+@BeforeClass
+public static void setupLocal() {
+Locale.setDefault(new Locale("en", "US"));
--- End diff --
This change also affects other unit tests which will be executed after the
tests in this class.
So to avoid this, for each unit test which depends on the locale, we should:
1. preserve current locale
2. change locale to "en"
3. execute test (in the try block)
4. restore locale (in the finally block).
As the example, you may use test testConstantFolding_allTypes() from the
class TestConstantFolding below.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #908: DRILL-5725: Update Jackson version to 2.7.8
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/908 Maven uses 'nearest-win' strategy to resolve conflicts and since we specify the Jackson library in the pom file, there is no need to exclude it from other libraries. Therefore the result of the command `mvn dependency:tree | grep com.fasterxml.jackson` is ``` [INFO] +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | \- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | \- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | \- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.module:jackson-module-afterburner:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.module:jackson-module-afterburner:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.module:jackson-module-afterburner:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.module:jackson-module-afterburner:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.module:jackson-module-afterburner:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.8:compile [INFO] | | +- com.fasterxml.jackson.core:jackson-core:jar:2.7.8:compile [INFO] | | \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.8:compile [INFO] | +- com.fasterxml.jackson.module:jackson-module-afterburner:jar
[GitHub] drill pull request #805: Drill-4139: Exception while trying to prune partiti...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/805#discussion_r133927602
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
---
@@ -548,23 +567,57 @@ public void populatePruningVector(ValueVector v, int
index, SchemaPath column, S
NullableVarCharVector varCharVector = (NullableVarCharVector) v;
Object s = partitionValueMap.get(f).get(column);
byte[] bytes;
-if (s instanceof String) { // if the metadata was read from a JSON
cache file it maybe a string type
- bytes = ((String) s).getBytes();
-} else if (s instanceof Binary) {
- bytes = ((Binary) s).getBytes();
-} else if (s instanceof byte[]) {
- bytes = (byte[]) s;
+if (s == null) {
+ varCharVector.getMutator().setNull(index);
+ return;
} else {
- throw new UnsupportedOperationException("Unable to create column
data for type: " + type);
+ bytes = getBytes(type, s);
}
varCharVector.getMutator().setSafe(index, bytes, 0, bytes.length);
return;
}
+ case INTERVAL: {
+NullableIntervalVector intervalVector = (NullableIntervalVector) v;
+Object s = partitionValueMap.get(f).get(column);
+byte[] bytes;
+if (s == null) {
+ intervalVector.getMutator().setNull(index);
+ return;
+} else {
+ bytes = getBytes(type, s);
+}
+intervalVector.getMutator().setSafe(index, 1,
+ ParquetReaderUtility.getIntFromLEBytes(bytes, 0),
+ ParquetReaderUtility.getIntFromLEBytes(bytes, 4),
+ ParquetReaderUtility.getIntFromLEBytes(bytes, 8));
+return;
+ }
default:
throw new UnsupportedOperationException("Unsupported type: " +
type);
}
}
+ /**
+ * Returns the sequence of bytes received from {@code Object source}.
+ *
+ * @param type the column type
+ * @param source the source of the bytes sequence
+ * @return bytes sequence obtained from {@code Object source}
+ */
+ private byte[] getBytes(MinorType type, Object source) {
+byte[] bytes;
+if (source instanceof String) { // if the metadata was read from a
JSON cache file it maybe a string type
+ bytes = Base64.decodeBase64(((String) source).getBytes());
--- End diff --
> Note, however, that the casting is unfortunate. The caller knows the
type. Better to have:
The caller does not know the type since `source` is received from the map
of objects. So the casting should be used there.
> Seems we'd also need a to/from Base64 method so that, on read, we do:
Value vector mutator interface does not have a method that allows passing
bytes. Such methods implemented in the concrete implementation of mutators, so
we could not create such to/from Base64 method.
> Strings need not be Base64 encoded
They should be Base64 encoded since byte array may have an encoding that
differs from the UTF-8.
I moved this conversion from the string to a byte array into the
`ParquetReaderUtility.correctBinaryInMetadataCache()` method. So now we are
considering the metadata version and chose the way how to encode the string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #805: Drill-4139: Exception while trying to prune partiti...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/805#discussion_r126637301
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
---
@@ -444,123 +478,235 @@ public MajorType getTypeForColumn(SchemaPath
schemaPath) {
public void populatePruningVector(ValueVector v, int index, SchemaPath
column, String file) {
String f = Path.getPathWithoutSchemeAndAuthority(new
Path(file)).toString();
-MinorType type = getTypeForColumn(column).getMinorType();
+MajorType majorType = getTypeForColumn(column);
+MinorType type = majorType.getMinorType();
switch (type) {
+ case BIT: {
+NullableBitVector bitVector = (NullableBitVector) v;
+Boolean value = (Boolean) partitionValueMap.get(f).get(column);
+if (value == null) {
+ bitVector.getMutator().setNull(index);
+} else {
+ bitVector.getMutator().setSafe(index, value ? 1 : 0);
+}
+return;
+ }
case INT: {
NullableIntVector intVector = (NullableIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value);
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value);
+}
return;
}
case SMALLINT: {
NullableSmallIntVector smallIntVector = (NullableSmallIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-smallIntVector.getMutator().setSafe(index, value.shortValue());
+if (value == null) {
+ smallIntVector.getMutator().setNull(index);
+} else {
+ smallIntVector.getMutator().setSafe(index, value.shortValue());
+}
return;
}
case TINYINT: {
NullableTinyIntVector tinyIntVector = (NullableTinyIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-tinyIntVector.getMutator().setSafe(index, value.byteValue());
+if (value == null) {
+ tinyIntVector.getMutator().setNull(index);
+} else {
+ tinyIntVector.getMutator().setSafe(index, value.byteValue());
+}
return;
}
case UINT1: {
NullableUInt1Vector intVector = (NullableUInt1Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value.byteValue());
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value.byteValue());
+}
return;
}
case UINT2: {
NullableUInt2Vector intVector = (NullableUInt2Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, (char) value.shortValue());
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, (char) value.shortValue());
+}
return;
}
case UINT4: {
NullableUInt4Vector intVector = (NullableUInt4Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value);
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value);
+}
return;
}
case BIGINT: {
NullableBigIntVector bigIntVector = (NullableBigIntVector) v;
Long value = (Long) partitionValueMap.get(f).get(column);
-bigIntVector.getMutator().setSafe(index, value);
+if (value == null) {
+ bigIntVector.getMutator().setNull(index);
+} else {
+ bigIntVector.getMutator().setSafe(index, value);
+}
return;
}
case FLOAT4: {
NullableFloat4Vector float4Vector = (NullableFloat4Vector) v;
Float value = (Float) partitionValueMap.get(f).get(column);
-float4Vector.getMutator().setSafe(index, value);
+if (value == null) {
+ float4Vector.getMutator().setNull(index);
+} else {
+ float4Vector.getMutator().setSafe(index, value);
+}
return;
}
case FLOAT8: {
NullableFloat8Vector float8Vector
[GitHub] drill pull request #805: Drill-4139: Exception while trying to prune partiti...
Github user vvysotskyi commented on a diff in the pull request: https://github.com/apache/drill/pull/805#discussion_r133933369 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetReaderUtility.java --- @@ -181,6 +186,101 @@ else if (parquetTableMetadata instanceof Metadata.ParquetTableMetadata_v2 && } /** + * Checks that the metadata file has version less than + * the version where was changed the serialization of BINARY values + * and assigns byte arrays to min/max values obtained from the deserialized string. + * + * @param parquetTableMetadata table metadata that should be corrected + */ --- End diff -- `ColumnMetadata_v1` class serializes / deserializes `primitiveType` field so it does not hard to check that field has `BINARY` type. But `ColumnMetadata_v2` and `ColumnMetadata_v3` does not serialize this field. They have classes `ColumnTypeMetadata_v2` and `ColumnTypeMetadata_v3` respectively which contain information about the field name, primitive and original types. In this method, we are receiving fields with `BINARY` type taking information about the field type from `ColumnTypeMetadata_v2` or `ColumnTypeMetadata_v3`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #805: Drill-4139: Exception while trying to prune partiti...
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/805#discussion_r126640837
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
---
@@ -444,123 +478,235 @@ public MajorType getTypeForColumn(SchemaPath
schemaPath) {
public void populatePruningVector(ValueVector v, int index, SchemaPath
column, String file) {
String f = Path.getPathWithoutSchemeAndAuthority(new
Path(file)).toString();
-MinorType type = getTypeForColumn(column).getMinorType();
+MajorType majorType = getTypeForColumn(column);
+MinorType type = majorType.getMinorType();
switch (type) {
+ case BIT: {
+NullableBitVector bitVector = (NullableBitVector) v;
+Boolean value = (Boolean) partitionValueMap.get(f).get(column);
+if (value == null) {
+ bitVector.getMutator().setNull(index);
+} else {
+ bitVector.getMutator().setSafe(index, value ? 1 : 0);
+}
+return;
+ }
case INT: {
NullableIntVector intVector = (NullableIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value);
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value);
+}
return;
}
case SMALLINT: {
NullableSmallIntVector smallIntVector = (NullableSmallIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-smallIntVector.getMutator().setSafe(index, value.shortValue());
+if (value == null) {
+ smallIntVector.getMutator().setNull(index);
+} else {
+ smallIntVector.getMutator().setSafe(index, value.shortValue());
+}
return;
}
case TINYINT: {
NullableTinyIntVector tinyIntVector = (NullableTinyIntVector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-tinyIntVector.getMutator().setSafe(index, value.byteValue());
+if (value == null) {
+ tinyIntVector.getMutator().setNull(index);
+} else {
+ tinyIntVector.getMutator().setSafe(index, value.byteValue());
+}
return;
}
case UINT1: {
NullableUInt1Vector intVector = (NullableUInt1Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value.byteValue());
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value.byteValue());
+}
return;
}
case UINT2: {
NullableUInt2Vector intVector = (NullableUInt2Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, (char) value.shortValue());
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, (char) value.shortValue());
+}
return;
}
case UINT4: {
NullableUInt4Vector intVector = (NullableUInt4Vector) v;
Integer value = (Integer) partitionValueMap.get(f).get(column);
-intVector.getMutator().setSafe(index, value);
+if (value == null) {
+ intVector.getMutator().setNull(index);
+} else {
+ intVector.getMutator().setSafe(index, value);
+}
return;
}
case BIGINT: {
NullableBigIntVector bigIntVector = (NullableBigIntVector) v;
Long value = (Long) partitionValueMap.get(f).get(column);
-bigIntVector.getMutator().setSafe(index, value);
+if (value == null) {
+ bigIntVector.getMutator().setNull(index);
+} else {
+ bigIntVector.getMutator().setSafe(index, value);
+}
return;
}
case FLOAT4: {
NullableFloat4Vector float4Vector = (NullableFloat4Vector) v;
Float value = (Float) partitionValueMap.get(f).get(column);
-float4Vector.getMutator().setSafe(index, value);
+if (value == null) {
+ float4Vector.getMutator().setNull(index);
+} else {
+ float4Vector.getMutator().setSafe(index, value);
+}
return;
}
case FLOAT8: {
NullableFloat8Vector float8Vector
