[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r315598163
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/log/TestLogReader.java
##
@@ -781,9 +781,19 @@ public void testPluginSerialization() throws IOException {
@Test
public void testFirewallSchema() throws RpcException {
-String sql = "SELECT * FROM cp.`regex/firewall.ssdlog`";
+String sql = "SELECT * FROM cp.`regex/firewall.ssdlog` limit 0";
RowSet result = client.queryBuilder().sql(sql).rowSet();
-result.print();
+TupleMetadata expectedSchema = new SchemaBuilder()
Review comment:
I have created https://issues.apache.org/jira/browse/DRILL-7354 to address
this change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314945584
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/log/TestLogReader.java
##
@@ -781,9 +781,19 @@ public void testPluginSerialization() throws IOException {
@Test
public void testFirewallSchema() throws RpcException {
-String sql = "SELECT * FROM cp.`regex/firewall.ssdlog`";
+String sql = "SELECT * FROM cp.`regex/firewall.ssdlog` limit 0";
RowSet result = client.queryBuilder().sql(sql).rowSet();
-result.print();
+TupleMetadata expectedSchema = new SchemaBuilder()
Review comment:
I have reverted this change since it caused a lot of tests failures. I'll
create a Jira to replace usage of another `SchemaBuilder` with
`BatchSchemaBuilder`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314824297
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -17,24 +17,26 @@
*/
package org.apache.drill.test;
+import java.io.OutputStreamWriter;
import java.util.Iterator;
import org.apache.drill.exec.exception.SchemaChangeException;
import org.apache.drill.exec.memory.BufferAllocator;
+import org.apache.drill.exec.physical.rowSet.RowSetStringBuilder;
import org.apache.drill.exec.proto.UserBitShared.QueryId;
import org.apache.drill.exec.proto.UserBitShared.QueryResult.QueryState;
import org.apache.drill.exec.proto.helper.QueryIdHelper;
import org.apache.drill.exec.record.RecordBatchLoader;
import org.apache.drill.exec.record.VectorContainer;
import org.apache.drill.exec.rpc.user.QueryDataBatch;
import org.apache.drill.test.BufferingQueryEventListener.QueryEvent;
-import org.apache.drill.test.rowSet.DirectRowSet;
+import org.apache.drill.exec.physical.rowSet.DirectRowSet;
public class QueryRowSetIterator implements Iterator,
Iterable {
private final BufferingQueryEventListener listener;
private int recordCount = 0;
private int batchCount = 0;
- QueryId queryId = null;
+ private QueryId queryId = null;
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314823770
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/log/TestLogReader.java
##
@@ -781,9 +781,19 @@ public void testPluginSerialization() throws IOException {
@Test
public void testFirewallSchema() throws RpcException {
-String sql = "SELECT * FROM cp.`regex/firewall.ssdlog`";
+String sql = "SELECT * FROM cp.`regex/firewall.ssdlog` limit 0";
RowSet result = client.queryBuilder().sql(sql).rowSet();
-result.print();
+TupleMetadata expectedSchema = new SchemaBuilder()
+.add("eventDate", TypeProtos.MinorType.TIMESTAMP,
TypeProtos.DataMode.OPTIONAL)
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314829114
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetWriterImpl.java
##
@@ -129,7 +129,7 @@ public RowSetWriter addRow(Object...values) {
@Override
public RowSetWriter addSingleCol(Object value) {
-return addRow(new Object[] {value});
+return addRow(value);
Review comment:
No need to cast, it was an example. In the method where the value is used
reference type is already `Object`, so we don't need to do a cast.
> Unless we wrap that single column in another array, the second form is the
same as:
> ```
> ... addRow(new Object[] {10, "foo"});
> ```
Actually not, it will be the same as
```
addRow((Object) new Object[] {10, "foo"});
```
because method argument in `addSingleCol` has `Object` type.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314823714
##
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/store/log/TestLogReader.java
##
@@ -781,9 +781,19 @@ public void testPluginSerialization() throws IOException {
@Test
public void testFirewallSchema() throws RpcException {
-String sql = "SELECT * FROM cp.`regex/firewall.ssdlog`";
+String sql = "SELECT * FROM cp.`regex/firewall.ssdlog` limit 0";
RowSet result = client.queryBuilder().sql(sql).rowSet();
-result.print();
+TupleMetadata expectedSchema = new SchemaBuilder()
Review comment:
I went further, removed usage of another `SchemaBuilder` and removed the
class. We already have `BatchSchemaBuilder`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314809061
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,112 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ * Example of the output:
+ *
+ * #: year, month, day
+ * 0: 2017, 12, 17
+ * 1: 2017, 12, 18
+ * 2: 2017, 12, 19
+ *
+ */
+public class RowSetStringBuilder {
Review comment:
Thanks, made the changes you have proposed.
Regarding writing to logs, we have `LogWriter` which implements `Writer`,
and with this class, we will be able to write to logs also.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314805977
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -101,7 +103,7 @@ public DirectRowSet next() {
public void printAll() {
for (DirectRowSet rowSet : this) {
- rowSet.print();
+ new RowSetStringBuilder(rowSet).writeTo(new
OutputStreamWriter(System.out));
Review comment:
Thanks, replaced it with `RowSetFormatter.print(rowSet);`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314798843
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetWriterImpl.java
##
@@ -129,7 +129,7 @@ public RowSetWriter addRow(Object...values) {
@Override
public RowSetWriter addSingleCol(Object value) {
-return addRow(new Object[] {value});
+return addRow(value);
Review comment:
Tests passed with this change. I'm afraid that we are talking about
different cases.
`addRow(new Object[]{"foo", 10})` will work as you expected - it will handle
incoming array as two column values, but the code in the method which was
changed is different a little bit, it is similar to this call:
`addRow((Object) new Object[]{"foo", 10})` - in this case, it will be
handled as a single column with array value.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314679601
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractRowSet.java
##
@@ -41,13 +38,19 @@ public AbstractRowSet(VectorContainer container,
TupleMetadata schema) {
}
@Override
- public VectorAccessible vectorAccessible() { return container(); }
+ public VectorAccessible vectorAccessible() {
Review comment:
I have created https://issues.apache.org/jira/browse/DRILL-7352 for this.
I'll populate it with more rules soon.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314448553
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetWriterImpl.java
##
@@ -129,7 +129,7 @@ public RowSetWriter addRow(Object...values) {
@Override
public RowSetWriter addSingleCol(Object value) {
-return addRow(new Object[] {value});
+return addRow(value);
Review comment:
I made this change because IDE proposed it but after some investigation, I
still think that it will work as it is expected. Java determines whether to
treat the incoming argument as a single element or as an array for vararg
functions at the compilation stage. So for the following case code will work as
you have described - the vararg will be a one-dimensional array of strings:
```
addRow(new String[]{"a", "b"});
```
But the following case will have different behavior - vararg will be treated
as a two-dimensional array:
```
Object s = new String[]{"a", "a"};
addRow(s);
```
So code here will work as it is expected.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314640505
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,112 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ * Example of the output:
+ *
+ * #: year, month, day
+ * 0: 2017, 12, 17
+ * 1: 2017, 12, 18
+ * 2: 2017, 12, 19
+ *
+ */
+public class RowSetStringBuilder {
Review comment:
Good point, thanks, reworked this class to use `Writer`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314656744
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -99,13 +99,6 @@ public DirectRowSet next() {
}
}
- public void printAll() {
Review comment:
Reverted removing this method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314655457
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/package-info.java
##
@@ -117,12 +134,14 @@
* are added while loading, their index is always at the end of the existing
* columns.
* Writing Data to the Batch
- * Each batch is delimited by a call to {@link #startBatch()} and a call to
- * {@link #harvestWithLookAhead()} to obtain the completed batch. Note that
readers do not
+ * Each batch is delimited by a call to {@link
org.apache.drill.exec.physical.rowSet.ResultSetLoader#startBatch()}
+ * and a call to {@link
org.apache.drill.exec.physical.rowSet.impl.VectorState#harvestWithLookAhead()}
+ * to obtain the completed batch. Note that readers do not
* call these methods; the scan operator does this work.
*
- * Each row is delimited by a call to {@link #startValue()} and a call to
- * {@link #saveRow()}. startRow() performs initialization necessary
+ * Each row is delimited by a call to {@link
org.apache.drill.exec.vector.accessor.writer.WriterEvents#startRow()}
Review comment:
Thanks for pointing this, reverted this change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314436298
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/DirectRowSet.java
##
@@ -141,5 +136,22 @@ public SingleRowSet toIndirect(Set skipIndices) {
}
@Override
- public SelectionVector2 getSv2() { return null; }
+ public SelectionVector2 getSv2() {
+return null;
+ }
+
+ public static class RowSetWriterBuilder extends BaseWriterBuilder {
Review comment:
These classes are still nested and placed in the same top-level class, but
they have moved to the bottom of the top-level class.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314432299
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractRowSet.java
##
@@ -41,13 +38,19 @@ public AbstractRowSet(VectorContainer container,
TupleMetadata schema) {
}
@Override
- public VectorAccessible vectorAccessible() { return container(); }
+ public VectorAccessible vectorAccessible() {
Review comment:
One-line methods are prevented in a lot of projects. Here is a couple of the
checkstyle rules which forbid such style:
https://checkstyle.sourceforge.io/config_blocks.html#LeftCurly and
https://checkstyle.sourceforge.io/config_blocks.html#RightCurly. I think at
some point, we will update the code to enable these and other rules.
Also, most of IDE can compress displaying such methods automatically.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314652572 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/package-info.java ## @@ -32,6 +32,22 @@ * batch" or "record batch." (But, in Drill, the term "record batch" also * usually means an operator on that set of records. Here, a row set is * just the rows &nash; separate from operations on that data. + * SingleRowSet (abstract) + * Represents a row set that contains a single record batch (the typical + * case. + * DirectRowSet + * A read-only single row set without a selection vector. + * IndirectRowSet + * A read-only, single row set with an SV2. Note that the SV2 itself is + * writable (such as for sorting.) + * ExtendableRowSet + * A write-only, single row set used to create a new row set. Because of + * the way Drill sets row counts, an extendable row set cannot be read; instead + * at the completion of the write the extendable row set becomes a direct or + * indirect row set. + * HyperRowSet + * A read-only row set made up of a collection of record batches, indexed via an + * SV4. As with the SV2, the SV4 itself is writable. Review comment: This part was moved from `package-info.java` in tests package. Removed this sentence to avoid confusion. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314434313 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractRowSet.java ## @@ -15,23 +15,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; import org.apache.drill.exec.memory.BufferAllocator; import org.apache.drill.exec.record.BatchSchema; import org.apache.drill.exec.record.VectorAccessible; import org.apache.drill.exec.record.VectorContainer; import org.apache.drill.exec.record.metadata.TupleMetadata; -import org.apache.drill.exec.vector.SchemaChangeCallBack; /** * Basic implementation of a row set for both the single and multiple - * (hyper) varieties, both the fixed and extendible varieties. + * (hyper) varieties, both the fixed and extendable varieties. Review comment: Thanks, done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314648059
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetWriterImpl.java
##
@@ -129,7 +129,7 @@ public RowSetWriter addRow(Object...values) {
@Override
public RowSetWriter addSingleCol(Object value) {
-return addRow(new Object[] {value});
+return addRow(value);
Review comment:
Actually, it should work as expected. Java determines how to treat incoming
arguments at the compilation stage, so if the reference has a non-array type,
it will be treated as a single element of vararg even if actual type of the
object is an array.
Here is these two different cases:
```
addRow(new String[]{"a", "a"});
```
and the same case as in the code:
```
Object value = new String[]{"a", "a"};
addRow(value);
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314435143
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractSingleRowSet.java
##
@@ -15,37 +15,27 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.drill.test.rowSet;
+package org.apache.drill.exec.physical.rowSet;
import org.apache.drill.exec.record.RecordBatchSizer;
import org.apache.drill.exec.physical.rowSet.model.ReaderIndex;
import
org.apache.drill.exec.physical.rowSet.model.MetadataProvider.MetadataRetrieval;
import org.apache.drill.exec.physical.rowSet.model.single.BaseReaderBuilder;
import org.apache.drill.exec.record.VectorContainer;
import org.apache.drill.exec.record.metadata.TupleMetadata;
-import org.apache.drill.test.rowSet.RowSet.SingleRowSet;
+import org.apache.drill.exec.physical.rowSet.RowSet.SingleRowSet;
/**
* Base class for row sets backed by a single record batch.
*/
public abstract class AbstractSingleRowSet extends AbstractRowSet implements
SingleRowSet {
- public static class RowSetReaderBuilder extends BaseReaderBuilder {
-
-public RowSetReader buildReader(AbstractSingleRowSet rowSet, ReaderIndex
rowIndex) {
- TupleMetadata schema = rowSet.schema();
- return new RowSetReaderImpl(schema, rowIndex,
- buildContainerChildren(rowSet.container(),
- new MetadataRetrieval(schema)));
-}
- }
-
- public AbstractSingleRowSet(AbstractSingleRowSet rowSet) {
+ protected AbstractSingleRowSet(AbstractSingleRowSet rowSet) {
Review comment:
These classes weren't removed, but they moved to the bottom of the top-level
class. I believe we follow such a style in the project.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314448553
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetWriterImpl.java
##
@@ -129,7 +129,7 @@ public RowSetWriter addRow(Object...values) {
@Override
public RowSetWriter addSingleCol(Object value) {
-return addRow(new Object[] {value});
+return addRow(value);
Review comment:
I made this change because IDE proposed it but after some investigation, I
still think that it will work as it is expected. Java determines whether to
treat the incoming argument as a single element or as an array for vararg
functions at the compilation stage. So for the following case code will work as
you have described - the vararg will be a one-dimensional array of strings:
```
addRow(new String[]{"a", "b"});
```
But the following case will have different behavior - vararg will be treated
as a two-dimensional array:
```
Object s = new String[]{"a", "a"};
addRow(s);
```
So code here will work as it is expected.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314649381 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/package-info.java ## @@ -32,6 +32,22 @@ * batch" or "record batch." (But, in Drill, the term "record batch" also * usually means an operator on that set of records. Here, a row set is * just the rows &nash; separate from operations on that data. + * SingleRowSet (abstract) Review comment: Moved `resultSet` classes into a separate package and updated `package-info.java`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314387494
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,111 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ */
+public class RowSetStringBuilder {
+ private RowSet rowSet;
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314388393
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,111 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ */
+public class RowSetStringBuilder {
+ private RowSet rowSet;
+
+ public RowSetStringBuilder(RowSet rowSet) {
+this.rowSet = rowSet;
+ }
+
+ public static void appendTupleSchema(StringBuilder stringBuilder,
TupleMetadata schema) {
+for (int i = 0; i < schema.size(); i++) {
+ if (i > 0) {
+stringBuilder.append(", ");
+ }
+ ColumnMetadata colSchema = schema.metadata(i);
+ stringBuilder.append(colSchema.name());
+ if (colSchema.isMap()) {
+stringBuilder.append("{");
+appendTupleSchema(stringBuilder, colSchema.mapSchema());
+stringBuilder.append("}");
+ }
+}
+ }
+
+ @Override
+ public String toString() {
+StringBuilder result = new StringBuilder();
+appendTo(result);
+return result.toString();
+ }
+
+ public void appendTo(StringBuilder stringBuilder) {
Review comment:
Thanks, done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314387694
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,111 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ */
+public class RowSetStringBuilder {
+ private RowSet rowSet;
+
+ public RowSetStringBuilder(RowSet rowSet) {
+this.rowSet = rowSet;
+ }
+
+ public static void appendTupleSchema(StringBuilder stringBuilder,
TupleMetadata schema) {
Review comment:
Made private and non-static.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314388642
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java
##
@@ -0,0 +1,111 @@
+/*
+ * 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.physical.rowSet;
+
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+
+/**
+ * Helper class to obtain string representation of RowSet.
+ */
+public class RowSetStringBuilder {
+ private RowSet rowSet;
+
+ public RowSetStringBuilder(RowSet rowSet) {
+this.rowSet = rowSet;
+ }
+
+ public static void appendTupleSchema(StringBuilder stringBuilder,
TupleMetadata schema) {
+for (int i = 0; i < schema.size(); i++) {
+ if (i > 0) {
+stringBuilder.append(", ");
+ }
+ ColumnMetadata colSchema = schema.metadata(i);
+ stringBuilder.append(colSchema.name());
+ if (colSchema.isMap()) {
+stringBuilder.append("{");
+appendTupleSchema(stringBuilder, colSchema.mapSchema());
+stringBuilder.append("}");
+ }
+}
+ }
+
+ @Override
+ public String toString() {
+StringBuilder result = new StringBuilder();
+appendTo(result);
+return result.toString();
+ }
+
+ public void appendTo(StringBuilder stringBuilder) {
+SelectionVectorMode selectionMode = rowSet.indirectionType();
+RowSetReader reader = rowSet.reader();
+int colCount = reader.tupleSchema().size();
+appendSchema(stringBuilder, selectionMode, reader);
+while (reader.next()) {
+ appendHeader(stringBuilder, reader, selectionMode);
+ for (int i = 0; i < colCount; i++) {
+if (i > 0) {
+ stringBuilder.append(", ");
+}
+stringBuilder.append(reader.column(i).getAsString());
+ }
+ stringBuilder.append("\n");
+}
+ }
+
+ private void appendSchema(StringBuilder stringBuilder, SelectionVectorMode
selectionMode, RowSetReader reader) {
+stringBuilder.append("#");
+switch (selectionMode) {
+ case FOUR_BYTE:
+stringBuilder.append(" (batch #, row #)");
+break;
+ case TWO_BYTE:
+stringBuilder.append(" (row #)");
+break;
+ default:
+break;
+}
+stringBuilder.append(": ");
+TupleMetadata schema = reader.tupleSchema();
+appendTupleSchema(stringBuilder, schema);
+stringBuilder.append("\n");
+ }
+
+ private void appendHeader(StringBuilder stringBuilder, RowSetReader reader,
SelectionVectorMode selectionMode) {
+stringBuilder.append(reader.logicalIndex());
+switch (selectionMode) {
+ case FOUR_BYTE:
+stringBuilder.append(" (");
+stringBuilder.append(reader.hyperVectorIndex());
+stringBuilder.append(", ");
+stringBuilder.append(reader.offset());
+stringBuilder.append(")");
+break;
+ case TWO_BYTE:
+stringBuilder.append(" (");
+stringBuilder.append(reader.offset());
+stringBuilder.append(")");
+break;
+ default:
Review comment:
No, it is not required.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314388748 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetStringBuilder.java ## @@ -0,0 +1,111 @@ +/* + * 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.physical.rowSet; + +import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode; +import org.apache.drill.exec.record.metadata.ColumnMetadata; +import org.apache.drill.exec.record.metadata.TupleMetadata; + +/** + * Helper class to obtain string representation of RowSet. Review comment: Agree, added an example. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314362662 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/IndirectRowSet.java ## @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Thanks, replaced with `Collections.emptySet()`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314367564 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetReader.java ## @@ -46,8 +46,7 @@ * The index of the underlying row which may be indexed by an * SV2 or SV4. * - * @return + * @return index of the underlying row */ - int offset(); } Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314360423 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/DirectRowSet.java ## @@ -33,9 +32,10 @@ import org.apache.drill.exec.record.VectorContainer; import org.apache.drill.exec.record.selection.SelectionVector2; import org.apache.drill.exec.vector.accessor.convert.ColumnConversionFactory; -import org.apache.drill.test.rowSet.RowSet.ExtendableRowSet; -import org.apache.drill.test.rowSet.RowSetWriterImpl.WriterIndexImpl; +import org.apache.drill.exec.physical.rowSet.RowSet.ExtendableRowSet; +import org.apache.drill.exec.physical.rowSet.RowSetWriterImpl.WriterIndexImpl; +import java.util.HashSet; Review comment: Thanks, done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314365904 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetPrinter.java ## @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Thanks, replaced `PrintStream` usage with `StringBuilder`, renamed class to `RowSetStringBuilder` and renamed its methods. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314362960 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSet.java ## @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Thanks, done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314367486 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/RowSetReader.java ## @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Thanks, renamed to `setPosition()`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314359926 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractSingleRowSet.java ## @@ -15,15 +15,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314369366 ## File path: exec/java-exec/src/test/java/org/apache/drill/test/rowSet/RowSetBuilder.java ## @@ -20,12 +20,15 @@ import java.util.Set; import org.apache.drill.exec.memory.BufferAllocator; +import org.apache.drill.exec.physical.rowSet.DirectRowSet; +import org.apache.drill.exec.physical.rowSet.RowSet; +import org.apache.drill.exec.physical.rowSet.RowSetWriter; import org.apache.drill.exec.record.BatchSchema; import org.apache.drill.exec.record.metadata.MetadataUtils; import org.apache.drill.exec.record.metadata.TupleMetadata; import org.apache.drill.exec.vector.accessor.convert.ColumnConversionFactory; import org.apache.drill.shaded.guava.com.google.common.collect.Sets; -import org.apache.drill.test.rowSet.RowSet.SingleRowSet; +import org.apache.drill.exec.physical.rowSet.RowSet.SingleRowSet; Review comment: Thanks, done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314361974
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/DirectRowSet.java
##
@@ -132,7 +131,7 @@ public RowSetReader reader() {
@Override
public SingleRowSet toIndirect() {
-return new IndirectRowSet(this, Sets.newHashSet());
+return new IndirectRowSet(this, new HashSet<>());
Review comment:
Agree, replaced with `Collections.emptySet()`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder
vvysotskyi commented on a change in pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314359422 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractRowSet.java ## @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.drill.test.rowSet; +package org.apache.drill.exec.physical.rowSet; Review comment: Thanks, done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
