You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/03/08 09:38:30 UTC
[GitHub] [iotdb] zjx1230 opened a new pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
zjx1230 opened a new pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184
You can see the details in [JIRA](https://issues.apache.org/jira/browse/IOTDB-2602
).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822755018
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -66,14 +67,57 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
- // filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
- queryDataSet.decreaseAlreadyReturnedRowNum();
- i--;
- continue;
+ Set<Integer> withoutNullColumnsIndex = queryDataSet.getWithoutNullColumnsIndex();
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
+ || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
+ // if the current RowRecord doesn't satisfy, we should also decrease
+ // AlreadyReturnedRowNum
+ queryDataSet.decreaseAlreadyReturnedRowNum();
+ i--;
+ continue;
+ }
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
fix
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -66,14 +67,57 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
- // filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
- queryDataSet.decreaseAlreadyReturnedRowNum();
- i--;
- continue;
+ Set<Integer> withoutNullColumnsIndex = queryDataSet.getWithoutNullColumnsIndex();
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
+ || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
+ // if the current RowRecord doesn't satisfy, we should also decrease
+ // AlreadyReturnedRowNum
+ queryDataSet.decreaseAlreadyReturnedRowNum();
+ i--;
+ continue;
+ }
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822377787
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -127,6 +130,15 @@ public AlignByDeviceDataSet(
this.rawDataQueryPlan.setEnableRedirect(alignByDevicePlan.isEnableRedirect());
}
+ int index = 1; // start 1, because first is device name
+ if (withoutNullColumnsIndex.isEmpty()) {
Review comment:
We don't need this `if` here, `withoutNullColumnsIndex` must be empty here,
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -77,6 +78,7 @@
private String currentDevice;
private List<String> executeColumns;
private int pathsNum = 0;
+ private Set<String> withoutNullColumns;
Review comment:
We don't need to save this field in `AlignByDeviceDataSet`, you can make it as a local variable in its constrcutor.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
add some comments about `measurements` which indicating that it doesn't contain alias.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -87,6 +89,7 @@ public AlignByDeviceDataSet(
// adapting AlignByDevice query for new vector
super.columnNum = alignByDevicePlan.getMeasurements().size() + 1; // + 1 for 'device'
this.measurements = alignByDevicePlan.getMeasurements();
+ this.withoutNullColumns = alignByDevicePlan.getWithoutNullColumns();
Review comment:
add some comments about measurements which indicating that it doesn't contain alias.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -127,6 +130,15 @@ public AlignByDeviceDataSet(
this.rawDataQueryPlan.setEnableRedirect(alignByDevicePlan.isEnableRedirect());
}
+ int index = 1; // start 1, because first is device name
+ if (withoutNullColumnsIndex.isEmpty()) {
+ for (String measurement : this.measurements) {
Review comment:
this measurement doesn't contain alias, but withoutNullColumns already replaced with alias, so you should use `measurementInfoMap` to get the alias.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -53,7 +58,11 @@
private boolean ascending = true;
- private Map<String, Integer> pathToIndex = new HashMap<>();
+ private Map<String, Integer> pathToIndex = new HashMap<>(); // align By time
+ private Set<String> alignByDeviceWithoutNullValidSet = new HashSet<>(); // align By Device
Review comment:
`withoutNullValidSet` is unnecessary, remove it.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
add some comments about `withoutNullColumns` which indicating that it contains alias.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -641,6 +641,9 @@ protected QueryDataSet processDataQuery(QueryPlan queryPlan, QueryContext contex
queryDataSet.setRowOffset(queryPlan.getRowOffset());
queryDataSet.setWithoutAllNull(queryPlan.isWithoutAllNull());
queryDataSet.setWithoutAnyNull(queryPlan.isWithoutAnyNull());
+ if (!(queryPlan instanceof AlignByDevicePlan)) {
+ queryDataSet.setWithoutNullColumnsIndex(queryPlan.getWithoutNullColumnsIndex());
+ }
Review comment:
add comment to explain that `withoutNullColumnsIndex` in `AlignByDeviceDataSet` has already been set in its constructor.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -337,6 +347,7 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
alignByDevicePlan.setEnableTracing(enableTracing);
alignByDevicePlan.deduplicate(generator);
+ checkWithoutNullColumnValidAlignByDevice(alignByDevicePlan);
Review comment:
You fill `withoutNullColumns` of `AlignByDevicePlan` in `convertSpecialClauseValues` and fill `withoutNullValidSet` of `AlignByDevicePlan` in `alignByDevicePlan.deduplicate(generator)`, then do this check whether all without null columns exit in withoutNullValidSet.
Actually, you can see that `withoutNullValidSet` in `AlignByDevicePlan` is unnecessary, you can make it as local variable here, and pass it as a parameter to `checkWithoutNullColumnValidAlignByDevice`.
BTW, add some comments about where you init `withoutNullColumns` of `AlignByDevicePlan` above this check 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47225529)
Coverage increased (+0.04%) to 67.867% when pulling **341df81c1bb6dc31ca6d0d52db9ba3d3837bf075 on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r823264067
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822429401
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -53,7 +58,11 @@
private boolean ascending = true;
- private Map<String, Integer> pathToIndex = new HashMap<>();
+ private Map<String, Integer> pathToIndex = new HashMap<>(); // align By time
+ private Set<String> alignByDeviceWithoutNullValidSet = new HashSet<>(); // align By Device
Review comment:
`withoutNullValidSet` is unnecessary, remove it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822556753
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
+ List<Expression> resultExpressions = new ArrayList<>();
+ for (Expression expression : expressions) {
+ if (queryOperator.getAliasSet().contains(expression.getExpressionString())) {
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822693349
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/RawQueryDataSetWithoutValueFilter.java
##########
@@ -509,30 +509,53 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
return tsQueryDataSet;
}
- /** if any column in the row record is null, we filter it. */
+ // /** if any column in the row record is null, we filter it. */
+ /** if columns in the row record match the condition of null value filter, we filter it. */
private boolean filterRowRecord(int seriesNum, long minTime)
throws IOException, InterruptedException {
- boolean hasNull = false;
+ boolean hasNull = false, isAllNull = true;
for (int seriesIndex = 0; seriesIndex < seriesNum; seriesIndex++) {
+ if (!withoutNullColumnsIndex.isEmpty() && !withoutNullColumnsIndex.contains(seriesIndex)) {
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47247349)
Coverage increased (+0.03%) to 67.86% when pulling **877a8a0442c2903cfd2259eba7bbe5f603ce148c on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822563824
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -104,6 +120,64 @@ private void concatSelect(QueryOperator queryOperator) throws LogicalOptimizeExc
queryOperator.getSelectComponent().setResultColumns(resultColumns);
}
+ private void concatWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ List<PartialPath> prefixPaths = queryOperator.getFromComponent().getPrefixPaths();
+ // has without null columns
+ if (queryOperator.getSpecialClauseComponent() != null
+ && !queryOperator.getSpecialClauseComponent().getWithoutNullColumns().isEmpty()) {
+ List<Expression> withoutNullColumns = new ArrayList<>();
+ for (Expression expression :
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns()) {
+ concatWithoutNullColumns(
+ prefixPaths, expression, withoutNullColumns, queryOperator.getAliasSet());
+ }
+ queryOperator.getSpecialClauseComponent().setWithoutNullColumns(withoutNullColumns);
+ }
+ }
+
+ private void concatWithoutNullColumns(
+ List<PartialPath> prefixPaths,
+ Expression expression,
+ List<Expression> withoutNullColumns,
+ Set<String> aliasSet)
+ throws LogicalOptimizeException {
+ if (expression instanceof TimeSeriesOperand) {
+ TimeSeriesOperand timeSeriesOperand = (TimeSeriesOperand) expression;
+ if (timeSeriesOperand
+ .getPath()
+ .getFullPath()
+ .startsWith(SQLConstant.ROOT + ".")) { // start with "root." don't concat
+ if (((TimeSeriesOperand) expression).getPath().getNodeLength() == 1) { // no split
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822706532
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
fix
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r824515614
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AggregationPlan.java
##########
@@ -178,6 +178,10 @@ public String getColumnForReaderFromPath(PartialPath path, int pathIndex) {
public String getColumnForDisplay(String columnForReader, int pathIndex) {
String columnForDisplay = columnForReader;
if (isGroupByLevel()) {
+ if (resultColumns.get(pathIndex).hasAlias()) {
+ return resultColumns.get(pathIndex).getAlias();
+ }
Review comment:
Use `groupByLevelController.getAlias(aggregatePath)` below.
##########
File path: integration/src/test/java/org/apache/iotdb/db/integration/without_null_filter/IoTDBWithoutNullAllFilterIT.java
##########
@@ -0,0 +1,1973 @@
+/*
+ * 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.iotdb.db.integration.without_null_filter;
Review comment:
用驼峰命名法吧
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -40,9 +42,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
public abstract class QueryPlan extends PhysicalPlan {
+ public static final String WITHOUT_NULL_FILTER_ERROR_MESSAGE =
+ "The without null columns don't match the columns queried.If has alias, please use the alias.";
Review comment:
```suggestion
"The without null columns don't match the columns queried. If there is an alias, please use the alias.";
```
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -67,13 +67,15 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
// filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
+ if (queryDataSet.withoutNullFilter(
+ rowRecord, queryDataSet.isWithoutAllNull(), queryDataSet.isWithoutAnyNull())) {
Review comment:
`queryDataSet.isWithoutAllNull(), queryDataSet.isWithoutAnyNull()` is not needed.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -346,6 +363,21 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
return alignByDevicePlan;
}
+ private void checkWithoutNullColumnValid(AlignByDevicePlan alignByDevicePlan)
+ throws QueryProcessException {
+ if (specialClauseComponent != null) {
+ // meta consistence check
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ if (!alignByDevicePlan.isValidWithoutNullColumn(expression.getExpressionString())) {
+ throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
+ }
+ }
+ }
Review comment:
Honest to say, why don't you combine the check valid method and the calculate index method? They both need to traverse the WithoutNullColumns.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -58,6 +58,7 @@
private QueryContext context;
private IExpression expression;
+ // contains original measurement, no include alias
Review comment:
```suggestion
// contains only original measurement, alias not included
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r824538207
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -58,6 +58,7 @@
private QueryContext context;
private IExpression expression;
+ // contains original measurement, no include alias
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822552232
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -68,8 +72,20 @@ public Operator transform(Operator operator)
if (!optimizable(queryOperator)) {
return queryOperator;
}
+
+ // add aliasSet
+ Set<String> aliasSet = new HashSet<>();
+ for (ResultColumn resultColumn : queryOperator.getSelectComponent().getResultColumns()) {
+ if (resultColumn.hasAlias()) {
+ aliasSet.add(resultColumn.getAlias());
+ }
+ }
+ queryOperator.setAliasSet(aliasSet);
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47161408)
Coverage increased (+0.001%) to 67.827% when pulling **9f436249bb42243e4ab1798d7b19faf8c6a71d90 on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47271256)
Coverage increased (+0.08%) to 67.902% when pulling **779b3bfeb2cda425b01f9efbffb9faf7dabcd35b on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r823838683
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -356,8 +385,31 @@ private void checkDataTypeConsistency(TSDataType checkedDataType, MeasurementInf
}
}
- protected void convertSpecialClauseValues(QueryPlan queryPlan) {
+ protected void convertSpecialClauseValues(QueryPlan queryPlan) throws QueryProcessException {
if (specialClauseComponent != null) {
+ // record the without null column corresponding index if the plan is not AlignByDevicePlan
+ // otherwise, we simply add all without null column names into AlignByDevicePlan,
+ // and then let itself do the logic of conversion in
+ // `checkWithoutNullColumnValidAlignByDevice` of `QueryOperator.generateAlignByDevicePlan`
+ if (!queryPlan
+ .getPathToIndex()
+ .isEmpty()) { // align by device queryPlan.getPathToIndex() is empty
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ if (queryPlan.getPathToIndex().containsKey(expression.getExpressionString())) {
+ queryPlan.addWithoutNullColumnIndex(
+ queryPlan.getPathToIndex().get(expression.getExpressionString()));
+ } else {
+ throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
+ }
+ }
+ } else if (queryPlan instanceof AlignByDevicePlan) {
+ // add without null column
+ AlignByDevicePlan alignByDevicePlan = (AlignByDevicePlan) queryPlan;
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ alignByDevicePlan.addWithoutNullColumns(expression.getExpressionString());
+ }
+ }
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -641,6 +641,11 @@ protected QueryDataSet processDataQuery(QueryPlan queryPlan, QueryContext contex
queryDataSet.setRowOffset(queryPlan.getRowOffset());
queryDataSet.setWithoutAllNull(queryPlan.isWithoutAllNull());
queryDataSet.setWithoutAnyNull(queryPlan.isWithoutAnyNull());
+ // AlignByDevicePlan's withoutNullColumnsIndex has been set in the method
+ // `getAlignByDeviceDataSet` above
+ if (!(queryPlan instanceof AlignByDevicePlan)) {
+ queryDataSet.setWithoutNullColumnsIndex(queryPlan.getWithoutNullColumnsIndex());
+ }
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r824539147
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -40,9 +42,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
public abstract class QueryPlan extends PhysicalPlan {
+ public static final String WITHOUT_NULL_FILTER_ERROR_MESSAGE =
+ "The without null columns don't match the columns queried.If has alias, please use the alias.";
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822577137
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -171,6 +188,14 @@ public void setColumnNameToDatasetOutputIndex(String columnName, Integer index)
pathToIndex.put(columnName, index);
}
+ public void addAlignByDeviceValidWithoutNullColumn(String columnName) {
+ alignByDeviceWithoutNullValidSet.add(columnName);
+ }
+
+ public boolean isValidWithoutNullColumnAlignByDevice(String columnName) {
+ return alignByDeviceWithoutNullValidSet.contains(columnName);
+ }
+
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r823263066
##########
File path: docs/UserGuide/Query-Data/Without-Null.md
##########
@@ -21,17 +21,162 @@
# Null Value Filter
-In IoTDB, the `WITHOUT NULL` clause can be used to filter null values in the result set. There are two filtering strategies:
+In practical application, users may want to filter some rows with null values in the query result set. In IoTDB, the `WITHOUT NULL` clause can be used to filter null values in the result set. There are two filtering strategies: `WITHOUT NULL ANY`和`WITHOUT NULL ALL`. In addition, the `WITHOUT NULL` clause supports specifying the corresponding columns for filtering.
-1. IoTDB will join all the sensor value by its time, and if some sensors don't have values in that timestamp, we will fill it with null. In some analysis scenarios, we only need the row if all the columns of it have value.
+> WITHOUT NULL ANY: if one of the columns in the specified column set is null, the conditions are met for filtering.
+>
+> WITHOUT NULL ALL: if all columns in the specified column set are null, the conditions are met for filtering.
+
+## Don't specify columns
+
+> By default, it is effective for all columns in the result set. That is the specified column set includes all columns in the result set.
+
+1. In the following query, if any column of one row in the result set is null, the row will be filtered out. That is the result set obtained does not contain any null values.
```sql
select * from root.ln.** where time <= 2017-11-01T00:01:00 WITHOUT NULL ANY
```
-2. In group by query, we will fill null for any group by interval if the columns don't have values in that group by interval. However, if all columns in that group by interval are null, maybe users don't need that RowRecord, so we can use `WITHOUT NULL ALL` to filter that row.
+2. In the following query, if all columns of one row in the result set are null, the row will be filtered out.
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47271312)
Coverage increased (+0.06%) to 67.886% when pulling **779b3bfeb2cda425b01f9efbffb9faf7dabcd35b on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47274916)
Coverage increased (+0.06%) to 67.889% when pulling **4983d61eb3528a15974fe996e3e2d3f25f02d1d7 on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47301185)
Coverage increased (+0.07%) to 67.9% when pulling **9f2629f021c61a6b0c1fd03620d0d023bb7b0d71 on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822568552
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -356,14 +379,35 @@ private void checkDataTypeConsistency(TSDataType checkedDataType, MeasurementInf
}
}
- protected void convertSpecialClauseValues(QueryPlan queryPlan) {
+ protected void convertSpecialClauseValues(QueryPlan queryPlan) throws QueryProcessException {
if (specialClauseComponent != null) {
+ // meta consistence check
+ if (!queryPlan
+ .getPathToIndex()
+ .isEmpty()) { // align by device queryPlan.getPathToIndex() is empty
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ if (queryPlan.getPathToIndex().containsKey(expression.getExpressionString())) {
+ queryPlan.addWithoutNullColumnIndex(
+ queryPlan.getPathToIndex().get(expression.getExpressionString()));
+ } else {
+ throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
+ }
+ }
+ }
queryPlan.setWithoutAllNull(specialClauseComponent.isWithoutAllNull());
queryPlan.setWithoutAnyNull(specialClauseComponent.isWithoutAnyNull());
queryPlan.setRowLimit(specialClauseComponent.getRowLimit());
queryPlan.setRowOffset(specialClauseComponent.getRowOffset());
queryPlan.setAscending(specialClauseComponent.isAscending());
queryPlan.setAlignByTime(specialClauseComponent.isAlignByTime());
+
+ // add without null column
+ if (queryPlan instanceof AlignByDevicePlan) {
+ AlignByDevicePlan alignByDevicePlan = (AlignByDevicePlan) queryPlan;
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ alignByDevicePlan.addWithoutNullColumns(expression.getExpressionString());
+ }
+ }
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822782140
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -127,6 +130,15 @@ public AlignByDeviceDataSet(
this.rawDataQueryPlan.setEnableRedirect(alignByDevicePlan.isEnableRedirect());
}
+ int index = 1; // start 1, because first is device name
+ if (withoutNullColumnsIndex.isEmpty()) {
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822784601
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -337,6 +347,7 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
alignByDevicePlan.setEnableTracing(enableTracing);
alignByDevicePlan.deduplicate(generator);
+ checkWithoutNullColumnValidAlignByDevice(alignByDevicePlan);
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47249535)
Coverage increased (+0.04%) to 67.861% when pulling **e6890ed4d649acae1a0afa714115c82aa4cbf909 on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] Alima777 merged pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
Alima777 merged pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47276824)
Coverage increased (+0.05%) to 67.872% when pulling **aa0aa3fa8de97cdbab325af568941f29ee8a315c on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822552046
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AggregationPlan.java
##########
@@ -183,6 +183,9 @@ public String getColumnForDisplay(String columnForReader, int pathIndex) {
String aggregatePath =
groupByLevelController.getGroupedPath(
String.format("%s(%s)", functionName, path.getFullPath()));
+ if (resultColumns.get(pathIndex).hasAlias()) {
+ return resultColumns.get(pathIndex).getAlias();
+ }
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822561506
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
+ List<Expression> resultExpressions = new ArrayList<>();
+ for (Expression expression : expressions) {
+ if (queryOperator.getAliasSet().contains(expression.getExpressionString())) {
+ resultExpressions.add(expression);
+ continue;
+ }
+ expression.removeWildcards(withoutNullWildcardsRemover, filterExpressions);
+ }
+
+ // group by level, use groupedPathMap
+ GroupByLevelController groupByLevelController =
+ queryOperator.getSpecialClauseComponent().getGroupByLevelController();
+ if (groupByLevelController != null) {
+ for (Expression expression : filterExpressions) {
+ String groupedPath =
+ groupByLevelController.getGroupedPath(expression.getExpressionString());
+ if (groupedPath != null) {
+ try {
+ resultExpressions.add(new TimeSeriesOperand(new PartialPath(groupedPath)));
+ } catch (IllegalPathException e) {
+ throw new LogicalOptimizeException(e.getMessage());
+ }
+ } else {
+ resultExpressions.add(expression);
+ }
+ }
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822425324
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -337,6 +347,7 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
alignByDevicePlan.setEnableTracing(enableTracing);
alignByDevicePlan.deduplicate(generator);
+ checkWithoutNullColumnValidAlignByDevice(alignByDevicePlan);
Review comment:
You fill `withoutNullColumns` of `AlignByDevicePlan` in `convertSpecialClauseValues` and fill `withoutNullValidSet` of `AlignByDevicePlan` in `alignByDevicePlan.deduplicate(generator)`, then do this check whether all without null columns exit in withoutNullValidSet.
BTW, add some comments about where you init `withoutNullColumns` of `AlignByDevicePlan` above this check method.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -337,6 +347,7 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
alignByDevicePlan.setEnableTracing(enableTracing);
alignByDevicePlan.deduplicate(generator);
+ checkWithoutNullColumnValidAlignByDevice(alignByDevicePlan);
Review comment:
set `withoutNullValidSet` in `AlignByDevicePlan` to null here, friendly for gc.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822769101
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -53,7 +58,11 @@
private boolean ascending = true;
- private Map<String, Integer> pathToIndex = new HashMap<>();
+ private Map<String, Integer> pathToIndex = new HashMap<>(); // align By time
+ private Set<String> alignByDeviceWithoutNullValidSet = new HashSet<>(); // align By Device
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822772100
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47269999)
Coverage increased (+0.06%) to 67.888% when pulling **bde851af931b97faadf6808e40e85196039c0bdc on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r824547873
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -346,6 +363,21 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
return alignByDevicePlan;
}
+ private void checkWithoutNullColumnValid(AlignByDevicePlan alignByDevicePlan)
+ throws QueryProcessException {
+ if (specialClauseComponent != null) {
+ // meta consistence check
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ if (!alignByDevicePlan.isValidWithoutNullColumn(expression.getExpressionString())) {
+ throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
+ }
+ }
+ }
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -67,13 +67,15 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
// filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
+ if (queryDataSet.withoutNullFilter(
+ rowRecord, queryDataSet.isWithoutAllNull(), queryDataSet.isWithoutAnyNull())) {
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822692938
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/RawQueryDataSetWithoutValueFilter.java
##########
@@ -509,30 +509,53 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
return tsQueryDataSet;
}
- /** if any column in the row record is null, we filter it. */
+ // /** if any column in the row record is null, we filter it. */
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822561102
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822246480
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
+ List<Expression> resultExpressions = new ArrayList<>();
+ for (Expression expression : expressions) {
+ if (queryOperator.getAliasSet().contains(expression.getExpressionString())) {
Review comment:
Add comments here to explain why we need alias judgement again.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -68,8 +72,20 @@ public Operator transform(Operator operator)
if (!optimizable(queryOperator)) {
return queryOperator;
}
+
+ // add aliasSet
+ Set<String> aliasSet = new HashSet<>();
+ for (ResultColumn resultColumn : queryOperator.getSelectComponent().getResultColumns()) {
+ if (resultColumn.hasAlias()) {
+ aliasSet.add(resultColumn.getAlias());
+ }
+ }
+ queryOperator.setAliasSet(aliasSet);
Review comment:
Move it to `IoTDBSqlVisitor.parseSelectClause()` method.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -104,6 +120,64 @@ private void concatSelect(QueryOperator queryOperator) throws LogicalOptimizeExc
queryOperator.getSelectComponent().setResultColumns(resultColumns);
}
+ private void concatWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ List<PartialPath> prefixPaths = queryOperator.getFromComponent().getPrefixPaths();
+ // has without null columns
+ if (queryOperator.getSpecialClauseComponent() != null
+ && !queryOperator.getSpecialClauseComponent().getWithoutNullColumns().isEmpty()) {
+ List<Expression> withoutNullColumns = new ArrayList<>();
+ for (Expression expression :
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns()) {
+ concatWithoutNullColumns(
+ prefixPaths, expression, withoutNullColumns, queryOperator.getAliasSet());
+ }
+ queryOperator.getSpecialClauseComponent().setWithoutNullColumns(withoutNullColumns);
+ }
+ }
+
+ private void concatWithoutNullColumns(
+ List<PartialPath> prefixPaths,
+ Expression expression,
+ List<Expression> withoutNullColumns,
+ Set<String> aliasSet)
+ throws LogicalOptimizeException {
+ if (expression instanceof TimeSeriesOperand) {
+ TimeSeriesOperand timeSeriesOperand = (TimeSeriesOperand) expression;
+ if (timeSeriesOperand
+ .getPath()
+ .getFullPath()
+ .startsWith(SQLConstant.ROOT + ".")) { // start with "root." don't concat
+ if (((TimeSeriesOperand) expression).getPath().getNodeLength() == 1) { // no split
Review comment:
Add comments here to explain that full path which starts with `root.` won't be split in `ioTDBSqlVisitor.parseExpression` method, so we should split here by ourselves.
##########
File path: docs/UserGuide/Query-Data/Without-Null.md
##########
@@ -21,17 +21,162 @@
# Null Value Filter
-In IoTDB, the `WITHOUT NULL` clause can be used to filter null values in the result set. There are two filtering strategies:
+In practical application, users may want to filter some rows with null values in the query result set. In IoTDB, the `WITHOUT NULL` clause can be used to filter null values in the result set. There are two filtering strategies: `WITHOUT NULL ANY`和`WITHOUT NULL ALL`. In addition, the `WITHOUT NULL` clause supports specifying the corresponding columns for filtering.
-1. IoTDB will join all the sensor value by its time, and if some sensors don't have values in that timestamp, we will fill it with null. In some analysis scenarios, we only need the row if all the columns of it have value.
+> WITHOUT NULL ANY: if one of the columns in the specified column set is null, the conditions are met for filtering.
+>
+> WITHOUT NULL ALL: if all columns in the specified column set are null, the conditions are met for filtering.
+
+## Don't specify columns
+
+> By default, it is effective for all columns in the result set. That is the specified column set includes all columns in the result set.
+
+1. In the following query, if any column of one row in the result set is null, the row will be filtered out. That is the result set obtained does not contain any null values.
```sql
select * from root.ln.** where time <= 2017-11-01T00:01:00 WITHOUT NULL ANY
```
-2. In group by query, we will fill null for any group by interval if the columns don't have values in that group by interval. However, if all columns in that group by interval are null, maybe users don't need that RowRecord, so we can use `WITHOUT NULL ALL` to filter that row.
+2. In the following query, if all columns of one row in the result set are null, the row will be filtered out.
Review comment:
Actually, in raw query, there won't be a row whose fields are all null. So `without null all` only take effect in group by query. The previous sql case is not right, you can correct the sql instead of the description.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -53,7 +58,11 @@
private boolean ascending = true;
- private Map<String, Integer> pathToIndex = new HashMap<>();
+ private Map<String, Integer> pathToIndex = new HashMap<>(); // align By time
+ private Set<String> alignByDeviceWithoutNullValidSet = new HashSet<>(); // align By Device
Review comment:
Move it to AlignByDevicePlan.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/QueryPlan.java
##########
@@ -171,6 +188,14 @@ public void setColumnNameToDatasetOutputIndex(String columnName, Integer index)
pathToIndex.put(columnName, index);
}
+ public void addAlignByDeviceValidWithoutNullColumn(String columnName) {
+ alignByDeviceWithoutNullValidSet.add(columnName);
+ }
+
+ public boolean isValidWithoutNullColumnAlignByDevice(String columnName) {
+ return alignByDeviceWithoutNullValidSet.contains(columnName);
+ }
+
Review comment:
Move these two methods to AlignByDevicePlan and add some java docs for thses two methods.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
add some comments to expplain what difference it has with `withoutNullValidSet`
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/RawQueryDataSetWithoutValueFilter.java
##########
@@ -509,30 +509,53 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
return tsQueryDataSet;
}
- /** if any column in the row record is null, we filter it. */
+ // /** if any column in the row record is null, we filter it. */
Review comment:
```suggestion
```
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -66,14 +67,57 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
- // filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
- queryDataSet.decreaseAlreadyReturnedRowNum();
- i--;
- continue;
+ Set<Integer> withoutNullColumnsIndex = queryDataSet.getWithoutNullColumnsIndex();
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
+ || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
+ // if the current RowRecord doesn't satisfy, we should also decrease
+ // AlreadyReturnedRowNum
+ queryDataSet.decreaseAlreadyReturnedRowNum();
+ i--;
+ continue;
+ }
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
Change the loop sequence
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/RawQueryDataSetWithoutValueFilter.java
##########
@@ -509,30 +509,53 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
return tsQueryDataSet;
}
- /** if any column in the row record is null, we filter it. */
+ // /** if any column in the row record is null, we filter it. */
+ /** if columns in the row record match the condition of null value filter, we filter it. */
private boolean filterRowRecord(int seriesNum, long minTime)
throws IOException, InterruptedException {
- boolean hasNull = false;
+ boolean hasNull = false, isAllNull = true;
for (int seriesIndex = 0; seriesIndex < seriesNum; seriesIndex++) {
+ if (!withoutNullColumnsIndex.isEmpty() && !withoutNullColumnsIndex.contains(seriesIndex)) {
Review comment:
We should consider aligned path here.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -356,14 +379,35 @@ private void checkDataTypeConsistency(TSDataType checkedDataType, MeasurementInf
}
}
- protected void convertSpecialClauseValues(QueryPlan queryPlan) {
+ protected void convertSpecialClauseValues(QueryPlan queryPlan) throws QueryProcessException {
if (specialClauseComponent != null) {
+ // meta consistence check
+ if (!queryPlan
+ .getPathToIndex()
+ .isEmpty()) { // align by device queryPlan.getPathToIndex() is empty
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ if (queryPlan.getPathToIndex().containsKey(expression.getExpressionString())) {
+ queryPlan.addWithoutNullColumnIndex(
+ queryPlan.getPathToIndex().get(expression.getExpressionString()));
+ } else {
+ throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
+ }
+ }
+ }
queryPlan.setWithoutAllNull(specialClauseComponent.isWithoutAllNull());
queryPlan.setWithoutAnyNull(specialClauseComponent.isWithoutAnyNull());
queryPlan.setRowLimit(specialClauseComponent.getRowLimit());
queryPlan.setRowOffset(specialClauseComponent.getRowOffset());
queryPlan.setAscending(specialClauseComponent.isAscending());
queryPlan.setAlignByTime(specialClauseComponent.isAlignByTime());
+
+ // add without null column
+ if (queryPlan instanceof AlignByDevicePlan) {
+ AlignByDevicePlan alignByDevicePlan = (AlignByDevicePlan) queryPlan;
+ for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
+ alignByDevicePlan.addWithoutNullColumns(expression.getExpressionString());
+ }
+ }
Review comment:
```suggestion
// record the without null column corresponding index if the plan is not AlignByDevicePlan
// otherwise, we simply add all without null column names into AlignByDevicePlan, and then let itself do the logic of conversion in `checkWithoutNullColumnValidAlignByDevice` of `QueryOperator.generateAlignByDevicePlan`
if (!queryPlan
.getPathToIndex()
.isEmpty()) { // align by device queryPlan.getPathToIndex() is empty
for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
if (queryPlan.getPathToIndex().containsKey(expression.getExpressionString())) {
queryPlan.addWithoutNullColumnIndex(
queryPlan.getPathToIndex().get(expression.getExpressionString()));
} else { // check whether current without null column exists in select clause, if not throw exception
throw new QueryProcessException(QueryPlan.WITHOUT_NULL_FILTER_ERROR_MESSAGE);
}
}
} else if (queryPlan instanceof AlignByDevicePlan) {
AlignByDevicePlan alignByDevicePlan = (AlignByDevicePlan) queryPlan;
for (Expression expression : specialClauseComponent.getWithoutNullColumns()) {
alignByDevicePlan.addWithoutNullColumns(expression.getExpressionString());
}
}
queryPlan.setWithoutAllNull(specialClauseComponent.isWithoutAllNull());
queryPlan.setWithoutAnyNull(specialClauseComponent.isWithoutAnyNull());
queryPlan.setRowLimit(specialClauseComponent.getRowLimit());
queryPlan.setRowOffset(specialClauseComponent.getRowOffset());
queryPlan.setAscending(specialClauseComponent.isAscending());
queryPlan.setAlignByTime(specialClauseComponent.isAlignByTime());
}
```
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
change the loop sequence
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
Review comment:
Change the field name to `actualExpressions`, `filterExpressions` is a little bit ambiguous, and also add comments to explain the field.
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
Review comment:
extract this commone statement in `if` and `else` block.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AggregationPlan.java
##########
@@ -183,6 +183,9 @@ public String getColumnForDisplay(String columnForReader, int pathIndex) {
String aggregatePath =
groupByLevelController.getGroupedPath(
String.format("%s(%s)", functionName, path.getFullPath()));
+ if (resultColumns.get(pathIndex).hasAlias()) {
+ return resultColumns.get(pathIndex).getAlias();
+ }
Review comment:
move it to the first line of this fi block
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/strategy/optimizer/ConcatPathOptimizer.java
##########
@@ -137,6 +211,52 @@ private void removeWildcardsInSelectPaths(QueryOperator queryOperator)
}
}
+ private void removeWildcardsWithoutNullColumns(QueryOperator queryOperator)
+ throws LogicalOptimizeException {
+ if (queryOperator.getIndexType() != null) {
+ return;
+ }
+
+ if (queryOperator.getSpecialClauseComponent() == null) {
+ return;
+ }
+
+ List<Expression> expressions =
+ queryOperator.getSpecialClauseComponent().getWithoutNullColumns();
+ WildcardsRemover withoutNullWildcardsRemover = new WildcardsRemover(queryOperator);
+ List<Expression> filterExpressions = new ArrayList<>();
+ List<Expression> resultExpressions = new ArrayList<>();
+ for (Expression expression : expressions) {
+ if (queryOperator.getAliasSet().contains(expression.getExpressionString())) {
+ resultExpressions.add(expression);
+ continue;
+ }
+ expression.removeWildcards(withoutNullWildcardsRemover, filterExpressions);
+ }
+
+ // group by level, use groupedPathMap
+ GroupByLevelController groupByLevelController =
+ queryOperator.getSpecialClauseComponent().getGroupByLevelController();
+ if (groupByLevelController != null) {
+ for (Expression expression : filterExpressions) {
+ String groupedPath =
+ groupByLevelController.getGroupedPath(expression.getExpressionString());
+ if (groupedPath != null) {
+ try {
+ resultExpressions.add(new TimeSeriesOperand(new PartialPath(groupedPath)));
+ } catch (IllegalPathException e) {
+ throw new LogicalOptimizeException(e.getMessage());
+ }
+ } else {
+ resultExpressions.add(expression);
+ }
+ }
Review comment:
Add a IT for this case, like:
`select sum(d1.s1), sum(d2.s1) from root.sg1 group by level = 1 without null any(sum(d1.s1), sum(d2.s1));`
It will only contains one column: `sum(root.sg1.*.s1)`.
##########
File path: tsfile/src/main/java/org/apache/iotdb/tsfile/read/query/dataset/QueryDataSet.java
##########
@@ -107,17 +112,59 @@ protected void initQueryDataSetFields(
}
}
+ public Set<Integer> getWithoutNullColumnsIndex() {
+ return withoutNullColumnsIndex;
+ }
+
+ public void setWithoutNullColumnsIndex(Set<Integer> withoutNullColumnsIndex) {
+ this.withoutNullColumnsIndex = withoutNullColumnsIndex;
+ }
+
public boolean hasNext() throws IOException {
// proceed to the OFFSET row by skipping rows
while (rowOffset > 0) {
if (hasNextWithoutConstraint()) {
RowRecord rowRecord = nextWithoutConstraint(); // DO NOT use next()
- // filter rows whose columns are null according to the rule
- if ((withoutAllNull && rowRecord.isAllNull())
- || (withoutAnyNull && rowRecord.hasNullField())) {
- continue;
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((withoutAllNull && rowRecord.isAllNull())
+ || (withoutAnyNull && rowRecord.hasNullField())) {
+ continue;
+ }
+ rowOffset--;
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
change the loop sequence
##########
File path: server/src/main/java/org/apache/iotdb/db/utils/QueryDataSetUtils.java
##########
@@ -66,14 +67,57 @@ public static TSQueryDataSet convertQueryDataSetByFetchSize(
for (int i = 0; i < fetchSize; i++) {
if (queryDataSet.hasNext()) {
RowRecord rowRecord = queryDataSet.next();
- // filter rows whose columns are null according to the rule
- if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
- || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
- // if the current RowRecord doesn't satisfy, we should also decrease AlreadyReturnedRowNum
- queryDataSet.decreaseAlreadyReturnedRowNum();
- i--;
- continue;
+ Set<Integer> withoutNullColumnsIndex = queryDataSet.getWithoutNullColumnsIndex();
+ if (withoutNullColumnsIndex.isEmpty()) {
+ // filter rows whose columns are null according to the rule
+ if ((queryDataSet.isWithoutAllNull() && rowRecord.isAllNull())
+ || (queryDataSet.isWithoutAnyNull() && rowRecord.hasNullField())) {
+ // if the current RowRecord doesn't satisfy, we should also decrease
+ // AlreadyReturnedRowNum
+ queryDataSet.decreaseAlreadyReturnedRowNum();
+ i--;
+ continue;
+ }
+ } else {
+ boolean anyNullFlag = false, allNullFlag = true;
+ int index = 0;
+ for (Field field : rowRecord.getFields()) {
+ if (!withoutNullColumnsIndex.contains(index)) {
Review comment:
`withoutNullColumnsIndex` should not be very large, but the number of rowRecord's fields may be large. Exchange the loop sequence
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/UDTFAlignByTimeDataSet.java
##########
@@ -132,7 +137,10 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
// Here we get a timestamp first and then construct the row column by column.
// We don't record this row when nullFieldsCnt > 0 and withoutAnyNull == true
// or nullFieldsCnt == columnNum and withoutAllNull == true
Review comment:
change the comments to current meaning
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822699257
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/UDTFAlignByTimeDataSet.java
##########
@@ -132,7 +137,10 @@ public TSQueryDataSet fillBuffer(int fetchSize, WatermarkEncoder encoder)
// Here we get a timestamp first and then construct the row column by column.
// We don't record this row when nullFieldsCnt > 0 and withoutAnyNull == true
// or nullFieldsCnt == columnNum and withoutAllNull == true
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r824560185
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AggregationPlan.java
##########
@@ -178,6 +178,10 @@ public String getColumnForReaderFromPath(PartialPath path, int pathIndex) {
public String getColumnForDisplay(String columnForReader, int pathIndex) {
String columnForDisplay = columnForReader;
if (isGroupByLevel()) {
+ if (resultColumns.get(pathIndex).hasAlias()) {
+ return resultColumns.get(pathIndex).getAlias();
+ }
Review comment:
fix
##########
File path: integration/src/test/java/org/apache/iotdb/db/integration/without_null_filter/IoTDBWithoutNullAllFilterIT.java
##########
@@ -0,0 +1,1973 @@
+/*
+ * 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.iotdb.db.integration.without_null_filter;
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#issuecomment-1061646712
[](https://coveralls.io/builds/47272505)
Coverage increased (+0.06%) to 67.886% when pulling **779b3bfeb2cda425b01f9efbffb9faf7dabcd35b on zjx1230:feature_iotdb_2602** into **5286d42d5bc5e2befcf3a254163e7c0d3394d286 on apache:master**.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822768365
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -127,6 +130,15 @@ public AlignByDeviceDataSet(
this.rawDataQueryPlan.setEnableRedirect(alignByDevicePlan.isEnableRedirect());
}
+ int index = 1; // start 1, because first is device name
+ if (withoutNullColumnsIndex.isEmpty()) {
+ for (String measurement : this.measurements) {
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AlignByDevicePlan.java
##########
@@ -48,6 +48,7 @@
// to record result measurement columns, e.g. temperature, status, speed
private List<String> measurements;
+ private Set<String> withoutNullColumns = new HashSet<>();
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/logical/crud/QueryOperator.java
##########
@@ -337,6 +347,7 @@ protected AlignByDevicePlan generateAlignByDevicePlan(PhysicalGenerator generato
alignByDevicePlan.setEnableTracing(enableTracing);
alignByDevicePlan.deduplicate(generator);
+ checkWithoutNullColumnValidAlignByDevice(alignByDevicePlan);
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -87,6 +89,7 @@ public AlignByDeviceDataSet(
// adapting AlignByDevice query for new vector
super.columnNum = alignByDevicePlan.getMeasurements().size() + 1; // + 1 for 'device'
this.measurements = alignByDevicePlan.getMeasurements();
+ this.withoutNullColumns = alignByDevicePlan.getWithoutNullColumns();
Review comment:
fix
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -77,6 +78,7 @@
private String currentDevice;
private List<String> executeColumns;
private int pathsNum = 0;
+ private Set<String> withoutNullColumns;
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] zjx1230 commented on a change in pull request #5184: [IOTDB-2602] Add the new feature of the null value filter to support filtering based on partial columns
Posted by GitBox <gi...@apache.org>.
zjx1230 commented on a change in pull request #5184:
URL: https://github.com/apache/iotdb/pull/5184#discussion_r822784124
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -641,6 +641,9 @@ protected QueryDataSet processDataQuery(QueryPlan queryPlan, QueryContext contex
queryDataSet.setRowOffset(queryPlan.getRowOffset());
queryDataSet.setWithoutAllNull(queryPlan.isWithoutAllNull());
queryDataSet.setWithoutAnyNull(queryPlan.isWithoutAnyNull());
+ if (!(queryPlan instanceof AlignByDevicePlan)) {
+ queryDataSet.setWithoutNullColumnsIndex(queryPlan.getWithoutNullColumnsIndex());
+ }
Review comment:
fix
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org