JackieTien97 commented on code in PR #13240:
URL: https://github.com/apache/iotdb/pull/13240#discussion_r1728090311
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java:
##########
@@ -506,19 +506,14 @@ public Node
visitDeleteStatement(RelationalSqlParser.DeleteStatementContext ctx)
}
@Override
- public Node visitUpdateStatement(RelationalSqlParser.UpdateStatementContext
ctx) {
- if (ctx.booleanExpression() != null) {
- return new Update(
- getLocation(ctx),
- new Table(getLocation(ctx), getQualifiedName(ctx.qualifiedName())),
- visit(ctx.updateAssignment(), UpdateAssignment.class),
- (Expression) visit(ctx.booleanExpression()));
- } else {
- return new Update(
- getLocation(ctx),
- new Table(getLocation(ctx), getQualifiedName(ctx.qualifiedName())),
- visit(ctx.updateAssignment(), UpdateAssignment.class));
- }
+ public Node visitUpdateStatement(final
RelationalSqlParser.UpdateStatementContext ctx) {
+ return new Update(
+ getLocation(ctx),
+ new Table(getLocation(ctx), getQualifiedName(ctx.qualifiedName())),
+ visit(ctx.updateAssignment(), UpdateAssignment.class),
+ Objects.nonNull(ctx.booleanExpression())
+ ? (Expression) visit(ctx.booleanExpression())
+ : null);
Review Comment:
```suggestion
visitIfPresent(context.booleanExpression(), Expression.class);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/node/CreateOrUpdateTableDeviceNode.java:
##########
@@ -316,4 +322,14 @@ public int hashCode() {
regionReplicaSet,
partitionKeyList);
}
+
+ @Override
+ public SchemaRegionPlanType getPlanType() {
+ return SchemaRegionPlanType.CREATE_TABLE_DEVICE;
Review Comment:
```suggestion
return SchemaRegionPlanType.CREATE_OR_UPDATE_TABLE_DEVICE;
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/util/SqlFormatter.java:
##########
@@ -696,7 +696,7 @@ protected Void visitUpdate(Update node, Integer indent) {
builder
.append("\n")
.append(indentString(indent + 1))
- .append(assignment.getName().getValue())
+ .append(((Identifier) assignment.getName()).getValue())
Review Comment:
```suggestion
.append(assignment.getName().getValue())
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/UpdateAssignment.java:
##########
@@ -21,28 +21,32 @@
import com.google.common.collect.ImmutableList;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
import java.util.List;
import java.util.Objects;
import static java.util.Objects.requireNonNull;
public class UpdateAssignment extends Node {
- private final Identifier name;
+ private final Expression name;
Review Comment:
left should always be an `Identifier`.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/AbstractTraverseDevice.java:
##########
@@ -23,17 +23,22 @@
import org.apache.iotdb.commons.schema.table.TsTable;
import org.apache.iotdb.db.queryengine.common.MPPQueryContext;
import org.apache.iotdb.db.queryengine.common.SessionInfo;
+import org.apache.iotdb.db.queryengine.common.header.ColumnHeader;
import org.apache.iotdb.db.queryengine.plan.relational.metadata.DeviceEntry;
import org.apache.iotdb.db.queryengine.plan.relational.metadata.MetadataUtil;
import
org.apache.iotdb.db.queryengine.plan.relational.metadata.QualifiedObjectName;
import
org.apache.iotdb.db.queryengine.plan.relational.metadata.fetcher.TableDeviceSchemaFetcher;
import
org.apache.iotdb.db.queryengine.plan.relational.planner.ir.ExtractCommonPredicatesExpressionRewriter;
+import com.google.common.collect.ImmutableList;
import org.apache.tsfile.file.metadata.IDeviceID;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
+import java.util.Optional;
+
+import static
org.apache.iotdb.db.queryengine.plan.relational.sql.ast.AbstractQueryDeviceWithCache.getDeviceColumnHeaderList;
// TODO table metadata: reuse query distinct logic
public abstract class AbstractTraverseDevice extends Statement {
Review Comment:
So strange for an ASTNode to contain these fields:
String database;
String tableName;
private List<List<SchemaFilter>> idDeterminedFilterList;
private Expression idFuzzyPredicate;
private List<IDeviceID> partitionKeyList;
protected List<ColumnHeader> columnHeaderList;
These should be contained in a PlanNode after logical planning
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -2532,31 +2584,48 @@ protected Scope visitCountDevice(final CountDevice
node, final Optional<Scope> c
private void analyzeQueryDevice(
final AbstractQueryDeviceWithCache node, final Optional<Scope>
context) {
- node.parseQualifiedName(sessionContext);
+ analyzeTraverseDevice(node, context, node.getWhere().isPresent());
+ final TsTable table =
+ DataNodeTableCache.getInstance().getTable(node.getDatabase(),
node.getTableName());
Review Comment:
use metadata.getTableSchema instead.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -362,8 +366,50 @@ protected Scope visitShowIndex(ShowIndex node,
Optional<Scope> context) {
}
@Override
- protected Scope visitUpdate(Update node, Optional<Scope> context) {
- throw new SemanticException("Update statement is not supported yet.");
+ protected Scope visitUpdate(final Update node, final Optional<Scope>
context) {
+ queryContext.setQueryType(QueryType.WRITE);
+ final TranslationMap translationMap = analyzeTraverseDevice(node,
context, true);
Review Comment:
TranslationMap should be used in logical planner step, not in statement
analyze step.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -362,8 +366,50 @@ protected Scope visitShowIndex(ShowIndex node,
Optional<Scope> context) {
}
@Override
- protected Scope visitUpdate(Update node, Optional<Scope> context) {
- throw new SemanticException("Update statement is not supported yet.");
+ protected Scope visitUpdate(final Update node, final Optional<Scope>
context) {
+ queryContext.setQueryType(QueryType.WRITE);
+ final TranslationMap translationMap = analyzeTraverseDevice(node,
context, true);
+ final TsTable table =
+ DataNodeTableCache.getInstance().getTable(node.getDatabase(),
node.getTableName());
Review Comment:
Avoid directly call the singleton.getInstance, using
`metadata.getTableSchema` instead. And actually you need to take care of what
if the returned Optional.empty(), which means there is no such table.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
