Copilot commented on code in PR #17270:
URL: https://github.com/apache/iotdb/pull/17270#discussion_r2901003444
##########
iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4:
##########
@@ -914,6 +914,7 @@ queryStatement
: query
#statementDefault
| EXPLAIN query #explain
| EXPLAIN ANALYZE VERBOSE? query
#explainAnalyze
+ | DESCRIBE QUERY query
#describeQuery
Review Comment:
`DESCRIBE QUERY` is added to the grammar, but there is no corresponding AST
builder override (e.g., in `AstBuilder`) to construct a `DescribeQuery` node.
With the default ANTLR visitor behavior, this alternative will likely return
the inner `query` AST directly, causing `DESCRIBE QUERY ...` to execute the
query instead of describing it. Add an `AstBuilder#visitDescribeQuery(...)`
that wraps the parsed `query` in the new `DescribeQuery` statement.
```suggestion
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -850,6 +851,22 @@ protected Scope visitExplain(Explain node, Optional<Scope>
context) {
return visitQuery((Query) node.getStatement(), context);
}
+ @Override
+ protected Scope visitDescribeQuery(DescribeQuery node, Optional<Scope>
context) {
+ analysis.setFinishQueryAfterAnalyze();
+ Scope scope = visitQuery(node.getQuery(), context);
+
+ RelationType outputDescriptor = analysis.getOutputDescriptor();
+ for (Field field : outputDescriptor.getVisibleFields()) {
+ String name = field.getName().orElse("unknown");
+ String type = field.getType().toString();
+ System.out.println("DESCRIBE_DEBUG: Column=" + name + ", Type=" +
type);
+ }
+
Review Comment:
`visitDescribeQuery` calls `analysis.getOutputDescriptor()` after analyzing
the inner query, but the analysis scope is set on `node.getQuery()` (see
`visitQuery`), not on the `DescribeQuery` statement node. This will throw when
`Analysis` tries to resolve the root scope. Use
`analysis.getOutputDescriptor(node.getQuery())` (or the returned `scope`) and
also assign a scope to the `DescribeQuery` node if it is the root statement.
```suggestion
// Retrieve output descriptor for the analyzed inner query
RelationType outputDescriptor =
analysis.getOutputDescriptor(node.getQuery());
for (Field field : outputDescriptor.getVisibleFields()) {
String name = field.getName().orElse("unknown");
String type = field.getType().toString();
System.out.println("DESCRIBE_DEBUG: Column=" + name + ", Type=" +
type);
}
// Assign a scope to the DescribeQuery node when it is the root
statement
if (!context.isPresent()) {
analysis.setScope(node, scope);
}
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/DescribeQuery.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.queryengine.plan.relational.sql.ast;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.tsfile.utils.RamUsageEstimator;
+
+import java.util.List;
+import java.util.Objects;
+
+import static java.util.Objects.requireNonNull;
+
+public class DescribeQuery extends Statement {
+
+ private static final long INSTANCE_SIZE =
+ RamUsageEstimator.shallowSizeOfInstance(DescribeQuery.class);
+
+ private final Query query;
+
+ public DescribeQuery(NodeLocation location, Query query) {
+ super(requireNonNull(location, "location is null"));
+ this.query = requireNonNull(query, "query is null");
+ }
+
+ public Query getQuery() {
+ return query;
+ }
+
+ @Override
+ public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
+ return visitor.visitDescribeQuery(this, context);
+ }
+
+ @Override
+ public List<Node> getChildren() {
+ return ImmutableList.of(query);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) return true;
+ if (obj == null || getClass() != obj.getClass()) return false;
Review Comment:
`equals` uses single-line `if` statements without braces (e.g., `if (this ==
obj) return true;`). This violates the repo Checkstyle `NeedBraces` rule and
will fail style checks. Add braces for these conditionals.
```suggestion
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -850,6 +851,22 @@ protected Scope visitExplain(Explain node, Optional<Scope>
context) {
return visitQuery((Query) node.getStatement(), context);
}
+ @Override
+ protected Scope visitDescribeQuery(DescribeQuery node, Optional<Scope>
context) {
+ analysis.setFinishQueryAfterAnalyze();
+ Scope scope = visitQuery(node.getQuery(), context);
+
+ RelationType outputDescriptor = analysis.getOutputDescriptor();
+ for (Field field : outputDescriptor.getVisibleFields()) {
+ String name = field.getName().orElse("unknown");
+ String type = field.getType().toString();
+ System.out.println("DESCRIBE_DEBUG: Column=" + name + ", Type=" +
type);
+ }
Review Comment:
Avoid `System.out.println` in the analyzer path. This will write to stdout
for every DESCRIBE and can be noisy in production; use the project logger at an
appropriate level or store the extracted column metadata in `Analysis` for the
planner/operator to consume instead of printing.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/DescribeQuery.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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
Review Comment:
The Apache license header looks malformed: it currently reads `Version 2.0
(the` followed by `License");` (missing the opening quote before License).
Please fix the header to match the standard `... (the "License"); ...` form
used elsewhere in the codebase.
```suggestion
* "License"); you may not use this file except in compliance
```
--
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]
