You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/03/02 16:18:00 UTC
[GitHub] [hive] pgaref opened a new pull request #931: HIVE-22959 Expose
FilterContext as part of Hive storage-api
pgaref opened a new pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pgaref commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389067478
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/io/filter/TestFilterContext.java
##########
@@ -0,0 +1,107 @@
+package org.apache.hadoop.hive.ql.io.filter;
Review comment:
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] t3rmin4t0r commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389020914
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
Review comment:
Add extra assert comment (assuming we'll be looking at the qtest log or something)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] t3rmin4t0r commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389021429
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/io/filter/TestFilterContext.java
##########
@@ -0,0 +1,107 @@
+package org.apache.hadoop.hive.ql.io.filter;
Review comment:
ASF license nit
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pgaref commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389068224
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
+ }
+
+ /**
+ * Copy context variables from the a given FilterContext.
+ * Always does a deep copy of the data.
+ *
+ * @param other FilterContext to copy from
+ */
+ public void copyFilterContextFrom(MutableFilterContext other) {
+ // assert if copying into self
Review comment:
Makes sense, added an extra condition 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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] t3rmin4t0r commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389029685
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
+ }
+
+ /**
+ * Copy context variables from the a given FilterContext.
+ * Always does a deep copy of the data.
+ *
+ * @param other FilterContext to copy from
+ */
+ public void copyFilterContextFrom(MutableFilterContext other) {
+ // assert if copying into self
Review comment:
Needs to be an if(), because if this == other, then we overwrite instance variables in assignment 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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pgaref commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389068042
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
Review comment:
Added a separate validate method that is checked with an assertion (the idea here is to use it only as part of testing)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] t3rmin4t0r commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389029434
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
Review comment:
Also move it to a different method so that the profiler will show it as a separate cost
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] t3rmin4t0r commented on a change in pull request #931:
HIVE-22959 Expose FilterContext as part of Hive storage-api
Posted by GitBox <gi...@apache.org>.
t3rmin4t0r commented on a change in pull request #931: HIVE-22959 Expose FilterContext as part of Hive storage-api
URL: https://github.com/apache/hive/pull/931#discussion_r389020620
##########
File path: storage-api/src/java/org/apache/hadoop/hive/ql/io/filter/MutableFilterContext.java
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.hadoop.hive.ql.io.filter;
+
+import java.util.Arrays;
+
+/**
+ * A representation of a Filter applied on the rows of a VectorizedRowBatch
+ * {@link org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch}.
+ *
+ * Each FilterContext consists of an array with the ids (int) of rows that are selected by the
+ * filter, an integer representing the number of selected rows, and a boolean showing if the filter
+ * actually selected any rows.
+ *
+ */
+public class MutableFilterContext extends FilterContext {
+
+ /**
+ * Set context with the given values by reference
+ *
+ * @param isSelectedInUse if the filter is applied
+ * @param selected an array of the selected rows
+ * @param selectedSize the number of the selected rows
+ */
+ public void setFilterContext(boolean isSelectedInUse, int[] selected, int selectedSize) {
+ this.currBatchIsSelectedInUse = isSelectedInUse;
+ this.currBatchSelected = selected;
+ this.currBatchSelectedSize = selectedSize;
+ // Avoid selected.length < selectedSize since we can borrow a larger array for selected
+ // debug loop for checking if selected is in order without duplicates (i.e [1,1,1] is illegal)
+ for (int i = 0; i < selectedSize-1; i++)
+ assert selected[i] < selected[i+1];
+ }
+
+ /**
+ * Copy context variables from the a given FilterContext.
+ * Always does a deep copy of the data.
+ *
+ * @param other FilterContext to copy from
+ */
+ public void copyFilterContextFrom(MutableFilterContext other) {
+ // assert if copying into self
Review comment:
We don't run with -ea in prod, so this is a bit of a "won't fail in prod, but will hit in testing"
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org