You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "Wei-hao-Li (via GitHub)" <gi...@apache.org> on 2025/07/17 02:10:43 UTC
[PR] Implement Extract expression in TableModel [iotdb]
Wei-hao-Li opened a new pull request, #15960:
URL: https://github.com/apache/iotdb/pull/15960
(no comment)
--
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
Re: [PR] Implement Extract expression in TableModel [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 commented on code in PR #15960:
URL: https://github.com/apache/iotdb/pull/15960#discussion_r2212223532
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/predicate/PredicateCombineIntoTableScanChecker.java:
##########
@@ -41,17 +41,23 @@
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.predicate.PredicatePushIntoScanChecker.isLiteral;
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.predicate.PredicatePushIntoScanChecker.isSymbolReference;
+import static org.apache.iotdb.db.queryengine.plan.relational.planner.ir.GlobalTimePredicateExtractVisitor.isExtractTimeColumn;
public class PredicateCombineIntoTableScanChecker extends PredicateVisitor<Boolean, Void> {
private final Set<String> measurementColumns;
+ private final String timeColumnName;
Review Comment:
no need to add this field?, timeColumnName has already been put in `measurementColumns`.
<img width="1426" height="1460" alt="image" src="https://github.com/user-attachments/assets/0fdba63e-567a-4b5c-98ae-278abbf63c06" />
maybe you can add some comments about `measurementColumns` and change it name from `measurementColumns` to `timeAndFieldsColumnNames`
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/IrTypeAnalyzer.java:
##########
@@ -306,6 +307,11 @@ protected Type visitArithmeticUnary(ArithmeticUnaryExpression node, Context cont
return setExpressionType(node, process(node.getValue(), context));
}
+ @Override
+ protected Type visitExtract(Extract node, Context context) {
+ return setExpressionType(node, INT64);
Review Comment:
```suggestion
process(node.getExpression(), context);
return setExpressionType(node, INT64);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/ir/GlobalTimePredicateExtractVisitor.java:
##########
@@ -269,9 +271,18 @@ public static boolean isTimeColumn(Expression e, String timeColumnName) {
&& ((SymbolReference) e).getName().equalsIgnoreCase(timeColumnName);
}
+ public static boolean isExtractTimeColumn(Expression e, String timeColumnName) {
+ return e instanceof Extract
+ && ((SymbolReference) ((Extract) e).getExpression())
Review Comment:
what if express of Extract is a constant, like `where EXTRACT(HOUR FROM 2025/07/08 01:18:51.123) =1`
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/ExpressionAnalyzer.java:
##########
@@ -1555,6 +1557,22 @@ protected Type visitParameter(Parameter node, StackableAstVisitorContext<Context
return setExpressionType(node, resultType);
}
+ @Override
+ protected Type visitExtract(Extract node, StackableAstVisitorContext<Context> context) {
+ if (node.getExpression() instanceof LongLiteral) {
+ // Don't visit child here to avoid setting its Type to INT32
+ setExpressionType(node.getExpression(), INT64);
Review Comment:
duplicated with the last return statement?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/predicate/PredicateCombineIntoTableScanChecker.java:
##########
@@ -115,14 +121,19 @@ protected Boolean visitLogicalExpression(LogicalExpression node, Void context) {
@Override
protected Boolean visitComparisonExpression(ComparisonExpression node, Void context) {
return (isMeasurementColumn(node.getLeft()) && isLiteral(node.getRight()))
- || (isMeasurementColumn(node.getRight()) && isLiteral(node.getLeft()));
+ || (isMeasurementColumn(node.getRight()) && isLiteral(node.getLeft()))
+ || (isExtractTimeColumn(node.getLeft(), timeColumnName) && isLiteral(node.getRight()))
Review Comment:
even if extract from a field, we should also push it down into tableScan
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/PushPredicateIntoTableScan.java:
##########
@@ -530,7 +531,7 @@ private SplitExpression splitPredicate(DeviceTableScanNode node, Expression pred
if (PredicatePushIntoMetadataChecker.check(idOrAttributeColumnNames, expression)) {
metadataExpressions.add(expression);
} else if (PredicateCombineIntoTableScanChecker.check(
- measurementColumnNames, expression)) {
+ measurementColumnNames, timeColumnName, expression)) {
Review Comment:
```suggestion
measurementColumnNames, expression)) {
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/PushPredicateIntoTableScan.java:
##########
@@ -543,7 +544,8 @@ private SplitExpression splitPredicate(DeviceTableScanNode node, Expression pred
if (PredicatePushIntoMetadataChecker.check(idOrAttributeColumnNames, predicate)) {
metadataExpressions.add(predicate);
- } else if (PredicateCombineIntoTableScanChecker.check(measurementColumnNames, predicate)) {
+ } else if (PredicateCombineIntoTableScanChecker.check(
+ measurementColumnNames, timeColumnName, predicate)) {
Review Comment:
```suggestion
measurementColumnNames, predicate)) {
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/ir/GlobalTimePredicateExtractVisitor.java:
##########
@@ -269,9 +271,18 @@ public static boolean isTimeColumn(Expression e, String timeColumnName) {
&& ((SymbolReference) e).getName().equalsIgnoreCase(timeColumnName);
}
+ public static boolean isExtractTimeColumn(Expression e, String timeColumnName) {
+ return e instanceof Extract
+ && ((SymbolReference) ((Extract) e).getExpression())
Review Comment:
have a try using `IrExpressionInterpreter` to do the constant fold
--
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
Re: [PR] Implement Extract expression in TableModel [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #15960:
URL: https://github.com/apache/iotdb/pull/15960
--
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