Copilot commented on code in PR #17372:
URL: https://github.com/apache/iotdb/pull/17372#discussion_r3007313267
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java:
##########
@@ -2155,6 +2161,62 @@ public Node
visitRevokeStatement(RelationalSqlParser.RevokeStatementContext ctx)
}
}
+ @Override
+ public Node visitCopyToStatement(RelationalSqlParser.CopyToStatementContext
ctx) {
+ RelationalSqlParser.CopyToStatementOptionsContext optionsContext =
ctx.copyToStatementOptions();
+ String targetFileName = parseStringLiteral(ctx.fileName.getText());
+ CopyToOptions.Builder copyToOptionsBuilder = new CopyToOptions.Builder();
+ if (optionsContext != null) {
+ for (RelationalSqlParser.CopyToStatementOptionContext context :
+ optionsContext.copyToStatementOption()) {
+ addCopyToOption(copyToOptionsBuilder, context);
+ }
+ }
+ Statement queryNode = null;
+ if (ctx.tableName != null) {
+ QualifiedName qualifiedName = getQualifiedName(ctx.tableName);
+ if (ctx.tableColumns != null) {
+ List<RelationalSqlParser.IdentifierContext> identifierList =
+ ctx.identifierList().identifier();
+ SelectItem[] selectItems = new SelectItem[identifierList.size()];
+ for (int i = 0; i < identifierList.size(); i++) {
+ Identifier identifier = (Identifier) visit(identifierList.get(i));
+ selectItems[i] = new SingleColumn(identifier, identifier);
+ }
+ queryNode = QueryUtil.simpleQuery(selectList(selectItems),
table(qualifiedName));
+ } else {
+ queryNode = QueryUtil.simpleQuery(selectList(new AllColumns()),
table(qualifiedName));
+ }
+ } else {
+ queryNode = (Statement) visit(ctx.query());
+ }
+ return new CopyTo(queryNode, targetFileName, copyToOptionsBuilder.build());
+ }
+
+ private void addCopyToOption(
+ CopyToOptions.Builder builder,
RelationalSqlParser.CopyToStatementOptionContext context) {
+ if (context.FORMAT() != null) {
+ Identifier formatIdentifier = (Identifier) visit(context.identifier());
+
builder.withFormat(CopyToOptions.Format.valueOf(formatIdentifier.getValue().toUpperCase()));
Review Comment:
FORMAT parsing uses CopyToOptions.Format.valueOf(...). If the user provides
an unsupported value, this will throw IllegalArgumentException and likely
surface as an internal error. Please catch this and rethrow a SemanticException
with a clear message listing supported formats.
```suggestion
String formatText = formatIdentifier.getValue().toUpperCase();
try {
builder.withFormat(CopyToOptions.Format.valueOf(formatText));
} catch (IllegalArgumentException e) {
String supportedFormats =
Arrays.stream(CopyToOptions.Format.values())
.map(Enum::name)
.collect(Collectors.joining(", "));
throw new SemanticException(
String.format(
"Unsupported COPY TO format '%s'. Supported formats: %s",
formatIdentifier.getValue(), supportedFormats),
e);
}
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/UnaliasSymbolReferences.java:
##########
@@ -388,6 +389,24 @@ public PlanAndMappings
visitExplainAnalyze(ExplainAnalyzeNode node, UnaliasConte
mapping);
}
+ @Override
+ public PlanAndMappings visitCopyTo(CopyToNode node, UnaliasContext
context) {
+ PlanAndMappings rewrittenSource = node.getChild().accept(this, context);
+ Map<Symbol, Symbol> mapping = new
HashMap<>(rewrittenSource.getMappings());
+ SymbolMapper mapper = symbolMapper(mapping);
+ List<Symbol> newChildPermittedOutputs =
mapper.map(node.getChildPermittedOutputs());
+ return new PlanAndMappings(
+ new CopyToNode(
+ node.getPlanNodeId(),
+ rewrittenSource.getRoot(),
+ node.getTargetFilePath(),
+ node.getCopyToOptions(),
+ newChildPermittedOutputs,
+ node.getInnerQueryDatasetHeader(),
+ node.getInnerQueryOutputSymbols()),
+ mapping);
Review Comment:
In UnaliasSymbolReferences, CopyToNode rewriting maps childPermittedOutputs
but leaves innerQueryOutputSymbols unchanged. If the child plan is rewritten
(symbol remapped), these stale symbols can no longer be found in the child's
output symbol list, leading to null lookups/NPE when building the column index
mapping in TableOperatorGenerator. Map innerQueryOutputSymbols with the same
SymbolMapper before constructing the new CopyToNode.
##########
integration-test/src/test/java/org/apache/iotdb/relational/it/query/recent/copyto/IoTDBCopyToTsFileIT.java:
##########
@@ -0,0 +1,490 @@
+/*
+ * 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.relational.it.query.recent.copyto;
+
+import org.apache.iotdb.isession.ITableSession;
+import org.apache.iotdb.isession.SessionDataSet;
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.TableClusterIT;
+import org.apache.iotdb.itbase.category.TableLocalStandaloneIT;
+import org.apache.iotdb.rpc.IoTDBConnectionException;
+import org.apache.iotdb.rpc.StatementExecutionException;
+
+import org.apache.tsfile.file.metadata.IDeviceID;
+import org.apache.tsfile.file.metadata.StringArrayDeviceID;
+import org.apache.tsfile.file.metadata.TimeseriesMetadata;
+import org.apache.tsfile.read.TsFileSequenceReader;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.iotdb.db.it.utils.TestUtils.prepareTableData;
+
+@RunWith(IoTDBTestRunner.class)
+@Category({TableLocalStandaloneIT.class, TableClusterIT.class})
+public class IoTDBCopyToTsFileIT {
+
+ private static final String DATABASE_NAME = "test_db";
+
+ protected static final String[] createSqls =
+ new String[] {
+ "CREATE DATABASE " + DATABASE_NAME,
+ "USE " + DATABASE_NAME,
+ "create table table1(tag1 string tag, tag2 string tag, s1 int32 field,
s2 int32 field)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (1, 't1_1', 't2',
1, 1)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (2, 't1_1', 't2',
2, 2)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (3, 't1_1', 't2',
3, 3)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (1, 't1_2', 't2',
1, 1)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (2, 't1_2', 't2',
2, 2)",
+ "insert into table1(time, tag1, tag2, s1, s2) values (3, 't1_2', 't2',
3, 3)",
+ "create table table2(tag1 string tag, tag2 string tag, s1 int32 field,
s2 int32 field)",
+ "insert into table2(time, tag1, tag2, s1, s2) values (1, 't1_1', 't2',
1, 1)",
+ };
+
+ private String targetFilePath = null;
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ EnvFactory.getEnv().initClusterEnvironment();
+ prepareTableData(createSqls);
+ }
+
+ @After
+ public void tearDownAfterTest() {
+ if (targetFilePath != null) {
+ try {
+ Files.deleteIfExists(new File(targetFilePath).toPath());
+ } catch (Exception ignored) {
+ }
+ targetFilePath = null;
+ }
+ }
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ EnvFactory.getEnv().cleanClusterEnvironment();
+ }
+
+ @Test
+ public void testCopyTable()
+ throws IoTDBConnectionException, StatementExecutionException,
IOException {
+ try (ITableSession session =
+ EnvFactory.getEnv().getTableSessionConnectionWithDB(DATABASE_NAME)) {
+ SessionDataSet sessionDataSet =
+ session.executeQueryStatement("copy table1 to '1.tsfile'
(memory_threshold 1000000)");
+ SessionDataSet.DataIterator iterator = sessionDataSet.iterator();
+ while (iterator.next()) {
+ targetFilePath = iterator.getString(1);
+ int rowCount = iterator.getInt(2);
+ int deviceCount = iterator.getInt(3);
+ long sizeInBytes = iterator.getLong(4);
+ String tableName = iterator.getString(5);
+ String timeColumn = iterator.getString(6);
+ String tagColumns = iterator.getString(7);
+
+ Assert.assertTrue(new File(targetFilePath).exists());
+ Assert.assertEquals(6, rowCount);
+ Assert.assertEquals(2, deviceCount);
Review Comment:
COPY_TO_TSFILE_COLUMN_HEADERS defines row_count and device_count as INT64,
but this test reads them via iterator.getInt(...), which truncates INT64 values
and can silently overflow for large exports. Prefer using getLong(...) (and
long variables) for these columns.
```suggestion
long rowCount = iterator.getLong(2);
long deviceCount = iterator.getLong(3);
long sizeInBytes = iterator.getLong(4);
String tableName = iterator.getString(5);
String timeColumn = iterator.getString(6);
String tagColumns = iterator.getString(7);
Assert.assertTrue(new File(targetFilePath).exists());
Assert.assertEquals(6L, rowCount);
Assert.assertEquals(2L, deviceCount);
```
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/node/CopyToNode.java:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.planner.node;
+
+import org.apache.iotdb.commons.schema.column.ColumnHeader;
+import org.apache.iotdb.commons.schema.column.ColumnHeaderConstant;
+import org.apache.iotdb.db.queryengine.common.header.DatasetHeader;
+import
org.apache.iotdb.db.queryengine.execution.operator.process.copyto.CopyToOptions;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanNode;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanNodeId;
+import org.apache.iotdb.db.queryengine.plan.planner.plan.node.PlanVisitor;
+import
org.apache.iotdb.db.queryengine.plan.planner.plan.node.process.SingleChildProcessNode;
+import org.apache.iotdb.db.queryengine.plan.relational.planner.Symbol;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.stream.Collectors;
+
+public class CopyToNode extends SingleChildProcessNode {
+
+ private final String targetFilePath;
+ private final CopyToOptions copyToOptions;
+ private final List<Symbol> childPermittedOutputs;
+ private final List<Symbol> innerQueryOutputSymbols;
+ private final DatasetHeader innerQueryDatasetHeader;
+
+ public CopyToNode(
+ PlanNodeId id,
+ PlanNode child,
+ String targetFilePath,
+ CopyToOptions copyToOptions,
+ List<Symbol> childPermittedOutputs,
+ DatasetHeader innerQueryDatasetHeader,
+ List<Symbol> innerQueryOutputSymbols) {
+ super(id);
+ this.child = child;
+ this.targetFilePath = targetFilePath;
+ this.copyToOptions = copyToOptions;
+ this.childPermittedOutputs = childPermittedOutputs;
+ this.innerQueryDatasetHeader = innerQueryDatasetHeader;
+ this.innerQueryOutputSymbols = innerQueryOutputSymbols;
+ }
+
+ public DatasetHeader getInnerQueryDatasetHeader() {
+ return innerQueryDatasetHeader;
+ }
+
+ public List<Symbol> getInnerQueryOutputSymbols() {
+ return innerQueryOutputSymbols;
+ }
+
+ public String getTargetFilePath() {
+ return targetFilePath;
+ }
+
+ public CopyToOptions getCopyToOptions() {
+ return copyToOptions;
+ }
+
+ public List<Symbol> getChildPermittedOutputs() {
+ return childPermittedOutputs;
+ }
+
+ @Override
+ public PlanNode clone() {
+ return new CopyToNode(
+ id,
+ child,
+ targetFilePath,
+ copyToOptions,
+ childPermittedOutputs,
+ innerQueryDatasetHeader,
+ innerQueryOutputSymbols);
+ }
+
+ @Override
+ public <R, C> R accept(PlanVisitor<R, C> visitor, C context) {
+ return visitor.visitCopyTo(this, context);
+ }
+
+ @Override
+ public List<Symbol> getOutputSymbols() {
+ return ColumnHeaderConstant.COPY_TO_TSFILE_COLUMN_HEADERS.stream()
+ .map(column -> new Symbol(column.getColumnName()))
+ .collect(Collectors.toList());
+ }
+
+ @Override
+ public List<String> getOutputColumnNames() {
+ return ColumnHeaderConstant.COPY_TO_TSFILE_COLUMN_HEADERS.stream()
Review Comment:
CopyToNode.getOutputSymbols()/getOutputColumnNames() are hard-wired to
ColumnHeaderConstant.COPY_TO_TSFILE_COLUMN_HEADERS even though the node already
carries copyToOptions with getRespColumnHeaders(). Using the options here
avoids duplicating the response schema logic and prevents mismatches if more
COPY formats/options are added later.
```suggestion
return copyToOptions.getRespColumnHeaders().stream()
.map(column -> new Symbol(column.getColumnName()))
.collect(Collectors.toList());
}
@Override
public List<String> getOutputColumnNames() {
return copyToOptions.getRespColumnHeaders().stream()
```
--
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]
