[GitHub] ilooner commented on issue #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on issue #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework. URL: https://github.com/apache/drill/pull/1344#issuecomment-414029645 @sohami Addressed review comments. Please take another look. I also implemented some tests for OperatorTestBuilder. The simple tests used the project operator to pass through input batches. While implementing those tests I discovered a bug in Project where the recordCount is not set on output containers, so I fixed that as part of this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211061250
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Thank you all for the greetings and nice words Boaz On 8/17/18 6:09 PM, salim achouche wrote: Congrats Boaz! Regards, Salim On Aug 17, 2018, at 2:33 PM, Robert Wu wrote: Congratulations, Boaz! Best regards, Rob -Original Message- From: Abhishek Girish Sent: Friday, August 17, 2018 2:17 PM To: dev Subject: Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi Congratulations, Boaz! On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia wrote: Congratulations Boaz! On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < kmanivan...@mapr.com> wrote: Congrats! Well deserved! On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: Congrats! On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai wrote: Congratulations Boaz!! Gautam On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz wrote: Congratulations Boaz. On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < shi.chun...@aliyun.com.invalid> wrote: Congrats Boaz! -- Sender:Arina Ielchiieva Sent at:2018 Aug 17 (Fri) 17:51 To:dev ; user Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC and he has accepted the invitation. Congratulations Boaz and thanks for your contributions! - Arina (on behalf of Drill PMC)
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211060910
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[jira] [Created] (DRILL-6698) Add support for handling output batches with selection vectors to OperatorTestBuilder
Timothy Farkas created DRILL-6698: - Summary: Add support for handling output batches with selection vectors to OperatorTestBuilder Key: DRILL-6698 URL: https://issues.apache.org/jira/browse/DRILL-6698 Project: Apache Drill Issue Type: Improvement Reporter: Timothy Farkas Currently only output batches without a selection vector are allowed in the OperatorTestBuilder. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congrats Boaz! Regards, Salim > On Aug 17, 2018, at 2:33 PM, Robert Wu wrote: > > Congratulations, Boaz! > > Best regards, > > Rob > > -Original Message- > From: Abhishek Girish > Sent: Friday, August 17, 2018 2:17 PM > To: dev > Subject: Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi > > Congratulations, Boaz! > > On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia > wrote: > >> Congratulations Boaz! >> >> On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < >> kmanivan...@mapr.com> wrote: >> >>> Congrats! Well deserved! >>> >>> On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: >>> Congrats! On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai >> wrote: > Congratulations Boaz!! > > Gautam > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > wrote: > >> Congratulations Boaz. >> >> On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < >> shi.chun...@aliyun.com.invalid> wrote: >> >>> Congrats Boaz! >>> >> -- >>> Sender:Arina Ielchiieva Sent at:2018 Aug >>> 17 (Fri) 17:51 To:dev ; user >>> Subject:[ANNOUNCE] New PMC member: >>> Boaz Ben-Zvi >>> >>> I am pleased to announce that Drill PMC invited Boaz Ben-Zvi >>> to >> the PMC >> and >>> he has accepted the invitation. >>> >>> Congratulations Boaz and thanks for your contributions! >>> >>> - Arina >>> (on behalf of Drill PMC) >>> >> > >>> >>
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations, Boaz! Thanks for working on Drill. --Robert On Fri, Aug 17, 2018 at 4:45 PM, Padma Penumarthy < penumarthy.pa...@gmail.com> wrote: > Congratulations Boaz. > > Thanks > Padma > > > On Fri, Aug 17, 2018 at 2:33 PM, Robert Wu wrote: > > > Congratulations, Boaz! > > > > Best regards, > > > > Rob > > > > -Original Message- > > From: Abhishek Girish > > Sent: Friday, August 17, 2018 2:17 PM > > To: dev > > Subject: Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > Congratulations, Boaz! > > > > On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia > > wrote: > > > > > Congratulations Boaz! > > > > > > On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < > > > kmanivan...@mapr.com> wrote: > > > > > > > Congrats! Well deserved! > > > > > > > > On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas > > wrote: > > > > > > > > > Congrats! > > > > > > > > > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai > > > wrote: > > > > > > > > > > > Congratulations Boaz!! > > > > > > > > > > > > Gautam > > > > > > > > > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > > > > > > > > > > > wrote: > > > > > > > > > > > > > Congratulations Boaz. > > > > > > > > > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > > > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > > > > > > > > > Congrats Boaz! > > > > > > > > > > > -- > > > > > > > > Sender:Arina Ielchiieva Sent at:2018 Aug > > > > > > > > 17 (Fri) 17:51 To:dev ; user > > > > > > > > Subject:[ANNOUNCE] New PMC member: > > > > > > > > Boaz Ben-Zvi > > > > > > > > > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi > > > > > > > > to > > > the > > > > > PMC > > > > > > > and > > > > > > > > he has accepted the invitation. > > > > > > > > > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > > > > > > > > > - Arina > > > > > > > > (on behalf of Drill PMC) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-414020875 Thanks for all the changes. I ran the regression tests and everything was green. +1 LGTM from my side. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] kr-arjun commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls
kr-arjun commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls URL: https://github.com/apache/drill/pull/1405#issuecomment-414020557 @ilooner I have pushed the latest changes. Could you please review. Thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations Boaz. Thanks Padma On Fri, Aug 17, 2018 at 2:33 PM, Robert Wu wrote: > Congratulations, Boaz! > > Best regards, > > Rob > > -Original Message- > From: Abhishek Girish > Sent: Friday, August 17, 2018 2:17 PM > To: dev > Subject: Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi > > Congratulations, Boaz! > > On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia > wrote: > > > Congratulations Boaz! > > > > On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < > > kmanivan...@mapr.com> wrote: > > > > > Congrats! Well deserved! > > > > > > On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas > wrote: > > > > > > > Congrats! > > > > > > > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai > > wrote: > > > > > > > > > Congratulations Boaz!! > > > > > > > > > > Gautam > > > > > > > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > > > > > > > > > wrote: > > > > > > > > > > > Congratulations Boaz. > > > > > > > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > > > > > > > Congrats Boaz! > > > > > > > > > -- > > > > > > > Sender:Arina Ielchiieva Sent at:2018 Aug > > > > > > > 17 (Fri) 17:51 To:dev ; user > > > > > > > Subject:[ANNOUNCE] New PMC member: > > > > > > > Boaz Ben-Zvi > > > > > > > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi > > > > > > > to > > the > > > > PMC > > > > > > and > > > > > > > he has accepted the invitation. > > > > > > > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > > > > > > > - Arina > > > > > > > (on behalf of Drill PMC) > > > > > > > > > > > > > > > > > > > > > > > > > > > >
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211049297
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211048926
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211048316
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] kr-arjun commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls
kr-arjun commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls URL: https://github.com/apache/drill/pull/1405#issuecomment-414005659 @ilooner I found a possible issue with current logic. It may fail if the path has a wildcard ( case where not ending with .drill. In this case list status may fail). I am pushing the changes for handling this scenario. Thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ilooner commented on issue #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on issue #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework. URL: https://github.com/apache/drill/pull/1344#issuecomment-414004294 @sohami Applied review comments. Please take another look. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211046248
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/svremover/AbstractGenericCopierTest.java
##
@@ -20,77 +20,106 @@
import org.apache.drill.common.types.TypeProtos;
import org.apache.drill.common.types.Types;
import org.apache.drill.exec.exception.SchemaChangeException;
-import org.apache.drill.exec.memory.RootAllocator;
+import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
import org.apache.drill.exec.record.BatchSchema;
import org.apache.drill.exec.record.MaterializedField;
import org.apache.drill.exec.record.RecordBatch;
import org.apache.drill.exec.record.VectorContainer;
-import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.record.VectorWrapper;
import org.apache.drill.exec.vector.SchemaChangeCallBack;
+import org.apache.drill.test.BaseDirTestWatcher;
+import org.apache.drill.test.OperatorFixture;
+import org.apache.drill.test.rowSet.DirectRowSet;
import org.apache.drill.test.rowSet.RowSet;
-import org.apache.drill.test.rowSet.RowSetBatch;
import org.apache.drill.test.rowSet.RowSetBuilder;
import org.apache.drill.test.rowSet.RowSetComparison;
import org.apache.drill.test.rowSet.schema.SchemaBuilder;
+import org.junit.Rule;
import org.junit.Test;
public abstract class AbstractGenericCopierTest {
+ @Rule
+ public final BaseDirTestWatcher baseDirTestWatcher = new
BaseDirTestWatcher();
+
@Test
- public void testCopyRecords() throws SchemaChangeException {
-try (RootAllocator allocator = new RootAllocator(10_000_000)) {
- final TupleMetadata batchSchema =
createTestSchema(BatchSchema.SelectionVectorMode.NONE);
+ public void testCopyRecords() throws Exception {
+try (OperatorFixture operatorFixture = new
OperatorFixture.Builder(baseDirTestWatcher).build()) {
+ final BufferAllocator allocator = operatorFixture.allocator();
+ final BatchSchema batchSchema =
createTestSchema(BatchSchema.SelectionVectorMode.NONE);
final RowSet srcRowSet = createSrcRowSet(allocator);
- final RowSet destRowSet = new RowSetBuilder(allocator,
batchSchema).build();
- final VectorContainer destContainer = destRowSet.container();
- final Copier copier = createCopier(new RowSetBatch(srcRowSet),
destContainer, null);
+ final VectorContainer destContainer = new VectorContainer(allocator,
batchSchema);
+
+ for (VectorWrapper vectorWrapper: destContainer) {
+vectorWrapper.getValueVector().allocateNew();
Review comment:
Thanks for catching this. I've moved the allocation code into a static
method and call it in GenericCopier.copyRecords.
It is still necessary to allocate the vectors in the appendRecords test
though, since appendRecords assumes the vector container is pre existing.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
Re: Drillbit client connect authorization
Hi Joel/Alex, Thanks for explaining the use case with multi tenant cluster. @Joel Unfortunately I have not used the setup described above but from explanation looks like the impersonation tickets will be used by Drillbit's on Tenant A to restrict the MapR platform access by a limited set of Drillbit authenticated user. Using this any user in Tenant B will not be able to execute query on Tenant A even though it can be authenticated successfully by the Drillbit in Tenant A. This way authorization check is done at data layer. @Alex, Adding an authorization check for a valid authenticated cluster user shouldn't be a big change. Based on a configured set's of users/group a subset of cluster user can be allowed to connect. But can you please point to how other services do these authorization checks when running in multi tenant environment ? Based on my understanding all these authorization check in Hadoop system are done at data layer or they have a separate security service which does these checks along with other security checks for authentication, etc. Also please feel free to open a JIRA ticket with details. Thanks, Sorabh On Fri, Aug 17, 2018 at 11:21 AM, Oleksandr Kalinin wrote: > Hi Sorabh, > > Thanks for you comments. Joel described in detail our current thinking on > how to overcome the issue. We are not yet 100% sure if it will actually > work though. > > Actually I raised this topic in this mailing list because I think it's not > only specific to our setup. It's more about having nice "Drill on YARN" > feature with very limited (frankly, no) access control which almost makes > the feature unusable in environments where it is attractive - multi tenant > secure clusters. Supported security mechanisms are good for authentication, > but using them for authorization seems suboptimal. Typically, YARN clusters > run in single Kerberos realm and the need to introduce multiple realms and > separate identities for Drill service is not at all convenient (I am pretty > sure that in many environments like ours it is a no go). And how about use > cases with no Kerberos setup? If we can workaround access control by > MapR-specific security tickets like described by Joel - good for us, but > what about other environments? > > So the question is more whether it make sense to consider introducing user > authorization feature. This thread refers only to session authorization to > complement YARN feature, but it could be extendable of course, e.g. in > similar ways like Drill already supports multiple authentication > mechanisms. > > Thanks & Best Regards, > Alex > > On Wed, Aug 15, 2018 at 11:30 PM, Sorabh Hamirwasia > wrote: > > > Hi Oleksandr, > > Drill doesn't do any user management in itself, instead relies on the > > corresponding security mechanisms in use to do it. It uses SASL framework > > to allow using different pluggable security mechanisms. So it should be > > upon the security mechanism in use to do the authorization level checks. > > For example in your use case if you want to allow only certain set's of > > users to connect to a cluster then you can choose to use Kerberos with > each > > cluster running in different realms. This will ensure client users > running > > in corresponding realm can only connect to cluster running in that realm. > > > > For the impersonation issue I think it's a configuration issue and the > > behavior is expected where all queries whether from user A or B are > > executed as admin users. > > > > Thanks, > > Sorabh > > > > On Mon, Aug 13, 2018 at 9:02 AM, Oleksandr Kalinin > > wrote: > > > > > Hello Drill community, > > > > > > In multi-tenant YARN clusters, running multiple Drill-on-YARN clusters > > > seems as attractive feature as it enables leveraging on YARN mechanisms > > of > > > resource management and isolation. However, there seems to be simple > > access > > > restriction issue. Assume : > > > > > > - Cluster A launched by user X > > > - Cluster B launched by user Y > > > > > > Both users X and Y will be able to connect and run queries against > > clusters > > > A and B (in fact, that applies to any positively authenticated user, > not > > > only X and Y). Whereas we obviously would like to ensure exclusive > usage > > of > > > clusters by their owners - who are owners of respective YARN resources. > > In > > > case users X and Y are non-privileged DFS users and impersonation is > not > > > enabled, then user A has access to data on behalf of user B and vice > > versa > > > which is additional potential security issue. > > > > > > I was looking for possibilities to control connect authorization, but > > > couldn't find anything related yet. Do I miss something maybe? Are > there > > > any other considerations, perhaps this point was already discussed > > before? > > > > > > It could be possible to tweak PAM setup to trigger authentication > failure > > > for "undesired" users but that looks like an overkill in terms of > > > complexity. > > > > > > From
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211041686
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211040661
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211040315
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211039960
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211039752
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211038430
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] ilooner commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
ilooner commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211038182
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/MockRecordBatch.java
##
@@ -158,19 +210,34 @@ public IterOutcome next() {
container.clear();
container = new VectorContainer(allocator, inputSchema);
}
- container.transferIn(input);
- container.setRecordCount(recordCount);
-
- // Transfer the sv2 as well
- final SelectionVector2 inputSv2 =
-(allTestContainersSv2 != null && allTestContainersSv2.size() > 0)
- ? allTestContainersSv2.get(currentContainerIndex) : null;
- if (inputSv2 != null) {
-sv2.allocateNewSafe(inputSv2.getCount());
-for (int i=0; i
RE: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations, Boaz! Best regards, Rob -Original Message- From: Abhishek Girish Sent: Friday, August 17, 2018 2:17 PM To: dev Subject: Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi Congratulations, Boaz! On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia wrote: > Congratulations Boaz! > > On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < > kmanivan...@mapr.com> wrote: > > > Congrats! Well deserved! > > > > On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: > > > > > Congrats! > > > > > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai > wrote: > > > > > > > Congratulations Boaz!! > > > > > > > > Gautam > > > > > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > > > > > > > wrote: > > > > > > > > > Congratulations Boaz. > > > > > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > > > > > Congrats Boaz! > > > > > > > -- > > > > > > Sender:Arina Ielchiieva Sent at:2018 Aug > > > > > > 17 (Fri) 17:51 To:dev ; user > > > > > > Subject:[ANNOUNCE] New PMC member: > > > > > > Boaz Ben-Zvi > > > > > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi > > > > > > to > the > > > PMC > > > > > and > > > > > > he has accepted the invitation. > > > > > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > > > > > - Arina > > > > > > (on behalf of Drill PMC) > > > > > > > > > > > > > > > > > > > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations, Boaz! On Fri, Aug 17, 2018 at 2:15 PM Sorabh Hamirwasia wrote: > Congratulations Boaz! > > On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < > kmanivan...@mapr.com> wrote: > > > Congrats! Well deserved! > > > > On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: > > > > > Congrats! > > > > > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai > wrote: > > > > > > > Congratulations Boaz!! > > > > > > > > Gautam > > > > > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > > > wrote: > > > > > > > > > Congratulations Boaz. > > > > > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > > > > > Congrats Boaz! > > > > > > > -- > > > > > > Sender:Arina Ielchiieva > > > > > > Sent at:2018 Aug 17 (Fri) 17:51 > > > > > > To:dev ; user > > > > > > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to > the > > > PMC > > > > > and > > > > > > he has accepted the invitation. > > > > > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > > > > > - Arina > > > > > > (on behalf of Drill PMC) > > > > > > > > > > > > > > > > > > > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations Boaz! On Fri, Aug 17, 2018 at 11:42 AM, Karthikeyan Manivannan < kmanivan...@mapr.com> wrote: > Congrats! Well deserved! > > On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: > > > Congrats! > > > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai wrote: > > > > > Congratulations Boaz!! > > > > > > Gautam > > > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > > wrote: > > > > > > > Congratulations Boaz. > > > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > > > Congrats Boaz! > > > > > -- > > > > > Sender:Arina Ielchiieva > > > > > Sent at:2018 Aug 17 (Fri) 17:51 > > > > > To:dev ; user > > > > > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the > > PMC > > > > and > > > > > he has accepted the invitation. > > > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > > > - Arina > > > > > (on behalf of Drill PMC) > > > > > > > > > > > > > > >
[jira] [Resolved] (DRILL-6694) NPE in UnnestRecordBatch when query uses a column name not present in data
[
https://issues.apache.org/jira/browse/DRILL-6694?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sorabh Hamirwasia resolved DRILL-6694.
--
Resolution: Fixed
> NPE in UnnestRecordBatch when query uses a column name not present in data
> --
>
> Key: DRILL-6694
> URL: https://issues.apache.org/jira/browse/DRILL-6694
> Project: Apache Drill
> Issue Type: Bug
> Components: Execution - Relational Operators
>Affects Versions: 1.14.0
>Reporter: Sorabh Hamirwasia
>Assignee: Sorabh Hamirwasia
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.15.0
>
>
> When the array column name doesn't exist in the underlying data and is used
> in query with Unnest then there is NPE. The reason is Unnest tries to get the
> ValueVector of unnest column from incoming based on TypedFieldId which will
> be null in this case and hence the exception.
> {code:java}
> [Error Id: 6f8461ee-92c7-4865-b5e6-3e2f756391c4 on pssc-67.qa.lab:31010] at
> org.apache.drill.common.exceptions.UserException$Builder.build(UserException.java:633)
> ~[drill-common-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.work.fragment.FragmentExecutor.sendFinalState(FragmentExecutor.java:361)
> [drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.work.fragment.FragmentExecutor.cleanup(FragmentExecutor.java:216)
> [drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.work.fragment.FragmentExecutor.run(FragmentExecutor.java:327)
> [drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.common.SelfCleaningRunnable.run(SelfCleaningRunnable.java:38)
> [drill-common-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> [na:1.8.0_151] at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> [na:1.8.0_151] at java.lang.Thread.run(Thread.java:748) [na:1.8.0_151]
> Caused by: java.lang.NullPointerException: null at
> org.apache.drill.exec.physical.impl.unnest.UnnestRecordBatch.schemaChanged(UnnestRecordBatch.java:422)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.unnest.UnnestRecordBatch.innerNext(UnnestRecordBatch.java:208)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:172)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:119)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:109)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractUnaryRecordBatch.innerNext(AbstractUnaryRecordBatch.java:64)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.project.ProjectRecordBatch.innerNext(ProjectRecordBatch.java:142)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:172)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:119)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.join.LateralJoinBatch.prefetchFirstBatchFromBothSides(LateralJoinBatch.java:331)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.join.LateralJoinBatch.buildSchema(LateralJoinBatch.java:356)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:152)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:119)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:109)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.aggregate.StreamingAggBatch.buildSchema(StreamingAggBatch.java:158)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:152)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
> org.apache.drill.exec.physical.impl.BaseRootExec.next(BaseRootExec.java:103)
> ~[drill-java-exec-1.15.0-SNAPSHOT.jar:1.15.0-SNAPSHOT] at
>
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211032924
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211022063
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211032665
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r210998188
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r210994889
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/MockRecordBatch.java
##
@@ -158,19 +210,34 @@ public IterOutcome next() {
container.clear();
container = new VectorContainer(allocator, inputSchema);
}
- container.transferIn(input);
- container.setRecordCount(recordCount);
-
- // Transfer the sv2 as well
- final SelectionVector2 inputSv2 =
-(allTestContainersSv2 != null && allTestContainersSv2.size() > 0)
- ? allTestContainersSv2.get(currentContainerIndex) : null;
- if (inputSv2 != null) {
-sv2.allocateNewSafe(inputSv2.getCount());
-for (int i=0; i
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211000586
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r211020462
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorTestBuilder.java
##
@@ -0,0 +1,300 @@
+/*
+ * 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.test;
+
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.drill.exec.physical.base.AbstractBase;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
+import org.apache.drill.exec.physical.impl.svremover.Copier;
+import org.apache.drill.exec.physical.impl.svremover.GenericCopier;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.record.RecordBatchSizer;
+import org.apache.drill.exec.record.VectorContainer;
+import org.apache.drill.exec.vector.FixedWidthVector;
+import org.apache.drill.exec.vector.ValueVector;
+import org.apache.drill.exec.vector.VariableWidthVector;
+import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.test.rowSet.IndirectRowSet;
+import org.apache.drill.test.rowSet.RowSet;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+public class OperatorTestBuilder {
+
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorTestBuilder.class);
+
+ private final List expectedResults = new ArrayList<>();
+ private final List upstreamBatches = new ArrayList<>();
+ private PhysicalOpUnitTestBase physicalOpUnitTestBase;
+
+ private PhysicalOperator physicalOperator;
+ private long initReservation = AbstractBase.INIT_ALLOCATION;
+ private long maxAllocation = AbstractBase.MAX_ALLOCATION;
+ private Optional expectedNumBatchesOpt = Optional.empty();
+ private Optional expectedTotalRowsOpt = Optional.empty();
+ private boolean combineOutputBatches;
+
+ public OperatorTestBuilder(PhysicalOpUnitTestBase physicalOpUnitTestBase) {
+this.physicalOpUnitTestBase = physicalOpUnitTestBase;
+ }
+
+ @SuppressWarnings({"unchecked", "resource"})
+ public void go() throws Exception {
+final List actualResults = new ArrayList<>();
+CloseableRecordBatch testOperator = null;
+
+try {
+ validate();
+ int expectedNumBatches =
expectedNumBatchesOpt.orElse(expectedResults.size());
+ physicalOpUnitTestBase.mockOpContext(physicalOperator, initReservation,
maxAllocation);
+
+ final BatchCreator opCreator =
(BatchCreator)
physicalOpUnitTestBase.opCreatorReg.getOperatorCreator(physicalOperator.getClass());
+ testOperator = opCreator.getBatch(physicalOpUnitTestBase.fragContext,
physicalOperator, (List)upstreamBatches);
+
+ batchIterator: for (int batchIndex = 0;; batchIndex++) {
+final RecordBatch.IterOutcome outcome = testOperator.next();
+
+switch (outcome) {
+ case NONE:
+if (!combineOutputBatches) {
+ Assert.assertEquals(expectedNumBatches, batchIndex);
+}
+// We are done iterating over batches. Now we need to compare them.
+break batchIterator;
+ case OK_NEW_SCHEMA:
+boolean skip = true;
+
+try {
+ skip = testOperator.getContainer().getRecordCount() == 0;
+} catch (IllegalStateException e) {
+ // We should skip this batch in this case. It means no data was
included with the okay schema
+} finally {
+ if (skip) {
+batchIndex--;
+break;
+ }
+}
+ case OK:
+if (!combineOutputBatches && batchIndex >= expectedNumBatches) {
+ testOperator.getContainer().clear();
+ Assert.fail("More batches received than
[GitHub] sohami commented on a change in pull request #1344: DRILL-6461: Added basic data correctness tests for hash agg, and improved operator unit testing framework.
sohami commented on a change in pull request #1344: DRILL-6461: Added basic
data correctness tests for hash agg, and improved operator unit testing
framework.
URL: https://github.com/apache/drill/pull/1344#discussion_r210989987
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/svremover/AbstractGenericCopierTest.java
##
@@ -20,77 +20,106 @@
import org.apache.drill.common.types.TypeProtos;
import org.apache.drill.common.types.Types;
import org.apache.drill.exec.exception.SchemaChangeException;
-import org.apache.drill.exec.memory.RootAllocator;
+import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.physical.impl.MockRecordBatch;
import org.apache.drill.exec.record.BatchSchema;
import org.apache.drill.exec.record.MaterializedField;
import org.apache.drill.exec.record.RecordBatch;
import org.apache.drill.exec.record.VectorContainer;
-import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.record.VectorWrapper;
import org.apache.drill.exec.vector.SchemaChangeCallBack;
+import org.apache.drill.test.BaseDirTestWatcher;
+import org.apache.drill.test.OperatorFixture;
+import org.apache.drill.test.rowSet.DirectRowSet;
import org.apache.drill.test.rowSet.RowSet;
-import org.apache.drill.test.rowSet.RowSetBatch;
import org.apache.drill.test.rowSet.RowSetBuilder;
import org.apache.drill.test.rowSet.RowSetComparison;
import org.apache.drill.test.rowSet.schema.SchemaBuilder;
+import org.junit.Rule;
import org.junit.Test;
public abstract class AbstractGenericCopierTest {
+ @Rule
+ public final BaseDirTestWatcher baseDirTestWatcher = new
BaseDirTestWatcher();
+
@Test
- public void testCopyRecords() throws SchemaChangeException {
-try (RootAllocator allocator = new RootAllocator(10_000_000)) {
- final TupleMetadata batchSchema =
createTestSchema(BatchSchema.SelectionVectorMode.NONE);
+ public void testCopyRecords() throws Exception {
+try (OperatorFixture operatorFixture = new
OperatorFixture.Builder(baseDirTestWatcher).build()) {
+ final BufferAllocator allocator = operatorFixture.allocator();
+ final BatchSchema batchSchema =
createTestSchema(BatchSchema.SelectionVectorMode.NONE);
final RowSet srcRowSet = createSrcRowSet(allocator);
- final RowSet destRowSet = new RowSetBuilder(allocator,
batchSchema).build();
- final VectorContainer destContainer = destRowSet.container();
- final Copier copier = createCopier(new RowSetBatch(srcRowSet),
destContainer, null);
+ final VectorContainer destContainer = new VectorContainer(allocator,
batchSchema);
+
+ for (VectorWrapper vectorWrapper: destContainer) {
+vectorWrapper.getValueVector().allocateNew();
Review comment:
Why we need to allocate memory for destination container here ? Shouldn't
the copier allocate it when `copyRecords` is called. Since it will do so based
on incoming record count. Also in case of `StraightCopier` it should not
allocate any memory for ValueVectors in destination container since there will
be direct transfer of buffer from incoming to outgoing container ValueVectors.
I think the bug is
[here](https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/GenericCopier.java#L56)
instead. It should be same as
[AbstractCopier::copyRecords](https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/AbstractCopier.java#L45).
Also `GenericCopier` is not efficient for cases like `StraightCopier` where no
actual copy is done only transfer of buffer happens.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sachouche commented on issue #1433: DRILL-6685: Fixed exception when reading Parquet data
sachouche commented on issue #1433: DRILL-6685: Fixed exception when reading Parquet data URL: https://github.com/apache/drill/pull/1433#issuecomment-413987125 @arina-ielchiieva, - Agreed, but I liked better the pattern used in the sample test (that you have sent) as it provides more flexibility - Each test can invoke separate flags without increasing complexity - Combining global settings along with overrides can become tricky (an override needs to ensure that all global settings are restituted to their original state after the test finishes) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature
sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#discussion_r211027553 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/AbstractSV2Copier.java ## @@ -32,6 +32,7 @@ private SelectionVector2 sv2; protected List transferPairs = new ArrayList<>(); + Review comment: please revert back this change. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature
sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#discussion_r211027636 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java ## @@ -27,6 +27,7 @@ import org.apache.drill.exec.record.VectorWrapper; import org.apache.drill.exec.record.WritableBatch; + Review comment: please revert back this extra line here and below. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature
sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#discussion_r211028086 ## File path: pom.xml ## @@ -389,6 +389,7 @@ com.mycila license-maven-plugin + Review comment: revert this change. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature
sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD
feature
URL: https://github.com/apache/drill/pull/1334#discussion_r211027800
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java
##
@@ -179,6 +179,7 @@ public void close() {
clear();
}
+
Review comment:
remove this extra line
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature
sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD
feature
URL: https://github.com/apache/drill/pull/1334#discussion_r211027163
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
##
@@ -156,9 +156,19 @@ public DoubleValidator(String name) {
}
}
- public static class IntegerValidator extends TypeValidator {
+ public static class IntegerValidator extends LongValidator {
public IntegerValidator(String name) {
- super(name, Kind.INTEGER);
+ super(name);
+}
+
+@Override
+public void validate(final OptionValue v, final OptionMetaData metaData,
final OptionSet manager) {
+ super.validate(v, metaData, manager);
+ if (v.num_val > Integer.MAX_VALUE || v.num_val < Integer.MIN_VALUE) {
+throw UserException.validationError()
+ .message(String.format("Option %s does not have a validate integer
value", getOptionName()))
Review comment:
does not have a **valid** integer value
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] priteshm commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls
priteshm commented on issue #1405: DRILL-6640: Modifying DotDrillUtil implementation to avoid using globStatus calls URL: https://github.com/apache/drill/pull/1405#issuecomment-413982671 @ilooner can you please review this as well? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congrats! Well deserved! On Fri, Aug 17, 2018, 11:31 AM Timothy Farkas wrote: > Congrats! > > On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai wrote: > > > Congratulations Boaz!! > > > > Gautam > > > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz > wrote: > > > > > Congratulations Boaz. > > > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > > shi.chun...@aliyun.com.invalid> wrote: > > > > > > > Congrats Boaz! > > > > -- > > > > Sender:Arina Ielchiieva > > > > Sent at:2018 Aug 17 (Fri) 17:51 > > > > To:dev ; user > > > > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the > PMC > > > and > > > > he has accepted the invitation. > > > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > > > - Arina > > > > (on behalf of Drill PMC) > > > > > > > > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congrats! On Fri, Aug 17, 2018 at 11:27 AM, Gautam Parai wrote: > Congratulations Boaz!! > > Gautam > > On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz wrote: > > > Congratulations Boaz. > > > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > > shi.chun...@aliyun.com.invalid> wrote: > > > > > Congrats Boaz! > > > -- > > > Sender:Arina Ielchiieva > > > Sent at:2018 Aug 17 (Fri) 17:51 > > > To:dev ; user > > > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC > > and > > > he has accepted the invitation. > > > > > > Congratulations Boaz and thanks for your contributions! > > > > > > - Arina > > > (on behalf of Drill PMC) > > > > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations Boaz!! Gautam On Fri, Aug 17, 2018 at 11:04 AM, Khurram Faraaz wrote: > Congratulations Boaz. > > On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < > shi.chun...@aliyun.com.invalid> wrote: > > > Congrats Boaz! > > -- > > Sender:Arina Ielchiieva > > Sent at:2018 Aug 17 (Fri) 17:51 > > To:dev ; user > > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC > and > > he has accepted the invitation. > > > > Congratulations Boaz and thanks for your contributions! > > > > - Arina > > (on behalf of Drill PMC) > > >
Re: Drillbit client connect authorization
Hi Sorabh, Thanks for you comments. Joel described in detail our current thinking on how to overcome the issue. We are not yet 100% sure if it will actually work though. Actually I raised this topic in this mailing list because I think it's not only specific to our setup. It's more about having nice "Drill on YARN" feature with very limited (frankly, no) access control which almost makes the feature unusable in environments where it is attractive - multi tenant secure clusters. Supported security mechanisms are good for authentication, but using them for authorization seems suboptimal. Typically, YARN clusters run in single Kerberos realm and the need to introduce multiple realms and separate identities for Drill service is not at all convenient (I am pretty sure that in many environments like ours it is a no go). And how about use cases with no Kerberos setup? If we can workaround access control by MapR-specific security tickets like described by Joel - good for us, but what about other environments? So the question is more whether it make sense to consider introducing user authorization feature. This thread refers only to session authorization to complement YARN feature, but it could be extendable of course, e.g. in similar ways like Drill already supports multiple authentication mechanisms. Thanks & Best Regards, Alex On Wed, Aug 15, 2018 at 11:30 PM, Sorabh Hamirwasia wrote: > Hi Oleksandr, > Drill doesn't do any user management in itself, instead relies on the > corresponding security mechanisms in use to do it. It uses SASL framework > to allow using different pluggable security mechanisms. So it should be > upon the security mechanism in use to do the authorization level checks. > For example in your use case if you want to allow only certain set's of > users to connect to a cluster then you can choose to use Kerberos with each > cluster running in different realms. This will ensure client users running > in corresponding realm can only connect to cluster running in that realm. > > For the impersonation issue I think it's a configuration issue and the > behavior is expected where all queries whether from user A or B are > executed as admin users. > > Thanks, > Sorabh > > On Mon, Aug 13, 2018 at 9:02 AM, Oleksandr Kalinin > wrote: > > > Hello Drill community, > > > > In multi-tenant YARN clusters, running multiple Drill-on-YARN clusters > > seems as attractive feature as it enables leveraging on YARN mechanisms > of > > resource management and isolation. However, there seems to be simple > access > > restriction issue. Assume : > > > > - Cluster A launched by user X > > - Cluster B launched by user Y > > > > Both users X and Y will be able to connect and run queries against > clusters > > A and B (in fact, that applies to any positively authenticated user, not > > only X and Y). Whereas we obviously would like to ensure exclusive usage > of > > clusters by their owners - who are owners of respective YARN resources. > In > > case users X and Y are non-privileged DFS users and impersonation is not > > enabled, then user A has access to data on behalf of user B and vice > versa > > which is additional potential security issue. > > > > I was looking for possibilities to control connect authorization, but > > couldn't find anything related yet. Do I miss something maybe? Are there > > any other considerations, perhaps this point was already discussed > before? > > > > It could be possible to tweak PAM setup to trigger authentication failure > > for "undesired" users but that looks like an overkill in terms of > > complexity. > > > > From user perspective, basic ACL configuration with users and groups > > authorized to connect to Drillbit would already be sufficient IMO. Or > > configuration switch to ensure that only owner user is authorized to > > connect. > > > > Best Regards, > > Alex > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations Boaz. On Fri, Aug 17, 2018 at 10:47 AM, shi.chunhui < shi.chun...@aliyun.com.invalid> wrote: > Congrats Boaz! > -- > Sender:Arina Ielchiieva > Sent at:2018 Aug 17 (Fri) 17:51 > To:dev ; user > Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC and > he has accepted the invitation. > > Congratulations Boaz and thanks for your contributions! > > - Arina > (on behalf of Drill PMC) >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congrats Boaz! -- Sender:Arina Ielchiieva Sent at:2018 Aug 17 (Fri) 17:51 To:dev ; user Subject:[ANNOUNCE] New PMC member: Boaz Ben-Zvi I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC and he has accepted the invitation. Congratulations Boaz and thanks for your contributions! - Arina (on behalf of Drill PMC)
[GitHub] vvysotskyi commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#issuecomment-413938089 Besides the commit were addressed CR comments, I have added two commits: the first one contains changes in commits before the rebase (some of the commits added methods which I tried to replace, so replaced them again). The second commit contains changes to ban some guava methods. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vrozov commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vrozov commented on a change in pull request #1397: DRILL-6633: Replace usage
of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210980777
##
File path: common/src/test/java/org/apache/drill/test/SubDirTestWatcher.java
##
@@ -83,10 +84,10 @@
private List subDirs;
protected SubDirTestWatcher(File baseDir, boolean createAtBeginning, boolean
deleteAtEnd, List subDirs) {
-this.baseDir = Preconditions.checkNotNull(baseDir);
+this.baseDir = Objects.requireNonNull(baseDir);
this.createAtBeginning = createAtBeginning;
this.deleteAtEnd = deleteAtEnd;
-this.subDirs = Preconditions.checkNotNull(subDirs);
+this.subDirs = Objects.requireNonNull(subDirs);
Preconditions.checkArgument(!subDirs.isEmpty(), "The list of subDirs is
empty.");
Review comment:
While in this place `Preconditions.checkNotNull()` and
`Objects.requireNonNull()` provide the same functionality, in general guava
`Preconditions` supports template error message that `Objects.requireNonNull`
does not support. I'd recommend keeping `Preconditions.checkNotNull` for
consistency. IMO, it is necessary to remove *deprecated* guava classes, not
*all* references to guava where similar functionality is provided by JDK.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vrozov commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vrozov commented on a change in pull request #1397: DRILL-6633: Replace usage
of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210978384
##
File path: common/src/main/java/org/apache/drill/common/KerberosUtil.java
##
@@ -57,9 +57,9 @@ public static String getPrincipalFromParts(final String
primary, final String in
public static String[] splitPrincipalIntoParts(final String principal) {
final String[] components = principal.split("[/@]");
checkState(components.length == 3);
Review comment:
@vdiravka What is wrong with using guava `Preconditions`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations, Boaz! On Fri, Aug 17, 2018 at 10:22 AM Kunal Khatua wrote: > Congratulations, Boaz!! > On 8/17/2018 10:11:32 AM, Paul Rogers wrote: > Congratulations Boaz! > - Paul > > > > On Friday, August 17, 2018, 2:56:27 AM PDT, Vitalii Diravka wrote: > > Congrats Boaz! > > Kind regards > Vitalii > > > On Fri, Aug 17, 2018 at 12:51 PM Arina Ielchiieva wrote: > > > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC > and > > he has accepted the invitation. > > > > Congratulations Boaz and thanks for your contributions! > > > > - Arina > > (on behalf of Drill PMC) > > >
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations, Boaz!! On 8/17/2018 10:11:32 AM, Paul Rogers wrote: Congratulations Boaz! - Paul On Friday, August 17, 2018, 2:56:27 AM PDT, Vitalii Diravka wrote: Congrats Boaz! Kind regards Vitalii On Fri, Aug 17, 2018 at 12:51 PM Arina Ielchiieva wrote: > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC and > he has accepted the invitation. > > Congratulations Boaz and thanks for your contributions! > > - Arina > (on behalf of Drill PMC) >
Re: [DISCUSSION] current project state
I think apart from a lot of the improvements proposed, like metadata store (+1), having support for a materialized view would be especially useful. The reason I am proposing this is that there are some storage plugins, for which Drill cannot pushdown filters, etc. Materialized views would allow a user to query, say, an S3 source that can be awfully slow, but allow for a more interactive querying on the materialized view. The building blocks for this, creating temporary tables, already exist. So it is more of a matter of how to build out a UX that allows easier management of this interim dataset (like TTL, refresh policy, etc.) While it's great that Drill can query so many sources, it can be a challenge to do interactive SQL with some of these sources due to their non-interactive nature. ~ Kunal On 8/17/2018 5:24:27 AM, Joel Pfaff wrote: Hello, Thanks for reaching the community to open this discussion. I have put a list of suggestions below, but before jumping to them. I think there is something more important at stake. To second Paul and Weijie, I think there is a need to define how we see Drill in the future. Where do we see its strength, where do we see weaknesses, and what to change/improve to put Drill as a leader for a certain scope. Suggestions (there is no specific priority): * Metadata management improvement: - It has been discussed a lot in this thread and a few other ones. So I am only going to focus on a few topics: We currently do rely on Spark based ETL to generate the parquet files for Drill. To be able to migrate to Drill's CTAS, we would need a better way to manage the schemas and types (including the definition of complex types). - I have seen multiple cases where we would need a better way to manage atomically a set of files as tables. As an example: our first implementation in the application was based on recursively copying a significant dataset, to have a safe place to remove/add files without impact on ongoing queries. After this change is done, the newly created directory is promoted as the new base directory for searches. The old directory is kept as a way to implement searches on snapshots. Later on, an improvement has been done to only rely on symlinks, but the implementation became really complex, for a problem that could be solved more efficiently on the querying engine layer. In that perspective, the Iceberg project is a very interesting solution. * SQL expressiveness: - We would also appreciate a SQL way to create workspaces (like a CREATE DATABASE). And progressively the integration of additional relational/analytical capabilities (INTERSECT/EXCEPT, and ROLLUP/CUBE). - I would like to have means to play a bit with the execution plans without touching the session options at each query. So a support of query hints could bring some interesting benefits. * Resource Management: - Drill made nice improvements related to the management of local resources (inside a DrillBit) with regards to CPU/Memory, but is still lacking on the resource management at the cluster level. Due to this limitation, we plan to manage one Drill cluster per functional group. So a mechanism of admission control with priority would help us to have to reduce the number of Drill clusters (and have bigger ones). * Resiliency: - When the clusters get large, the probability of losing nodes increases. It would be helpful to support resuming/restarting queries that were executing on a node that failed. * Hosting: - While we do store everything currently on the Hadoop clusters, we expect to move some of the historical storage to object stores. And we may have cases where the functional group handling the data does not need a Hadoop cluster, but still would like to be able to search through his dataset stored on the Object Store. As a longer term goal, being able to run Drill on Kubernetes would allow us to give the Drill power to these users on a shared infrastucture. * Drill Extensions: - Several querying enging support the inlining of UDF with the query. In Spark, the Querying DSL allows for Lambdas to be serialized with the query (in Java/Scala/Python/R). In BigQuery, the SQL supports primitives to inline JavaScript UDF (I found it crazy at first, but after a second thought, I found JS to be an excellent idea as a lightweight sandbox for untrusted code). For these kinds of extensions, a migration to Arrow as an internal value vector representation, would provide the basic libraries to exchange data between the interpreters. Regards, Joel On Wed, Aug 15, 2018 at 12:44 AM weijie tong wrote: > My thinking about this topic. Drill does well now. But be better,we need to > be idealist to bring in more use cases or more advanced query performance > compared to other projects like Flink , Spark, Presto,Impala. To > performance, I wonder do we need to adopt the project Gandiva which is so > exciting or we does our own similar implementation without migrating to > Arrow. If we choose to adopt
Re: [DISCUSSION] current project state
Hi Joel, Excellent suggestions! Thanks much for sharing your real-world experience; this is invaluable information for the project. Any others out there who can share their experience and what would help Drill be even before for their use case? Arina, how do we want to capture and prioritize the ideas we've gotten from Joel and Weijie? Jira tickets? Some document with high-level themes that links to specific JIRA tickets? The core development team has done great work, but it can't do everything. How might we identify those within the scope of the core team, and those that could use contributions from the community? Thanks, - Paul On Friday, August 17, 2018, 5:24:28 AM PDT, Joel Pfaff wrote: Hello, Thanks for reaching the community to open this discussion. I have put a list of suggestions below, but before jumping to them. I think there is something more important at stake. To second Paul and Weijie, I think there is a need to define how we see Drill in the future. Where do we see its strength, where do we see weaknesses, and what to change/improve to put Drill as a leader for a certain scope. Suggestions (there is no specific priority): * Metadata management improvement: - It has been discussed a lot in this thread and a few other ones. So I am only going to focus on a few topics: We currently do rely on Spark based ETL to generate the parquet files for Drill. To be able to migrate to Drill's CTAS, we would need a better way to manage the schemas and types (including the definition of complex types). - I have seen multiple cases where we would need a better way to manage atomically a set of files as tables. As an example: our first implementation in the application was based on recursively copying a significant dataset, to have a safe place to remove/add files without impact on ongoing queries. After this change is done, the newly created directory is promoted as the new base directory for searches. The old directory is kept as a way to implement searches on snapshots. Later on, an improvement has been done to only rely on symlinks, but the implementation became really complex, for a problem that could be solved more efficiently on the querying engine layer. In that perspective, the Iceberg project is a very interesting solution. * SQL expressiveness: - We would also appreciate a SQL way to create workspaces (like a CREATE DATABASE). And progressively the integration of additional relational/analytical capabilities (INTERSECT/EXCEPT, and ROLLUP/CUBE). - I would like to have means to play a bit with the execution plans without touching the session options at each query. So a support of query hints could bring some interesting benefits. * Resource Management: - Drill made nice improvements related to the management of local resources (inside a DrillBit) with regards to CPU/Memory, but is still lacking on the resource management at the cluster level. Due to this limitation, we plan to manage one Drill cluster per functional group. So a mechanism of admission control with priority would help us to have to reduce the number of Drill clusters (and have bigger ones). * Resiliency: - When the clusters get large, the probability of losing nodes increases. It would be helpful to support resuming/restarting queries that were executing on a node that failed. * Hosting: - While we do store everything currently on the Hadoop clusters, we expect to move some of the historical storage to object stores. And we may have cases where the functional group handling the data does not need a Hadoop cluster, but still would like to be able to search through his dataset stored on the Object Store. As a longer term goal, being able to run Drill on Kubernetes would allow us to give the Drill power to these users on a shared infrastucture. * Drill Extensions: - Several querying enging support the inlining of UDF with the query. In Spark, the Querying DSL allows for Lambdas to be serialized with the query (in Java/Scala/Python/R). In BigQuery, the SQL supports primitives to inline JavaScript UDF (I found it crazy at first, but after a second thought, I found JS to be an excellent idea as a lightweight sandbox for untrusted code). For these kinds of extensions, a migration to Arrow as an internal value vector representation, would provide the basic libraries to exchange data between the interpreters. Regards, Joel On Wed, Aug 15, 2018 at 12:44 AM weijie tong wrote: > My thinking about this topic. Drill does well now. But be better,we need to > be idealist to bring in more use cases or more advanced query performance > compared to other projects like Flink , Spark, Presto,Impala. To > performance, I wonder do we need to adopt the project Gandiva which is so > exciting or we does our own similar implementation without migrating to > Arrow. If we choose to adopt it,we should go about migrating to Arrow. > Arrow is good at with some other language implementations which give more > options to
[GitHub] sachouche commented on a change in pull request #1433: DRILL-6685: Fixed exception when reading Parquet data
sachouche commented on a change in pull request #1433: DRILL-6685: Fixed
exception when reading Parquet data
URL: https://github.com/apache/drill/pull/1433#discussion_r210976231
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetBulkReader.java
##
@@ -0,0 +1,115 @@
+/*
+ * 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.store.parquet;
+
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Tests the Parquet bulk reader */
+@SuppressWarnings("deprecation")
Review comment:
@arina-ielchiieva
Thank you for the info and the sample test! that really helped.. I have now
ported the new Parquet test to using the ClusterTest base class.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on issue #1279: DRILL-5735: Allow search/sort in the Options webUI
kkhatua commented on issue #1279: DRILL-5735: Allow search/sort in the Options webUI URL: https://github.com/apache/drill/pull/1279#issuecomment-413931480 @arina-ielchiieva 1. The descriptions are, as of now, part of `sys.options_val` (and sys.internal_options_val`) due to the width of `sys.options` . I intend to use [DRILL-6684](https://issues.apache.org/jira/browse/DRILL-6684) to swap the names of the two system tables. That is a bigger exercise because unit tests, etc need to be adjusted accordingly. 2. I've dropped it from the last commit. I'll work with Bridget and other developers to update their features' property descriptions. As of now, we've barely got half of these covered. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Re: [ANNOUNCE] New PMC member: Boaz Ben-Zvi
Congratulations Boaz! - Paul On Friday, August 17, 2018, 2:56:27 AM PDT, Vitalii Diravka wrote: Congrats Boaz! Kind regards Vitalii On Fri, Aug 17, 2018 at 12:51 PM Arina Ielchiieva wrote: > I am pleased to announce that Drill PMC invited Boaz Ben-Zvi to the PMC and > he has accepted the invitation. > > Congratulations Boaz and thanks for your contributions! > > - Arina > (on behalf of Drill PMC) >
[GitHub] kkhatua commented on issue #1233: Updated with links to previous releases
kkhatua commented on issue #1233: Updated with links to previous releases URL: https://github.com/apache/drill/pull/1233#issuecomment-413925036 Closing this for now. Will reopen as part of a broader set of changes for the website. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] kkhatua closed pull request #1233: Updated with links to previous releases
kkhatua closed pull request #1233: Updated with links to previous releases
URL: https://github.com/apache/drill/pull/1233
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/download.html b/download.html
index 9651e43d0e5..8d70a2079d0 100755
--- a/download.html
+++ b/download.html
@@ -22,7 +22,32 @@
https://github.com/apache/drill/tree/{{
site.data.version.full_version }}">Source code
-If you're looking for an older release, see the release notes.
+
+ VersionDateRelease
NotesDownload
+
+1.13.018 March, 2018https://drill.apache.org/docs/apache-drill-1-13-0-release-notes/;>Apache
Drill 1.13.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.13.0/;>Download
1.13.0
+1.12.015 December, 2017https://drill.apache.org/docs/apache-drill-1-12-0-release-notes/;>Apache
Drill 1.12.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.12.0/;>Download
1.12.0
+1.11.0July 31, 2017https://drill.apache.org/docs/apache-drill-1-11-0-release-notes/;>Apache
Drill 1.11.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.11.0/;>Download
1.11.0
+1.10.015 March, 2017https://drill.apache.org/docs/apache-drill-1-10-0-release-notes/;>Apache
Drill 1.10.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.10.0/;>Download
1.10.0
+1.9.029 November, 2016https://drill.apache.org/docs/apache-drill-1-9-0-release-notes/;>Apache
Drill 1.9.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.9.0/;>Download
1.9.0
+1.8.030 August, 2016https://drill.apache.org/docs/apache-drill-1-8-0-release-notes/;>Apache
Drill 1.8.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.8.0/;>Download
1.8.0
+1.7.0June 28, 2016https://drill.apache.org/docs/apache-drill-1-7-0-release-notes/;>Apache
Drill 1.7.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.7.0/;>Download
1.7.0
+1.6.016 March, 2016https://drill.apache.org/docs/apache-drill-1-6-0-release-notes/;>Apache
Drill 1.6.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.6.0/;>Download
1.6.0
+1.5.016 February, 2016https://drill.apache.org/docs/apache-drill-1-5-0-release-notes/;>Apache
Drill 1.5.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.5.0/;>Download
1.5.0
+1.4.014 December, 2015https://drill.apache.org/docs/apache-drill-1-4-0-release-notes/;>Apache
Drill 1.4.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.4.0/;>Download
1.4.0
+1.3.0November 22, 2015https://drill.apache.org/docs/apache-drill-1-3-0-release-notes/;>Apache
Drill 1.3.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.3.0/;>Download
1.3.0
+1.2.016 October, 2015https://drill.apache.org/docs/apache-drill-1-2-0-release-notes/;>Apache
Drill 1.2.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.2.0/;>Download
1.2.0
+1.1.005 July, 2015https://drill.apache.org/docs/apache-drill-1-1-0-release-notes/;>Apache
Drill 1.1.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.1.0/;>Download
1.1.0
+1.0.019 May, 2015https://drill.apache.org/docs/apache-drill-1-0-0-release-notes/;>Apache
Drill 1.0.0 Release Noteshttp://archive.apache.org/dist/drill/drill-1.0.0/;>Download
1.0.0
+0.9.003 May, 2015https://drill.apache.org/docs/apache-drill-0-9-0-release-notes/;>Apache
Drill 0.9.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.9.0/;>Download
0.9.0
+0.8.025 March, 2015https://drill.apache.org/docs/apache-drill-0-8-0-release-notes/;>Apache
Drill 0.8.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.8.0/;>Download
0.8.0
+0.7.016 December, 2014https://drill.apache.org/docs/apache-drill-0-7-0-release-notes/;>Apache
Drill 0.7.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.7.0/;>Download
0.7.0
+0.6.001 October, 2014https://drill.apache.org/docs/apache-drill-0-6-0-release-notes/;>Apache
Drill 0.6.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.6.0-incubating/;>Download
0.6.0
+0.5.012 September, 2014https://drill.apache.org/docs/apache-drill-0-5-0-release-notes/;>Apache
Drill 0.5.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.5.0-incubating/;>Download
0.5.0
+0.4.031 July, 2014https://drill.apache.org/docs/apache-drill-0-4-0-release-notes/;>Apache
Drill 0.4.0 Release Noteshttp://archive.apache.org/dist/drill/drill-0.4.0-incubating/;>Download
0.4.0
+
+
+
[GitHub] arina-ielchiieva commented on a change in pull request #1433: DRILL-6685: Fixed exception when reading Parquet data
arina-ielchiieva commented on a change in pull request #1433: DRILL-6685: Fixed
exception when reading Parquet data
URL: https://github.com/apache/drill/pull/1433#discussion_r210962799
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetBulkReader.java
##
@@ -0,0 +1,115 @@
+/*
+ * 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.store.parquet;
+
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Tests the Parquet bulk reader */
+@SuppressWarnings("deprecation")
Review comment:
@sachouche actually you don't need to define schema upfront etc, you can
write test almost the same ways as before. Please take a look at this example:
https://github.com/apache/drill/blob/b8376cce07dc6b5fe2b3408a4b4a88c3b21816dd/exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestFilesTable.java
@ilooner since you originally deprecated `BaseTestQuery`, maybe he can
comment on this as well?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] arina-ielchiieva commented on a change in pull request #1433: DRILL-6685: Fixed exception when reading Parquet data
arina-ielchiieva commented on a change in pull request #1433: DRILL-6685: Fixed
exception when reading Parquet data
URL: https://github.com/apache/drill/pull/1433#discussion_r210962799
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetBulkReader.java
##
@@ -0,0 +1,115 @@
+/*
+ * 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.store.parquet;
+
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Tests the Parquet bulk reader */
+@SuppressWarnings("deprecation")
Review comment:
@sachouche actually you don't need to define schema upfront etc, you can
write test almost the same ways as before. Please take a look at, for an
example:
https://github.com/apache/drill/blob/b8376cce07dc6b5fe2b3408a4b4a88c3b21816dd/exec/java-exec/src/test/java/org/apache/drill/exec/store/ischema/TestFilesTable.java
@ilooner since you originally deprecated `BaseTestQuery`, maybe he can
comment on this as well?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sachouche commented on a change in pull request #1433: DRILL-6685: Fixed exception when reading Parquet data
sachouche commented on a change in pull request #1433: DRILL-6685: Fixed
exception when reading Parquet data
URL: https://github.com/apache/drill/pull/1433#discussion_r210958573
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetBulkReader.java
##
@@ -0,0 +1,115 @@
+/*
+ * 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.store.parquet;
+
+import org.apache.drill.test.BaseTestQuery;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Tests the Parquet bulk reader */
+@SuppressWarnings("deprecation")
Review comment:
@arina-ielchiieva,
- Majority of the tests are using BaseTestQuery; I even found existing tests
easier to write than with ClusterTest (setting session flags, no need to define
upfront schema, result-set validation). My guess, we should create a JIRA to
move all existing BaseTestQuery to ClusterTest so that DEV get a chance to get
more familiar with this test harness.
- I use Eclipse; the UI shows warnings all over the place
- Suppressing the deprecation allows me to focus on more important warnings
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka closed pull request #1436: DRILL-6696: IOBE in Operator Metric Registry
vdiravka closed pull request #1436: DRILL-6696: IOBE in Operator Metric Registry
URL: https://github.com/apache/drill/pull/1436
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorMetricRegistry.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorMetricRegistry.java
index dcb944512cf..ac867020de1 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorMetricRegistry.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorMetricRegistry.java
@@ -32,14 +32,18 @@
import org.apache.drill.exec.record.AbstractBinaryRecordBatch;
import org.apache.drill.exec.store.parquet.columnreaders.ParquetRecordReader;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
/**
* Registry of operator metrics.
*/
public class OperatorMetricRegistry {
// private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(OperatorMetricRegistry.class);
- // Mapping: operator type --> metric id --> metric name
- private static final String[][] OPERATOR_METRICS = new
String[CoreOperatorType.values().length][];
+ // Mapping: key : operator type, value : metric id --> metric name
+ private static final Map OPERATOR_METRICS = new
HashMap<>();
static {
register(CoreOperatorType.SCREEN_VALUE,
ScreenCreator.ScreenRoot.Metric.class);
@@ -61,13 +65,12 @@
private static void register(final int operatorType, final Class metricDef) {
// Currently registers a metric def that has enum constants
-final MetricDef[] enumConstants = metricDef.getEnumConstants();
+MetricDef[] enumConstants = metricDef.getEnumConstants();
if (enumConstants != null) {
- final String[] names = new String[enumConstants.length];
- for (int i = 0; i < enumConstants.length; i++) {
-names[i] = enumConstants[i].name();
- }
- OPERATOR_METRICS[operatorType] = names;
+ String[] names = Arrays.stream(enumConstants)
+ .map(MetricDef::name)
+ .toArray((String[]::new));
+ OPERATOR_METRICS.put(operatorType, names);
}
}
@@ -77,8 +80,8 @@ private static void register(final int operatorType, final
Classhttp://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;
+
+import org.apache.drill.categories.OperatorTest;
+import org.apache.drill.exec.ops.OperatorMetricRegistry;
+import org.apache.drill.exec.proto.UserBitShared;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+@Category(OperatorTest.class)
+public class TestOperatorMetrics extends BaseTestQuery {
+
+ @Test
+ public void testMetricNames() {
+assertEquals(new String[]{"BYTES_SENT"},
+
OperatorMetricRegistry.getMetricNames(UserBitShared.CoreOperatorType.SCREEN_VALUE));
+
+assertEquals(new String[]{"SPILL_COUNT", "RETIRED1",
"PEAK_BATCHES_IN_MEMORY", "MERGE_COUNT", "MIN_BUFFER",
+ "INPUT_BATCHES"},
+
OperatorMetricRegistry.getMetricNames(UserBitShared.CoreOperatorType.EXTERNAL_SORT_VALUE));
+ }
+
+ @Test
+ public void testNonExistentMetricNames() {
+
assertNull(OperatorMetricRegistry.getMetricNames(UserBitShared.CoreOperatorType.NESTED_LOOP_JOIN_VALUE));
+
+assertNull(OperatorMetricRegistry.getMetricNames(202));
+ }
+
+}
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] weijietong commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
weijietong commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#issuecomment-413870603 get it, thanks for the explanation. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#issuecomment-413868661 @weijietong, this PR is an intermediate step for https://github.com/apache/drill/pull/1264 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] weijietong commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
weijietong commented on issue #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#issuecomment-413867613 A question about this PR: what's the benefit we can gain with this replacement ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] arina-ielchiieva commented on issue #1126: DRILL-6179: Added pcapng-format support
arina-ielchiieva commented on issue #1126: DRILL-6179: Added pcapng-format support URL: https://github.com/apache/drill/pull/1126#issuecomment-413863585 Thanks Vlad! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] arina-ielchiieva commented on issue #1436: DRILL-6696: IOBE in Operator Metric Registry
arina-ielchiieva commented on issue #1436: DRILL-6696: IOBE in Operator Metric Registry URL: https://github.com/apache/drill/pull/1436#issuecomment-413863367 +1, LGTM. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] Vlad-Storona commented on issue #1126: DRILL-6179: Added pcapng-format support
Vlad-Storona commented on issue #1126: DRILL-6179: Added pcapng-format support URL: https://github.com/apache/drill/pull/1126#issuecomment-413863196 I have rebased to the latest master. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1436: DRILL-6696: IOBE in Operator Metric Registry
vdiravka commented on a change in pull request #1436: DRILL-6696: IOBE in
Operator Metric Registry
URL: https://github.com/apache/drill/pull/1436#discussion_r210902578
##
File path:
exec/java-exec/src/test/java/org/apache/drill/TestOperatorMetrics.java
##
@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+import org.apache.drill.categories.OperatorTest;
+import org.apache.drill.exec.ops.OperatorMetricRegistry;
+import org.apache.drill.exec.proto.UserBitShared;
+import org.apache.drill.test.BaseTestQuery;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+@Category(OperatorTest.class)
+public class TestOperatorMetrics extends BaseTestQuery {
+
+ @Test
+ public void testMetricNames() {
Review comment:
It makes sense. Done
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1436: DRILL-6696: IOBE in Operator Metric Registry
vdiravka commented on a change in pull request #1436: DRILL-6696: IOBE in
Operator Metric Registry
URL: https://github.com/apache/drill/pull/1436#discussion_r210902302
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorMetricRegistry.java
##
@@ -61,14 +66,11 @@
private static void register(final int operatorType, final Class metricDef) {
// Currently registers a metric def that has enum constants
-final MetricDef[] enumConstants = metricDef.getEnumConstants();
-if (enumConstants != null) {
- final String[] names = new String[enumConstants.length];
- for (int i = 0; i < enumConstants.length; i++) {
-names[i] = enumConstants[i].name();
- }
- OPERATOR_METRICS[operatorType] = names;
-}
+String[] names = Arrays.stream(metricDef.getEnumConstants())
Review comment:
My bad, not right `null` check. EnumConstants array should be checked.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210847475
##
File path:
common/src/main/java/org/apache/drill/common/collections/MapWithOrdinal.java
##
@@ -129,22 +127,17 @@ public void clear() {
@Override
public Collection values() {
- return Lists.newArrayList(Iterables.transform(secondary.entries(), new
Function, V>() {
-@Override
-public V apply(IntObjectMap.Entry entry) {
- return Preconditions.checkNotNull(entry).value();
-}
- }));
+ return StreamSupport.stream(secondary.entries().spliterator(), false)
+ .map(entry -> Objects.requireNonNull(entry).value())
+ .collect(Collectors.toList());
}
@Override
public Set> entrySet() {
- return Sets.newHashSet(Iterables.transform(primary.entrySet(), new
Function>, Entry>() {
-@Override
-public Entry apply(Entry> entry) {
- return new AbstractMap.SimpleImmutableEntry<>(entry.getKey(),
entry.getValue().getValue());
-}
- }));
+ return primary.entrySet().stream()
+ .map((Function>, Entry>) entry ->
Review comment:
Thanks for pointing this, removed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210890610
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/server/rest/spnego/TestSpnegoConfig.java
##
@@ -123,7 +123,7 @@ public void testSpnegoConfigOnlyKeytab() throws Exception {
@Test
public void testSpnegoConfigOnlyPrincipal() throws Exception {
try {
- final DrillConfig newConfig = new
DrillConfig(DrillConfig.create().withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
ConfigValueFactory.fromAnyRef(true)).withValue(ExecConstants.AUTHENTICATION_MECHANISMS,
ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))).withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL)).withValue(ExecConstants.USER_AUTHENTICATOR_IMPL,
ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)));
+ final DrillConfig newConfig = new
DrillConfig(DrillConfig.create().withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
ConfigValueFactory.fromAnyRef(true)).withValue(ExecConstants.AUTHENTICATION_MECHANISMS,
ConfigValueFactory.fromIterable(Collections.singletonList("plain"))).withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL)).withValue(ExecConstants.USER_AUTHENTICATOR_IMPL,
ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)));
Review comment:
done
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210880852
##
File path:
contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoGroupScan.java
##
@@ -558,7 +552,7 @@ public PhysicalOperator
getNewWithChildren(List children)
logger.debug("Took {} µs to get operator affinity",
watch.elapsed(TimeUnit.NANOSECONDS) / 1000);
logger.debug("Affined drillbits : " + affinityMap.values());
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#discussion_r210875477 ## File path: contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java ## @@ -46,7 +47,6 @@ import org.apache.thrift.TException; import com.google.common.collect.ImmutableList; Review comment: Replaced where possible `ImmutableList.of()` by `Collections.emptyList()`. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210892513
##
File path:
exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java
##
@@ -69,23 +69,19 @@ public static void tearDownClass() {
public void test() throws Exception {
withNoDefaultSchema()
.withConnection(
-new Function() {
- public Void apply( Connection connection ) {
-try {
- Statement statement = connection.createStatement();
- ResultSet resultSet = statement.executeQuery( "USE dfs.tmp" );
- // TODO: Purge nextUntilEnd(...) and calls when remaining
fragment
- // race conditions are fixed (not just DRILL-2245 fixes).
- // resultSet.close( resultSet );
- statement.close();
- // connection.close() is in withConnection(...)
- return null;
-} catch ( SQLException e ) {
- throw new RuntimeException( e );
-}
+(Function) connection -> {
Review comment:
I don't think that it is essential at least for current PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210885040
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
##
@@ -586,9 +587,9 @@ public BatchGroup mergeAndSpill(LinkedList
batchGroups) throws Schem
c1.setRecordCount(count);
String spillDir = dirs.next();
-Path currSpillPath = new Path(Joiner.on("/").join(spillDir, fileName));
+Path currSpillPath = new Path(spillDir + "/" + fileName);
currSpillDirs.add(currSpillPath);
-String outputFile = Joiner.on("/").join(currSpillPath, spillCount++);
+String outputFile = currSpillPath+ "/" + spillCount++;
Review comment:
`String.join()` requires only `CharSequence` arguments, but the last one is
`int`. Added space.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210885867
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/StatsCollector.java
##
@@ -99,9 +99,9 @@ public Void visitStore(Store store, Wrapper wrapper) {
@Override
public Void visitOp(PhysicalOperator op, Wrapper wrapper) {
-final Stats stats = wrapper.getStats();
+Stats stats = wrapper.getStats();
if (op instanceof HasAffinity) {
- final HasAffinity hasAffinity = (HasAffinity)op;
+ HasAffinity hasAffinity = (HasAffinity)op;
Review comment:
added
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210884817
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
##
@@ -586,9 +587,9 @@ public BatchGroup mergeAndSpill(LinkedList
batchGroups) throws Schem
c1.setRecordCount(count);
String spillDir = dirs.next();
-Path currSpillPath = new Path(Joiner.on("/").join(spillDir, fileName));
+Path currSpillPath = new Path(spillDir + "/" + fileName);
Review comment:
Thanks, used it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210848117
##
File path: common/src/test/java/org/apache/drill/test/SubDirTestWatcher.java
##
@@ -83,10 +84,10 @@
private List subDirs;
protected SubDirTestWatcher(File baseDir, boolean createAtBeginning, boolean
deleteAtEnd, List subDirs) {
-this.baseDir = Preconditions.checkNotNull(baseDir);
+this.baseDir = Objects.requireNonNull(baseDir);
this.createAtBeginning = createAtBeginning;
this.deleteAtEnd = deleteAtEnd;
-this.subDirs = Preconditions.checkNotNull(subDirs);
+this.subDirs = Objects.requireNonNull(subDirs);
Preconditions.checkArgument(!subDirs.isEmpty(), "The list of subDirs is
empty.");
Review comment:
See my previous comment
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210880245
##
File path:
contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoGroupScan.java
##
@@ -278,8 +273,8 @@ private void init() throws IOException {
}
private void handleUnshardedCollection(List hosts) {
-String chunkName = Joiner.on('.').join(scanSpec.getDbName(),
scanSpec.getCollectionName());
-Set addressList = Sets.newHashSet();
+String chunkName = String.join(".", Arrays.asList(scanSpec.getDbName(),
scanSpec.getCollectionName()));
Review comment:
Thanks, removed list creation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210837966
##
File path: common/src/main/java/org/apache/drill/common/KerberosUtil.java
##
@@ -57,9 +57,9 @@ public static String getPrincipalFromParts(final String
primary, final String in
public static String[] splitPrincipalIntoParts(final String principal) {
final String[] components = principal.split("[/@]");
checkState(components.length == 3);
Review comment:
I think we should leave it as it is since the goal of this PR is to replace
guava usage by JDK where it is possible, but there is no sense to replace usage
of one library by another.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#discussion_r210882583 ## File path: exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java ## @@ -23,7 +23,6 @@ package org.apache.drill.exec.store; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; Review comment: Thanks, removed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210884029
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/registry/LocalFunctionRegistry.java
##
@@ -296,20 +297,20 @@ private void
registerOperatorsWithInference(DrillOperatorTable operatorTable, Ma
mapAgg.put(name, new
DrillSqlAggOperator.DrillSqlAggOperatorBuilder().setName(name));
}
-final DrillSqlAggOperator.DrillSqlAggOperatorBuilder
drillSqlAggOperatorBuilder = mapAgg.get(name);
+DrillSqlAggOperator.DrillSqlAggOperatorBuilder
drillSqlAggOperatorBuilder = mapAgg.get(name);
drillSqlAggOperatorBuilder
.addFunctions(entry.getValue())
.setArgumentCount(entry.getKey(), entry.getKey());
}
}
-for(final Entry entry :
map.entrySet()) {
+for (Entry entry :
map.entrySet()) {
operatorTable.addOperatorWithInference(
entry.getKey(),
entry.getValue().build());
}
-for(final Entry
entry : mapAgg.entrySet()) {
+for(Entry entry :
mapAgg.entrySet()) {
Review comment:
added
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210884296
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
##
@@ -559,13 +558,13 @@ private void setupHashTable() throws
SchemaChangeException {
leftExpr = null;
} else {
if (probeBatch.getSchema().getSelectionVectorMode() !=
BatchSchema.SelectionVectorMode.NONE) {
-final String errorMsg = new StringBuilder().append("Hash join does not
support probe batch with selection vectors. ").append("Probe batch has
selection mode = ").append
+String errorMsg = new StringBuilder().append("Hash join does not
support probe batch with selection vectors. ").append("Probe batch has
selection mode = ").append
Review comment:
done
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210883577
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
##
@@ -127,7 +126,7 @@ public boolean equals(Object obj) {
}
}
- Map previousExpressions =
Maps.newHashMap();
+ Map previousExpressions = new HashMap<>();
Review comment:
added
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210886866
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java
##
@@ -163,199 +163,196 @@ public void reduce(final RexBuilder rexBuilder,
List constExps, final L
continue;
}
- Function literator = new Function() {
-@Override
-public RexNode apply(ValueHolder output) {
- switch(materializedExpr.getMajorType().getMinorType()) {
-case INT: {
- int value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-((NullableIntHolder) output).value : ((IntHolder)
output).value;
- return rexBuilder.makeLiteral(new BigDecimal(value),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.INTEGER, newCall.getType().isNullable()), false);
-}
-case BIGINT: {
- long value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-((NullableBigIntHolder) output).value : ((BigIntHolder)
output).value;
- return rexBuilder.makeLiteral(new BigDecimal(value),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.BIGINT, newCall.getType().isNullable()), false);
-}
-case FLOAT4: {
- float value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-((NullableFloat4Holder) output).value : ((Float4Holder)
output).value;
- return rexBuilder.makeLiteral(new BigDecimal(value),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.FLOAT, newCall.getType().isNullable()), false);
-}
-case FLOAT8: {
- double value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-((NullableFloat8Holder) output).value : ((Float8Holder)
output).value;
- return rexBuilder.makeLiteral(new BigDecimal(value),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.DOUBLE, newCall.getType().isNullable()), false);
-}
-case VARCHAR: {
- String value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-
StringFunctionHelpers.getStringFromVarCharHolder((NullableVarCharHolder)output)
:
-
StringFunctionHelpers.getStringFromVarCharHolder((VarCharHolder)output);
- return rexBuilder.makeLiteral(value,
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.VARCHAR, newCall.getType().isNullable()), false);
-}
-case BIT: {
- boolean value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-((NullableBitHolder) output).value == 1 : ((BitHolder)
output).value == 1;
- return rexBuilder.makeLiteral(value,
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.BOOLEAN, newCall.getType().isNullable()), false);
-}
-case DATE: {
- Calendar value = (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) ?
-new DateTime(((NullableDateHolder) output).value,
DateTimeZone.UTC).toCalendar(null) :
-new DateTime(((DateHolder) output).value,
DateTimeZone.UTC).toCalendar(null);
- return
rexBuilder.makeLiteral(DateString.fromCalendarFields(value),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.DATE, newCall.getType().isNullable()), false);
-}
-case DECIMAL9: {
- long value;
- int scale;
- if (materializedExpr.getMajorType().getMode() ==
TypeProtos.DataMode.OPTIONAL) {
-NullableDecimal9Holder decimal9Out =
(NullableDecimal9Holder)output;
-value = decimal9Out.value;
-scale = decimal9Out.scale;
- } else {
-Decimal9Holder decimal9Out = (Decimal9Holder)output;
-value = decimal9Out.value;
-scale = decimal9Out.scale;
- }
- return rexBuilder.makeLiteral(
-new BigDecimal(BigInteger.valueOf(value), scale),
-
TypeInferenceUtils.createCalciteTypeWithNullability(typeFactory,
SqlTypeName.DECIMAL, newCall.getType().isNullable()),
-false);
-}
-case DECIMAL18: {
- long value;
- int scale;
- if (materializedExpr.getMajorType().getMode() ==
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210877043
##
File path:
contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java
##
@@ -157,10 +155,10 @@ public JdbcStoragePlugin getPlugin() {
}
/**
- * Returns whether a condition is supported by {@link JdbcJoin}.
+ * Returns whether a condition is supported by {@link JdbcRules.JdbcJoin}.
*
* Corresponds to the capabilities of
- * {@link SqlImplementor#convertConditionToSqlNode}.
+ * {@link
org.apache.calcite.rel.rel2sql.SqlImplementor#convertConditionToSqlNode}.
Review comment:
Due to https://issues.apache.org/jira/browse/DRILL-6677 I think it is better
to leave it as it is until this Jira is resolved.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210883199
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZookeeperClient.java
##
@@ -192,24 +190,24 @@ public boolean hasPath(final String path, final boolean
consistent, final DataCh
* @param consistent consistency check
* @param version version holder
*/
- public byte[] get(final String path, final boolean consistent, final
DataChangeVersion version) {
-Preconditions.checkNotNull(path, "path is required");
+ public byte[] get(String path, boolean consistent, DataChangeVersion
version) {
+Objects.requireNonNull(path, "path is required");
-final String target = PathUtils.join(root, path);
+String target = PathUtils.join(root, path);
if (consistent) {
try {
if (version != null) {
Stat stat = new Stat();
- final byte[] bytes =
curator.getData().storingStatIn(stat).forPath(target);
+ byte[] bytes = curator.getData().storingStatIn(stat).forPath(target);
version.setVersion(stat.getVersion());
return bytes;
}
return curator.getData().forPath(target);
- } catch (final Exception ex) {
+ } catch (Exception ex) {
Review comment:
AFAIK, there is no convention for such cases. `ex` also looks OK.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210887786
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/schema/ObjectSchema.java
##
@@ -71,19 +70,16 @@ public void resetMarkedFields() {
@Override
public Iterable removeUnreadFields() {
-final List removedFields = Lists.newArrayList();
-Iterables.removeIf(fields.values(), new Predicate() {
-@Override
-public boolean apply(Field field) {
-if (!field.isRead()) {
-removedFields.add(field);
-return true;
-} else if (field.hasSchema()) {
-Iterables.addAll(removedFields,
field.getAssignedSchema().removeUnreadFields());
-}
-
-return false;
+List removedFields = new ArrayList<>();
+fields.values().removeIf(field -> {
+if (!field.isRead()) {
+removedFields.add(field);
+return true;
+} else if (field.hasSchema()) {
+Iterables.addAll(removedFields,
field.getAssignedSchema().removeUnreadFields());
Review comment:
I think it would be better to left `Iterables.addAll`, since it looks
clearer and does not allocate a new array.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210885761
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SoftAffinityFragmentParallelizer.java
##
@@ -137,17 +131,17 @@ public void parallelizeFragment(final Wrapper
fragmentWrapper, final Paralleliza
if (endpoints.size() < width) {
// Get a list of endpoints that are not part of the affinity endpoint
list
List endpointsWithNoAffinity;
- final Set endpointsWithAffinity =
endpointAffinityMap.keySet();
+ Set endpointsWithAffinity =
endpointAffinityMap.keySet();
if (endpointAffinityMap.size() > 0) {
-endpointsWithNoAffinity = Lists.newArrayList();
+endpointsWithNoAffinity = new ArrayList<>();
for (DrillbitEndpoint ep : activeEndpoints) {
Review comment:
Thanks, replaced it with stream and list creation with collect
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210888793
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
##
@@ -242,7 +241,7 @@ public String getAdminUserGroups(OptionManager
optionManager) {
// if this option has not been changed by the user then return the
// process user groups
if (adminUserGroups.equals(DEFAULT_ADMIN_USER_GROUPS)) {
-adminUserGroups =
Joiner.on(",").join(ImpersonationUtil.getProcessUserGroupNames());
+adminUserGroups = String.join(",",
ImpersonationUtil.getProcessUserGroupNames());
Review comment:
If you meant a comma inside the string literal, then I doubt that space will
be useful there, since the result of this method is often split by `","`. So I
leave it as it is.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210882276
##
File path:
contrib/storage-mongo/src/test/java/org/apache/drill/exec/store/mongo/TestMongoChunkAssignment.java
##
@@ -162,7 +163,7 @@ public void setUp() throws UnknownHostException {
@Test
public void testMongoGroupScanAssignmentMix() throws UnknownHostException,
ExecutionSetupException {
-final List endpoints = Lists.newArrayList();
+final List endpoints = new ArrayList<>();
Review comment:
Let's concentrate on the changes which I did, since the project has a lot of
places, where code may be improved. Also, using `final` keyword is not critical
in general and may be used at the discretion of the developer in similar cases.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210889890
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaFilter.java
##
@@ -77,12 +77,9 @@ public FunctionExprNode(String function, List
args) {
@Override
public String toString() {
- StringBuilder builder = new StringBuilder();
- builder.append(function);
- builder.append("(");
- builder.append(Joiner.on(",").join(args));
- builder.append(")");
- return builder.toString();
+ return function + args.stream()
+ .map(ExprNode::toString)
+ .collect(Collectors.joining(",", "(", ")"));
Review comment:
Adding a space here may cause failures of tests with checks for the query
plan. You may create a Jira for further improvement of the output, but I don't
think that we should do it in the scope of this Jira.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210858864
##
File path:
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/config/HBasePersistentStore.java
##
@@ -138,9 +137,11 @@ public synchronized void delete(String key) {
@Override
public Iterator> getRange(int skip, int take) {
-final Iterator> iter = new Iter(take);
-Iterators.advance(iter, skip);
-return Iterators.limit(iter, take);
+Iterable> entries = () -> new Iter(take);
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace
usage of Guava classes by JDK ones
URL: https://github.com/apache/drill/pull/1397#discussion_r210876020
##
File path:
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/fn/hive/TestInbuiltHiveUDFs.java
##
@@ -62,16 +62,16 @@ public void testEncode() throws Exception {
@Test
public void testXpath_Double() throws Exception {
-final String query = "select xpath_double ('2040',
'a/b * a/c') as col \n" +
+String query = "select xpath_double ('2040', 'a/b *
a/c') as col \n" +
"from hive.kv \n" +
Review comment:
I think the current position of `limit` is ok since it helps to stand out
its presence.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#discussion_r210848039 ## File path: contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/MapRDBGroupScan.java ## @@ -42,9 +44,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; Review comment: The same as for `Preconditions.checkArgument()` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones
vvysotskyi commented on a change in pull request #1397: DRILL-6633: Replace usage of Guava classes by JDK ones URL: https://github.com/apache/drill/pull/1397#discussion_r210850249 ## File path: contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePluginConfig.java ## @@ -30,7 +31,6 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; Review comment: I assume `ImmutableMap` was used here to prevent modification of the returned map. So I leave it as it is. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
