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/02/16 07:40:17 UTC
[GitHub] [iotdb] cornmonster opened a new pull request #5063: [IOTDB-2543] Support le & lt filters in last query
cornmonster opened a new pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063
Please see [IOTDB-2543](https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-2543?filter=myopenissues) for detailed information.
--
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 #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46597135)
Coverage increased (+0.004%) to 67.728% when pulling **741a0e0394c8ae13499f86ba4270646943a322d4 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46833538)
Coverage increased (+0.007%) to 67.818% when pulling **cd95929559dfa231295f87708d9c958ab23f1083 on cornmonster:feature/le_and_lt_filter_in_last_query** into **aa9efee3905bc0ba78b6c61b1e9207daf99d0d19 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 commented on pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46592065)
Coverage decreased (-0.04%) to 67.687% when pulling **25384391b05a2692ebde0a0e1706723ae4981819 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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] ericpai commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807715613
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -215,26 +220,35 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
// Compute Last result for the rest series paths by scanning Tsfiles
int index = 0;
- for (int i = 0; i < resultContainer.size(); i++) {
- if (Boolean.FALSE.equals(resultContainer.get(i).left)) {
- resultContainer.get(i).right = readerList.get(index++).readLastPoint();
- if (resultContainer.get(i).right.getValue() != null) {
- resultContainer.get(i).left = true;
- if (CACHE_ENABLED) {
- cacheAccessors.get(i).write(resultContainer.get(i).right);
- if (context.isDebug()) {
- DEBUG_LOGGER.info(
- "[LastQueryExecutor] Update last cache for path: {} with timestamp: {}",
- seriesPaths,
- resultContainer.get(i).right.getTimestamp());
- }
+ for (int i = 0; i < cachedLastPairs.size(); i++) {
+ if (cachedLastPairs.get(i).right == null) {
+ cachedLastPairs.get(i).right = readerList.get(index++).readLastPoint();
+ // Update the cache only when,
+ // 1. the last value cache doesn't exist
+ // 2. the value is not null
+ // 3. last value cache is enabled
+ // 3. the filter is gt (greater than) or ge (greater than or equal to)
Review comment:
`3.` => `4.`
--
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] cornmonster commented on a change in pull request #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r810962173
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -215,26 +220,35 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
// Compute Last result for the rest series paths by scanning Tsfiles
int index = 0;
- for (int i = 0; i < resultContainer.size(); i++) {
- if (Boolean.FALSE.equals(resultContainer.get(i).left)) {
- resultContainer.get(i).right = readerList.get(index++).readLastPoint();
- if (resultContainer.get(i).right.getValue() != null) {
- resultContainer.get(i).left = true;
- if (CACHE_ENABLED) {
- cacheAccessors.get(i).write(resultContainer.get(i).right);
- if (context.isDebug()) {
- DEBUG_LOGGER.info(
- "[LastQueryExecutor] Update last cache for path: {} with timestamp: {}",
- seriesPaths,
- resultContainer.get(i).right.getTimestamp());
- }
+ for (int i = 0; i < cachedLastPairs.size(); i++) {
+ if (cachedLastPairs.get(i).right == null) {
Review comment:
Good catch! I am busy working on a test right now, will update the PR in 2 days.
--
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] cornmonster commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807760980
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
Review comment:
Good catch! I have fixed the comment and updated the documents as @ericpai suggested.
--
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] HTHou commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807732872
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
Review comment:
Change this? `// Only > and >= are supported in time filter`
--
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 #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46593855)
Coverage decreased (-0.02%) to 67.702% when pulling **9661e0d82679c3efb42df8cc5887433743e2d5a9 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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] JackieTien97 commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807837062
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -171,8 +175,9 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
nonCachedPaths,
nonCachedDataTypes,
context.isDebug());
+
if (nonCachedPaths.isEmpty()) {
- return resultContainer;
+ return cachedLastPairs.stream().map(pair -> pair.right).collect(Collectors.toList());
Review comment:
Actually, the reason that we directly return the resultContainer is that we want to reuse it instead of creating another ArrayList, even if the pair.left will never be used. It is for performance. If you really want to not return extra result, you can make readLastPairsFromCache to return Pair<boolean[], List<TimeValuePair>> instead and return res.right.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -215,26 +220,35 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
// Compute Last result for the rest series paths by scanning Tsfiles
int index = 0;
- for (int i = 0; i < resultContainer.size(); i++) {
- if (Boolean.FALSE.equals(resultContainer.get(i).left)) {
- resultContainer.get(i).right = readerList.get(index++).readLastPoint();
- if (resultContainer.get(i).right.getValue() != null) {
- resultContainer.get(i).left = true;
- if (CACHE_ENABLED) {
- cacheAccessors.get(i).write(resultContainer.get(i).right);
- if (context.isDebug()) {
- DEBUG_LOGGER.info(
- "[LastQueryExecutor] Update last cache for path: {} with timestamp: {}",
- seriesPaths,
- resultContainer.get(i).right.getTimestamp());
- }
+ for (int i = 0; i < cachedLastPairs.size(); i++) {
+ if (cachedLastPairs.get(i).right == null) {
Review comment:
Here, if the filter is gt (greater than) or ge (greater than or equal to) and cachedLastPairs.get(i).left is Boolean.TRUE, we don't need to read from disk again.
##########
File path: docs/UserGuide/Query-Data/Last-Query.md
##########
@@ -21,27 +21,27 @@
# Last Query
-The last query is a special query provided in the time series database Apache IoTDB. The last query returns the data point with the largest timestamp in the time series, that is, the latest state of a sequence. Users can specify the last query through `select last`. It is especially important in IoT data analysis scenarios as the latest point data characterizes the current state. In order to provide a millisecond-level return speed, Apache IoTDB optimizes the cache for the last query to meet users' performance requirements for real-time monitoring of devices.
+The last query is a special type of query in Apache IoTDB. It returns the data point with the largest timestamp of the specified time series. In other word, it returns the latest state of a time series. This feature is especially important in IoT data analysis scenarios. To meets the performance requirement of real-time device monitoring systems, Apache IoTDB caches the latest values of all time series to achieve microsecond read latency.
Review comment:
```suggestion
The last query is a special type of query in Apache IoTDB. It returns the data point with the largest timestamp of the specified time series. In other word, it returns the latest state of a time series. This feature is especially important in IoT data analysis scenarios. To meet the performance requirement of real-time device monitoring systems, Apache IoTDB caches the latest values of all time series to achieve microsecond read latency.
```
--
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] ericpai commented on pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041292154
Need to update the docs. As we have two scenarios of `LAST` query now:
1. Query with only time filters `>` AND `>=` or without any filters: IoTDB will try to get from the last cache first and then tsfile, and then update the cache.
2. Query with time filters '<' or any other composed ones: It will query the tsfile directly without updating the cache.
We should let users know the difference of the performace impact of them. And leave the choice to users. The `LAST` keyword in 2 is just a syntax sugar.
--
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] cornmonster commented on pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041301662
> Need to update the docs. As we have two scenarios of `LAST` query now:
>
> 1. Query with only time filters `>` AND `>=` or without any filters: IoTDB will try to get from the last cache first and then tsfile, and then update the cache.
> 2. Query with time filters '<' or any other composed ones: It will query the tsfile directly without updating the cache.
>
> We should let users know the difference of the performace impact of them. And leave the choice to users. The `LAST` keyword in 2 is just a syntax sugar.
Agree.
--
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] cornmonster commented on pull request #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1048577862
@JackieTien97
--
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] HTHou commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807729663
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
private boolean isValidExpression(IExpression expression) {
- if (expression instanceof GlobalTimeExpression) {
- Filter filter = ((GlobalTimeExpression) expression).getFilter();
- return filter instanceof TimeGtEq || filter instanceof TimeGt;
- }
- return false;
+ return expression instanceof GlobalTimeExpression;
Review comment:
Only time filters are supported in LAST query.
What about other time filter like `TimeEq`, `TimeNotEq` or `TimeIn` ?
--
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] ericpai commented on pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041330841
LGTM
--
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] cornmonster commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807756227
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
private boolean isValidExpression(IExpression expression) {
- if (expression instanceof GlobalTimeExpression) {
- Filter filter = ((GlobalTimeExpression) expression).getFilter();
- return filter instanceof TimeGtEq || filter instanceof TimeGt;
- }
- return false;
+ return expression instanceof GlobalTimeExpression;
Review comment:
Good question.
Time filter is used in 2 scenarios,
1. judge if the last value cache satisfies the filter
2. being passed to the LastPointReader
Any time filter works with the two scenarios above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46754251)
Coverage increased (+0.09%) to 67.812% when pulling **64748bfeacf47584a4a52a7403f7dfd2e78f8cf7 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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] JackieTien97 merged pull request #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
JackieTien97 merged pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063
--
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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46598082)
Coverage increased (+0.03%) to 67.757% when pulling **741a0e0394c8ae13499f86ba4270646943a322d4 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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] cornmonster commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807721986
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -215,26 +220,35 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
// Compute Last result for the rest series paths by scanning Tsfiles
int index = 0;
- for (int i = 0; i < resultContainer.size(); i++) {
- if (Boolean.FALSE.equals(resultContainer.get(i).left)) {
- resultContainer.get(i).right = readerList.get(index++).readLastPoint();
- if (resultContainer.get(i).right.getValue() != null) {
- resultContainer.get(i).left = true;
- if (CACHE_ENABLED) {
- cacheAccessors.get(i).write(resultContainer.get(i).right);
- if (context.isDebug()) {
- DEBUG_LOGGER.info(
- "[LastQueryExecutor] Update last cache for path: {} with timestamp: {}",
- seriesPaths,
- resultContainer.get(i).right.getTimestamp());
- }
+ for (int i = 0; i < cachedLastPairs.size(); i++) {
+ if (cachedLastPairs.get(i).right == null) {
+ cachedLastPairs.get(i).right = readerList.get(index++).readLastPoint();
+ // Update the cache only when,
+ // 1. the last value cache doesn't exist
+ // 2. the value is not null
+ // 3. last value cache is enabled
+ // 3. the filter is gt (greater than) or ge (greater than or equal to)
Review comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46595205)
Coverage decreased (-0.02%) to 67.703% when pulling **06587e52bb8fe96870742301c979a1e2579ce6fb on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46625629)
Coverage decreased (-0.004%) to 67.719% when pulling **0335598a741508aa6de0f5dabf54fe8c6baea988 on cornmonster:feature/le_and_lt_filter_in_last_query** into **832058c04ed215f67966d15aab8ecf2a78fb4bba 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] HTHou commented on a change in pull request #5063: [IOTDB-2543] Support le & lt filters in last query
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807732872
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
Review comment:
Change this?
--
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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r807848057
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/LastQueryPlan.java
##########
@@ -80,16 +77,12 @@ public void setExpression(IExpression expression) throws QueryProcessException {
if (isValidExpression(expression)) {
super.setExpression(expression);
} else {
- throw new QueryProcessException("Only '>' and '>=' are supported in LAST query");
+ throw new QueryProcessException("Only time filters are supported in LAST query");
}
}
// Only > and >= are supported in time filter
private boolean isValidExpression(IExpression expression) {
- if (expression instanceof GlobalTimeExpression) {
- Filter filter = ((GlobalTimeExpression) expression).getFilter();
- return filter instanceof TimeGtEq || filter instanceof TimeGt;
- }
- return false;
+ return expression instanceof GlobalTimeExpression;
Review comment:
Maybe you could add `TimeEq`, `TimeNotEq` and `TimeIn` case in 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] JackieTien97 commented on a change in pull request #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r813490287
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -150,36 +153,115 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
seriesPaths, dataTypes, context, expression, lastQueryPlan.getDeviceToMeasurements());
}
- public static List<Pair<Boolean, TimeValuePair>> calculateLastPairForSeriesLocally(
+ public static List<TimeValuePair> calculateLastPairForSeriesLocally(
List<PartialPath> seriesPaths,
List<TSDataType> dataTypes,
QueryContext context,
IExpression expression,
Map<String, Set<String>> deviceMeasurementsMap)
throws QueryProcessException, StorageEngineException, IOException {
- List<LastCacheAccessor> cacheAccessors = new ArrayList<>();
Filter filter = (expression == null) ? null : ((GlobalTimeExpression) expression).getFilter();
- List<PartialPath> nonCachedPaths = new ArrayList<>();
- List<TSDataType> nonCachedDataTypes = new ArrayList<>();
- List<Pair<Boolean, TimeValuePair>> resultContainer =
- readLastPairsFromCache(
- seriesPaths,
- dataTypes,
- filter,
- cacheAccessors,
- nonCachedPaths,
- nonCachedDataTypes,
- context.isDebug());
- if (nonCachedPaths.isEmpty()) {
- return resultContainer;
+ if (CACHE_ENABLED) {
+ List<LastCacheAccessor> cacheAccessors = new ArrayList<>();
+ for (PartialPath path : seriesPaths) {
+ if (ID_TABLE_ENABLED) {
+ cacheAccessors.add(new IDTableLastCacheAccessor(path));
+ } else {
+ cacheAccessors.add(new MManagerLastCacheAccessor(path));
+ }
+ }
+
+ List<TimeValuePair> lastPairs =
+ readLastPairsFromCache(seriesPaths, cacheAccessors, context.isDebug());
+
+ List<Integer> nonCachedIndices = new ArrayList<>();
+ List<PartialPath> nonCachedPaths = new ArrayList<>();
+ List<TSDataType> nonCachedDataTypes = new ArrayList<>();
+ for (int i = 0; i < lastPairs.size(); i++) {
+ TimeValuePair lastPair = lastPairs.get(i);
+ if (lastPair == null) {
+ nonCachedPaths.add(((MeasurementPath) seriesPaths.get(i)).transformToExactPath());
+ nonCachedDataTypes.add(dataTypes.get(i));
+ nonCachedIndices.add(i);
+ } else if (!satisfyFilter(filter, lastPair)) {
+ lastPairs.set(i, null);
+ boolean isFilterGtOrGe = (filter instanceof Gt || filter instanceof GtEq);
+ if (!isFilterGtOrGe) {
+ nonCachedPaths.add(((MeasurementPath) seriesPaths.get(i)).transformToExactPath());
+ nonCachedDataTypes.add(dataTypes.get(i));
+ nonCachedIndices.add(i);
+ }
+ }
+ }
+
+ List<TimeValuePair> nonCachedLastPairs =
+ readLastPairsFromStorage(
+ nonCachedPaths, nonCachedDataTypes, filter, context, deviceMeasurementsMap);
+ for (int i = 0; i < nonCachedLastPairs.size(); i++) {
+ // Update the cache only when,
+ // 1. the last value cache doesn't exist
+ // 2. the actual last value is not null
+ // 3. last value cache is enabled
+ // 4. the filter is gt (greater than) or ge (greater than or equal to)
+ if (lastPairs.get(i) == null
Review comment:
```suggestion
if (lastPairs.get(nonCachedIndices.get(i)) == null
```
--
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 #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#issuecomment-1041248576
[](https://coveralls.io/builds/46756478)
Coverage increased (+0.002%) to 67.813% when pulling **2c41d6cba857952ddbc51aa5aa006f39f3986348 on cornmonster:feature/le_and_lt_filter_in_last_query** into **aa9efee3905bc0ba78b6c61b1e9207daf99d0d19 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] cornmonster commented on a change in pull request #5063: [IOTDB-2543] Support all time filters in last query
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #5063:
URL: https://github.com/apache/iotdb/pull/5063#discussion_r811654463
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/LastQueryExecutor.java
##########
@@ -171,8 +175,9 @@ public QueryDataSet execute(QueryContext context, LastQueryPlan lastQueryPlan)
nonCachedPaths,
nonCachedDataTypes,
context.isDebug());
+
if (nonCachedPaths.isEmpty()) {
- return resultContainer;
+ return cachedLastPairs.stream().map(pair -> pair.right).collect(Collectors.toList());
Review comment:
Good point. I refactored the code to solve this problem and make the code more readable. Please take a look.
--
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