[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16911166#comment-16911166
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909883#comment-16909883
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314967436
##
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:
Thanks for the explanation, makes sense.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909882#comment-16909882
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314967421
##
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:
Thanks for trying to fix this. Doing the change as another PR sounds good.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909727#comment-16909727 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-522241693 I have reverted breaking change, so for now, all tests passed. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909676#comment-16909676
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909355#comment-16909355 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-522131852 The latest change caused tests failures. It is appeared because with new BatchSchemaBuilder TupleMetadata was used more frequently and some code does not set zero scale and precision, but PrimitiveColumnMetadata does. I'll let you know when this issue is fixed. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909298#comment-16909298
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909296#comment-16909296
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909299#comment-16909299
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909300#comment-16909300
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909297#comment-16909297
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909295#comment-16909295
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909247#comment-16909247
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314816402
##
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:
Ah! But I don't want to do the cast; that is the purpose of this method. I
want:
```
...addRow(mapValue(10, "foo"), 20); // convenience
...addRow(new Object[] {10, "foo"}, 20); // low-level
...
...addSingleCol(mapValue(10, "foo"); // convenience
... addSingleCol(new Object[] {10, "foo"}); // low-level
```
Unless we wrap that single column in another array, the second form is the
same as:
```
... addRow(new Object[] {10, "foo"});
```
Which will be interpreted as a set of two columns, not as a single column
with a map-value.
I could be wrong, and I'd have to test actual code to be sure. In fact, I'll
do that test this weekend.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909236#comment-16909236 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-522085882 @arina-ielchiieva, could be my misunderstanding, though I do recall making the change... Point is, both forms are perfectly valid Java; it is just a preference. To avoid misunderstandings we might want to add to our formatting rules. Again, just to be clear, I have no issue with whatever rules we decide on; but let's decide on a common set to avoid ping-pong format changes as each of us touches the code. As it turns out, Charles started a discussion on the dev list. Perhaps you can offer there the format rules you've found to work, such as `int[] foo;` instead of `int foo[];`, classes-at-bottom, no single-line method declarations, preferred import sort order and so on. Might also include preferences for Java over Guava methods for common operations. That way, we can all follow the best practices you've discovered. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909206#comment-16909206
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909191#comment-16909191 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-522068340 @paul-rogers > I also recall that Arina once suggested removing an anonymous inner class in favor of a declared class. I doubt this is true :) Maybe thrown out of the context though... 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909174#comment-16909174
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314787032
##
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:
Simpler: `addNullable("eventDate", MinorType.TIMESTAMP)`
Import `MinorType`, use the `addNullable` method rather than the more
verbose form of using the `DataMode`. That is, express intent, not
implementation.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909178#comment-16909178
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314789387
##
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:
As long as you are touching this code, please remove the do-nothing
initializers:
```
private int recordCount;
private int batchCount;
private QueryId queryId;
```
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909175#comment-16909175 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314786038 ## 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 separate from operations on that data. + * SingleRowSet (abstract) Review comment: Thanks! 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909172#comment-16909172
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314783903
##
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:
I can live with this, though the nested-classes at top is a more common
pattern in the Java community. The general rule is that constants and nested
classes at top, then fields, then methods. Original Java formatters tended to
put fields at the bottom. Some projects mix nested classes with methods: the
class is defined adjacent to the method that uses it.
I also recall that Arina once suggested removing an anonymous inner class in
favor of a declared class. This is, again, a style issue since anonymous inner
classes are a very common style, especially in UI and functional programming.
In general, we need to establish rules for the project, else each person
that touches the file will reformat code to their preferred style. We should
discuss those rules on the dev list before we start enforcing them.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909173#comment-16909173
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314787920
##
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:
This is *not* a simplification; it exposes too much detail. Better:
```
RowSetFormatter.print(rowSet);
```
Of course, if we go this route, we might as well leave the `print()` method
in `RowSet` and have it call the above code.
Just to be clear, the purpose of `print()` is to help us developers
visualize what's going on in Drill; it is not for production use. This is
important because, if you can't visualize a batch, you'll make many mistakes
because you can't tell what's what.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909177#comment-16909177
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314785834
##
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:
The class name is still "RowSetStringBuilder" Better to rename it to, say,
"RowSetFormatter". The constructor should take both a row set and a writer. We
can then provide static methods for the two most common cases: print to stdout
(which I use all the time when creating tests) and to string (which you seem to
prefer):
```
public static void print(RowSet rowSet) {
new RowSetFormatter(rowSet, System.out).write();
}
public static String toString(RowSet rowSet) {
StringWriter out = new StringWriter();
new RowSetFormatter(rowSet, out).write();
return out.toString();
}
```
The constructor allows other options, such as writing to a file. Sadly, it
won't work to write to a log; we'd have to materialize to a string...
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909176#comment-16909176
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314788921
##
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:
Another change you might consider is renaming either this, or the other
`SchemaBuilder`. Since this is the one we want to use moving forward, maybe
rename the other to `BatchSchemaBuilder` so that future folks don't have to
figure out the correct import when the reference the 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16909143#comment-16909143
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314780985
##
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:
While this would seem to be true, it won't work for the case of a map:
addRow(new Object[]{"foo", 10})
Is the above two column values, or a single column that happens to be an
object?
There is actually no need to debate this, however. Without this feature,
some unit tests will fail. (I know that because I added this method directly in
response to those test failures...)
Still, if you can get those tests to pass, even with the map value
ambiguity, then my concern will be addressed.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908984#comment-16908984
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908979#comment-16908979 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-521974104 Overall, LGTM. Let's wait for @paul-rogers approval. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908974#comment-16908974
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314672905
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -99,6 +101,13 @@ public DirectRowSet next() {
}
}
+ public void printAll() {
Review comment:
Not sure we should allow printing into stout in the code. When code was for
tests, it was ok but since we moved it, I think we should consider passing the
stream instead.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908972#comment-16908972
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314672905
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -99,6 +101,13 @@ public DirectRowSet next() {
}
}
+ public void printAll() {
Review comment:
Not sure we should allow printing into stout in the code. When code was for
tests, it was ok but since we moved it, I think we should consider passing the
stream instead.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908967#comment-16908967
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314672905
##
File path:
exec/java-exec/src/test/java/org/apache/drill/test/QueryRowSetIterator.java
##
@@ -99,6 +101,13 @@ public DirectRowSet next() {
}
}
+ public void printAll() {
Review comment:
No sure we should allow printing into stout in the code. When code was for
tests, it was ok but since we moved it, I think we should consider passing the
stream instead.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908958#comment-16908958
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314670539
##
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 think we should enforce such checkstyle. Taking into account that most of
IDE can compress such methods automatically plus many other projects follow the
same pattern. Personally, I was following such code style as well. @vvysotskyi
could you please create Jira?
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908956#comment-16908956
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314669714
##
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:
Agree on moving nested classes to the bottom since it makes code mode
readable, first you see all code that belongs to the class and then nested
classes.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908953#comment-16908953 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-521967080 @paul-rogers, I have reverted debatable formatting changes. Let's postpone this discussion until we decide to add some additional checkstyle rules. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908947#comment-16908947
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908926#comment-16908926 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908919#comment-16908919 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908924#comment-16908924 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908927#comment-16908927
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908922#comment-16908922
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908921#comment-16908921
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908920#comment-16908920
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908925#comment-16908925
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908923#comment-16908923
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908918#comment-16908918
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908917#comment-16908917
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908362#comment-16908362 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-521708575 @vvysotskyi, thanks for making this change. It will allow certain other tasks to be much easier. The first observation is that it would be best not to put the RowSet and ResultSetLoader stuff in the same package: they represent two independent systems and our package naming should make that clear. Perhaps 1) Rename the existing physical.rowSet to physical.resultSet. 2) Move the RowSet classes to a new physical.rowSet. The difference, so we're clear, is that RowSet works on a single batch, while the ResultSet works across a stream of batches. If operators, say, want to use the row set stuff to read batches, then we need a ResultSetReader that automagically handles things like schema changes across batches, etc. To be very clear: "row set" -- A collection of rows. Typically called a "batch" in Drill. "result set" -- A collection of rows for an entire query. Typically consists of multiple (related) batches. I used the term "row set" only because the classes would otherwise get lost with all the existing "Batch" classes. And, we don't have a good name for the result set. I borrowed this name from SQL. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908300#comment-16908300 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-521715916 Two general comments. First, not sure why we needed to convert compact single-line methods (a long-time Drill standard) into four-line methods. Perhaps this is the result of an overly-helpful code formatter? Second, looks like a bunch of nested helper classes were moved elsewhere. Not sure this is helpful as it clutters the code with small files used in exactly one place. The nested classes (a perfectly normal Java practice) makes clear that these are, in fact, helpers. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908293#comment-16908293
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314398293
##
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:
The problem with this approach is that row sets can be large: creating a
string to print causes memory pressure.
Consider creating a row set builder that takes a stream (or Writer). The
Writer can be a String writer. That way, when used with a large data set, the
writer can be to stdout, or whatever.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908289#comment-16908289
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314395723
##
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:
Why change formatting? Drill has long used single-line methods for
conciseness. Is this an artifact of some code formatter? I didn't see
discussion on the dev list about changing our coding standards...
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908297#comment-16908297
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314400808
##
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:
WriterEvents is meant to be entirely internal; it should not be part of the
API. WriterEvents is the internal implementation of the public API described
here.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908292#comment-16908292
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314396758
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/AbstractSingleRowSet.java
##
@@ -66,4 +57,14 @@ public long size() {
protected RowSetReader buildReader(ReaderIndex rowIndex) {
return new RowSetReaderBuilder().buildReader(this, rowIndex);
}
+
+ public static class RowSetReaderBuilder extends BaseReaderBuilder {
Review comment:
See note above.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908296#comment-16908296
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314397262
##
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:
See note above about cluttering the project with many small classes used in
only one location.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908288#comment-16908288 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314395350 ## 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: extensible 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908299#comment-16908299
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314401892
##
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:
Please either leave this, or move it to a utility class. This turns out to
be a very handy way to validate or debug a unit test: add a call while working
on the test to visualize the results, remove the call when 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908291#comment-16908291
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314397062
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/rowSet/DirectRowSet.java
##
@@ -45,21 +45,10 @@
public class DirectRowSet extends AbstractSingleRowSet implements
ExtendableRowSet {
- public static class RowSetWriterBuilder extends BaseWriterBuilder {
-
-public RowSetWriterBuilder(ColumnConversionFactory conversionFactory) {
- super(conversionFactory);
-}
-
-public RowSetWriter buildWriter(DirectRowSet rowSet) {
- WriterIndexImpl index = new WriterIndexImpl();
- TupleMetadata schema = rowSet.schema();
- RowSetWriterImpl writer = new RowSetWriterImpl(rowSet, schema, index,
- buildContainerChildren(rowSet.container(),
- new MetadataRetrieval(schema)));
- return writer;
-}
- }
+ /**
+ * Initial row count, used for preliminary memory allocation.
+ */
+ public static final int INITIAL_ROW_COUNT = 10;
Review comment:
See note above about removal of nested 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908290#comment-16908290
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314396624
##
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:
Why remove nested classes? These are perfectly normal part of Java that
allows small, single-purpose classes to be associated with the one and only
class that uses them. Again, if we want to change the Drill style guidelines,
let's have the discussion on dev list first.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908294#comment-16908294
]
ASF GitHub Bot commented on DRILL-7350:
---
paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related
classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314398737
##
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:
Won't work. Must be an array. The idea is that addRow() takes an array of
columns. But, if the row has a single column, and that one column is repeated,
the Java code is ambiguous. The original (correct) implementation handles this
case.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908298#comment-16908298 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314400385 ## 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 separate from operations on that data. + * SingleRowSet (abstract) Review comment: See general comment. Please do not combine the RowSet and ResultSetLoader classes into a single package: they present very different models used for different purposes. Yes, it is confusing because there is no ResultSetReader at present, but there should be to handle schema changes, etc. across batches. RowSet handles only a single row set such as in tests. It was deliberately designed to NOT be ResultSetReader solution. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908295#comment-16908295 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314399600 ## 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 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: The SV is not writable within a RowSet. Changing the SV will mess up indexing. The way to use that is to have a DirectRowSet on the original batch. Create the SV. Then, use the DirectRowSet and the SV to create an indirect row set. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908269#comment-16908269 ] ASF GitHub Bot commented on DRILL-7350: --- paul-rogers commented on issue #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#issuecomment-521708575 @vvysotskyi, thanks for making this change. It will allow certain other tasks to be much easier. The first observation is that it would be best not to put the RowSet and ResultSetLoader stuff in the same package: they represent two independent systems and our package naming should make that clear. Perhaps 1) Rename the existing physical.rowSet to physical.resultSet. 2) Move the RowSet classes to a new physical.rowSet. The difference, so we're clear, is that RowSet works on a single batch, while the ResultSet works across a stream of batches. If operators, say, want to use the row set stuff to read batches, then we need a ResultSetReader that automagically handles things like schema changes across batches, etc. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908255#comment-16908255
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908256#comment-16908256
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908253#comment-16908253
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908257#comment-16908257 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908254#comment-16908254
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908235#comment-16908235
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314378996
##
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:
final?
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908238#comment-16908238 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314382345 ## 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: It would be nice to add an example of the output. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908236#comment-16908236
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314382220
##
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:
Same here?
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908239#comment-16908239
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314378880
##
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:
Do we need default here?
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908237#comment-16908237
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314382035
##
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:
Why this is public and 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908216#comment-16908216 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908219#comment-16908219 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908214#comment-16908214 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908212#comment-16908212 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908221#comment-16908221 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908218#comment-16908218 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908217#comment-16908217
]
ASF GitHub Bot commented on DRILL-7350:
---
vvysotskyi commented on 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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908213#comment-16908213 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908220#comment-16908220 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908215#comment-16908215 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908135#comment-16908135
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314321216
##
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:
1. Remove `protected SchemaChangeCallBack callBack = new
SchemaChangeCallBack();`
2. Better error message: `public long size() {
throw new UnsupportedOperationException("getSize");
}`
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908121#comment-16908121 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314319626 ## 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: New line. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908128#comment-16908128 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314323618 ## 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: Replace usage of `Sets.newHashSet()` 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908123#comment-16908123
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314321216
##
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:
1. Remove `protected SchemaChangeCallBack callBack = new
SchemaChangeCallBack();`
2.Better error message: `public long size() {
throw new UnsupportedOperationException("getSize");
}`
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908126#comment-16908126 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314320356 ## 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: Please rename method: `void setPosn(int index);` 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908120#comment-16908120 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314319854 ## 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: Make sure Printer returns strings instead of sout 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908129#comment-16908129 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314324678 ## 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: 1. Capacity here can use constant we introduce for `10`. 2. Also can be considered to be moved. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908125#comment-16908125
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314322007
##
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:
`public RowSetWriter writer() {
return writer(10);
}`
Create a constant instead with Java-doc.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908122#comment-16908122
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314324081
##
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:
1. Remove public: `public interface SingleRowSet extends RowSet {`
2. Please try to fix all java doc errors.
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908132#comment-16908132 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314319854 ## 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: Make sure Printer returns strings instead of sout Maybe rename it to other name since now it won't print. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[
https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908127#comment-16908127
]
ASF GitHub Bot commented on DRILL-7350:
---
arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet
related classes from test folder
URL: https://github.com/apache/drill/pull/1843#discussion_r314323421
##
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:
Should be immutable?
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
> Move RowSet related classes from test folder
>
>
> Key: DRILL-7350
> URL: https://issues.apache.org/jira/browse/DRILL-7350
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.17.0
>
>
> Move RowSet related classes from test folder to main to be able to use them
> for Metastore.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908124#comment-16908124 ] ASF GitHub Bot commented on DRILL-7350: --- arina-ielchiieva commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843#discussion_r314321490 ## 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: Constructors can be protected. 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (DRILL-7350) Move RowSet related classes from test folder
[ https://issues.apache.org/jira/browse/DRILL-7350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16908095#comment-16908095 ] ASF GitHub Bot commented on DRILL-7350: --- vvysotskyi commented on pull request #1843: DRILL-7350: Move RowSet related classes from test folder URL: https://github.com/apache/drill/pull/1843 - Moved row-set related classes from test folder into main and changed package from `org.apache.drill.test.rowSet` to `org.apache.drill.exec.physical.rowSet` where other row-set related classes were placed. - Updated package info for both packages. For details please see [DRILL-7350](https://issues.apache.org/jira/browse/DRILL-7350). 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 > Move RowSet related classes from test folder > > > Key: DRILL-7350 > URL: https://issues.apache.org/jira/browse/DRILL-7350 > Project: Apache Drill > Issue Type: Task >Reporter: Volodymyr Vysotskyi >Assignee: Volodymyr Vysotskyi >Priority: Major > Fix For: 1.17.0 > > > Move RowSet related classes from test folder to main to be able to use them > for Metastore. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
