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/12/30 10:09:09 UTC
[GitHub] [iotdb] JackieTien97 opened a new pull request, #8683: Fix bug in last query
JackieTien97 opened a new pull request, #8683:
URL: https://github.com/apache/iotdb/pull/8683
While there exist multi data regions for one series slot, we will add LastMergeSortNode to merge all last query result from different data regions, it will select max time for the same time-series.
However, if so, we still need to add default order by parameter for each LastQueryNode in each DataRegion and sort its children by timeseries asc, because `LastQueryMergeOperator` will assume that all its children returning TsBlock has already been sorted.
--
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] lancelly commented on a diff in pull request #8683: Fix bug in last query
Posted by GitBox <gi...@apache.org>.
lancelly commented on code in PR #8683:
URL: https://github.com/apache/iotdb/pull/8683#discussion_r1059731038
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/distribution/SourceRewriter.java:
##########
@@ -525,13 +528,46 @@ public List<PlanNode> visitLastQuery(LastQueryNode node, DistributionPlanContext
PlanNode root = processRawMultiChildNode(node, context);
if (context.queryMultiRegion) {
PlanNode newRoot = genLastQueryRootNode(node, context);
+ // add sort op for each if we add LastQueryMergeNode as root
+ if (newRoot instanceof LastQueryMergeNode && node.getMergeOrderParameter().isEmpty()) {
+ OrderByParameter orderByParameter =
+ new OrderByParameter(
+ Collections.singletonList(new SortItem(SortKey.TIMESERIES, Ordering.ASC)));
+ addSortForEachLastQueryNode(root, orderByParameter);
+ }
root.getChildren().forEach(newRoot::addChild);
return Collections.singletonList(newRoot);
} else {
return Collections.singletonList(root);
}
}
+ private void addSortForEachLastQueryNode(PlanNode root, OrderByParameter orderByParameter) {
+ if (root instanceof LastQueryNode) {
+ LastQueryNode lastQueryNode = (LastQueryNode) root;
+ lastQueryNode.setMergeOrderParameter(orderByParameter);
+ // sort children node
+ lastQueryNode.setChildren(
+ lastQueryNode.getChildren().stream()
+ .sorted(
+ Comparator.comparing(
+ child -> {
+ String fullPath = "";
+ if (child instanceof LastQueryScanNode) {
+ fullPath = ((LastQueryScanNode) child).getSeriesPath().getFullPath();
+ } else if (child instanceof AlignedLastQueryScanNode) {
+ fullPath = ((AlignedLastQueryScanNode) child).getSeriesPath().getDevice();
+ }
+ return fullPath;
+ }))
+ .collect(Collectors.toList()));
+ } else {
+ for (PlanNode child : root.getChildren()) {
+ addSortForEachLastQueryNode(child, orderByParameter);
+ }
+ }
Review Comment:
}
```suggestion
} else {
for (PlanNode child : root.getChildren()) {
addSortForEachLastQueryNode(child, orderByParameter);
}
}
```
for (PlanNode child : root.getChildren()) {
addSortForEachLastQueryNode(child, orderByParameter);
}
--
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] sonarcloud[bot] commented on pull request #8683: Fix bug in last query
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #8683:
URL: https://github.com/apache/iotdb/pull/8683#issuecomment-1367843258
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=8683)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #8683: Fix bug in last query
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #8683:
URL: https://github.com/apache/iotdb/pull/8683#issuecomment-1369327029
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=8683)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=8683&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=8683&metric=new_duplicated_lines_density&view=list)
--
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] lancelly commented on a diff in pull request #8683: Fix bug in last query
Posted by GitBox <gi...@apache.org>.
lancelly commented on code in PR #8683:
URL: https://github.com/apache/iotdb/pull/8683#discussion_r1059731095
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/distribution/SourceRewriter.java:
##########
@@ -525,13 +528,46 @@ public List<PlanNode> visitLastQuery(LastQueryNode node, DistributionPlanContext
PlanNode root = processRawMultiChildNode(node, context);
if (context.queryMultiRegion) {
PlanNode newRoot = genLastQueryRootNode(node, context);
+ // add sort op for each if we add LastQueryMergeNode as root
+ if (newRoot instanceof LastQueryMergeNode && node.getMergeOrderParameter().isEmpty()) {
+ OrderByParameter orderByParameter =
+ new OrderByParameter(
+ Collections.singletonList(new SortItem(SortKey.TIMESERIES, Ordering.ASC)));
+ addSortForEachLastQueryNode(root, orderByParameter);
+ }
root.getChildren().forEach(newRoot::addChild);
return Collections.singletonList(newRoot);
} else {
return Collections.singletonList(root);
}
}
+ private void addSortForEachLastQueryNode(PlanNode root, OrderByParameter orderByParameter) {
+ if (root instanceof LastQueryNode) {
+ LastQueryNode lastQueryNode = (LastQueryNode) root;
+ lastQueryNode.setMergeOrderParameter(orderByParameter);
+ // sort children node
+ lastQueryNode.setChildren(
+ lastQueryNode.getChildren().stream()
+ .sorted(
+ Comparator.comparing(
+ child -> {
+ String fullPath = "";
+ if (child instanceof LastQueryScanNode) {
+ fullPath = ((LastQueryScanNode) child).getSeriesPath().getFullPath();
+ } else if (child instanceof AlignedLastQueryScanNode) {
+ fullPath = ((AlignedLastQueryScanNode) child).getSeriesPath().getDevice();
+ }
+ return fullPath;
+ }))
+ .collect(Collectors.toList()));
+ } else {
+ for (PlanNode child : root.getChildren()) {
+ addSortForEachLastQueryNode(child, orderByParameter);
+ }
+ }
Review Comment:
```suggestion
}
for (PlanNode child : root.getChildren()) {
addSortForEachLastQueryNode(child, orderByParameter);
}
```
--
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] lancelly commented on a diff in pull request #8683: Fix bug in last query
Posted by GitBox <gi...@apache.org>.
lancelly commented on code in PR #8683:
URL: https://github.com/apache/iotdb/pull/8683#discussion_r1059728166
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/distribution/SourceRewriter.java:
##########
@@ -525,13 +528,46 @@ public List<PlanNode> visitLastQuery(LastQueryNode node, DistributionPlanContext
PlanNode root = processRawMultiChildNode(node, context);
if (context.queryMultiRegion) {
PlanNode newRoot = genLastQueryRootNode(node, context);
+ // add sort op for each if we add LastQueryMergeNode as root
+ if (newRoot instanceof LastQueryMergeNode && node.getMergeOrderParameter().isEmpty()) {
+ OrderByParameter orderByParameter =
+ new OrderByParameter(
+ Collections.singletonList(new SortItem(SortKey.TIMESERIES, Ordering.ASC)));
+ addSortForEachLastQueryNode(root, orderByParameter);
+ }
root.getChildren().forEach(newRoot::addChild);
return Collections.singletonList(newRoot);
} else {
return Collections.singletonList(root);
}
}
+ private void addSortForEachLastQueryNode(PlanNode root, OrderByParameter orderByParameter) {
+ if (root instanceof LastQueryNode) {
Review Comment:
It's possible that the children of LastQueryNode are still LastQueryNodes and these children won't be handled by this method.
<img width="798" alt="截屏2023-01-01 17 30 24" src="https://user-images.githubusercontent.com/108499334/210166400-3c6728ea-c73d-4893-8a63-526ae68c7c54.png">
--
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 merged pull request #8683: [IOTDB-5245] Fix bug in last query
Posted by GitBox <gi...@apache.org>.
JackieTien97 merged PR #8683:
URL: https://github.com/apache/iotdb/pull/8683
--
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