From 86ab422ec8346178e0560f26991477dc117f644c Mon Sep 17 00:00:00 2001 From: Chen YZ <43774645+Cpaulyz@users.noreply.github.com> Date: Wed, 5 Jul 2023 08:39:47 +0800 Subject: [PATCH] Make error messages more detailed when creating and upserting templates --- .../db/it/schema/IoTDBSchemaTemplateIT.java | 28 +++++++++++++++++++ .../persistence/schema/TemplateTableTest.java | 15 ++++++++++ .../plan/analyze/AnalyzeVisitor.java | 14 ++++++++-- .../template/ClusterTemplateManager.java | 25 ++++++++++------- .../db/schemaengine/template/Template.java | 9 ++++-- 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/schema/IoTDBSchemaTemplateIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/schema/IoTDBSchemaTemplateIT.java index 318beae302..f6b1dbdab9 100644 --- a/integration-test/src/test/java/org/apache/iotdb/db/it/schema/IoTDBSchemaTemplateIT.java +++ b/integration-test/src/test/java/org/apache/iotdb/db/it/schema/IoTDBSchemaTemplateIT.java @@ -800,4 +800,32 @@ public class IoTDBSchemaTemplateIT extends AbstractSchemaIT { Assert.assertTrue(expectedResult.isEmpty()); } } + + @Test + public void testAlterTemplateTimeseries() throws Exception { + try (Connection connection = EnvFactory.getEnv().getConnection(); + Statement statement = connection.createStatement()) { + statement.execute("SET SCHEMA TEMPLATE t1 TO root.sg1.d1;"); + statement.execute("CREATE TIMESERIES OF SCHEMA TEMPLATE ON root.sg1.d1;"); + try { + statement.execute( + "ALTER timeseries root.sg1.d1.s1 UPSERT tags(s0_tag1=s0_tag1, s0_tag2=s0_tag2) attributes(s0_attr1=s0_attr1, s0_attr2=s0_attr2);"); + Assert.fail("expect exception because the template timeseries does not support tag"); + } catch (Exception e) { + Assert.assertTrue( + e.getMessage() + .contains( + "Cannot alter template timeseries [root.sg1.d1.s1] since schema template [t1] already set on path [root.sg1.d1]")); + } + try { + statement.execute("ALTER timeseries root.sg1.d1.s1 UPSERT ALIAS=s0Alias;"); + Assert.fail("expect exception because the template timeseries does not support alias"); + } catch (Exception e) { + Assert.assertTrue( + e.getMessage() + .contains( + "Cannot alter template timeseries [root.sg1.d1.s1] since schema template [t1] already set on path [root.sg1.d1]")); + } + } + } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/schema/TemplateTableTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/schema/TemplateTableTest.java index 9ba0466e35..be87f049cb 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/schema/TemplateTableTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/schema/TemplateTableTest.java @@ -88,6 +88,21 @@ public class TemplateTableTest { } } + @Test + public void testDuplicatePath() { + List measurements = Arrays.asList("s1", "s1"); + List dataTypes = Arrays.asList(TSDataType.FLOAT, TSDataType.BOOLEAN); + List encodings = Arrays.asList(TSEncoding.RLE, TSEncoding.PLAIN); + List compressors = + Arrays.asList(CompressionType.SNAPPY, CompressionType.SNAPPY); + try { + new Template("template", measurements, dataTypes, encodings, compressors); + Assert.fail("expect IllegalPathException"); + } catch (IllegalPathException e) { + // do nothing + } + } + private Template newSchemaTemplate(String name) throws IllegalPathException { List measurements = Arrays.asList(name + "_" + "temperature", name + "_" + "status"); List dataTypes = Arrays.asList(TSDataType.FLOAT, TSDataType.BOOLEAN); diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeVisitor.java index a1267b7d09..8ed2c381e8 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/AnalyzeVisitor.java @@ -2141,9 +2141,17 @@ public class AnalyzeVisitor extends StatementVisitor Analysis analysis = new Analysis(); analysis.setStatement(alterTimeSeriesStatement); - if (alterTimeSeriesStatement.getAlias() != null) { - checkIsTemplateCompatible( - alterTimeSeriesStatement.getPath(), alterTimeSeriesStatement.getAlias()); + Pair templateInfo = + schemaFetcher.checkTemplateSetAndPreSetInfo( + alterTimeSeriesStatement.getPath(), alterTimeSeriesStatement.getAlias()); + if (templateInfo != null) { + throw new RuntimeException( + new TemplateImcompatibeException( + String.format( + "Cannot alter template timeseries [%s] since schema template [%s] already set on path [%s].", + alterTimeSeriesStatement.getPath().getFullPath(), + templateInfo.left.getName(), + templateInfo.right))); } PathPatternTree patternTree = new PathPatternTree(); diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java index 72487914cb..da89421d20 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java @@ -26,6 +26,7 @@ import org.apache.iotdb.commons.consensus.ConfigRegionId; import org.apache.iotdb.commons.exception.IllegalPathException; import org.apache.iotdb.commons.exception.IoTDBException; import org.apache.iotdb.commons.exception.MetadataException; +import org.apache.iotdb.commons.exception.runtime.SchemaExecutionException; import org.apache.iotdb.commons.path.PartialPath; import org.apache.iotdb.commons.path.PathPatternUtil; import org.apache.iotdb.commons.utils.TestOnly; @@ -135,16 +136,20 @@ public class ClusterTemplateManager implements ITemplateManager { private TCreateSchemaTemplateReq constructTCreateSchemaTemplateReq( CreateSchemaTemplateStatement statement) { TCreateSchemaTemplateReq req = new TCreateSchemaTemplateReq(); - Template template = - new Template( - statement.getName(), - statement.getMeasurements(), - statement.getDataTypes(), - statement.getEncodings(), - statement.getCompressors(), - statement.isAligned()); - req.setName(template.getName()); - req.setSerializedTemplate(template.serialize()); + try { + Template template = + new Template( + statement.getName(), + statement.getMeasurements(), + statement.getDataTypes(), + statement.getEncodings(), + statement.getCompressors(), + statement.isAligned()); + req.setName(template.getName()); + req.setSerializedTemplate(template.serialize()); + } catch (IllegalPathException e) { + throw new SchemaExecutionException(e); + } return req; } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/Template.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/Template.java index 0ca85de243..ac16825bbf 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/Template.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/Template.java @@ -55,7 +55,8 @@ public class Template implements Serializable { List measurements, List dataTypes, List encodings, - List compressors) { + List compressors) + throws IllegalPathException { this(name, measurements, dataTypes, encodings, compressors, false); } @@ -65,11 +66,15 @@ public class Template implements Serializable { List dataTypes, List encodings, List compressors, - boolean isAligned) { + boolean isAligned) + throws IllegalPathException { this.isDirectAligned = isAligned; this.schemaMap = new ConcurrentHashMap<>(); this.name = name; for (int i = 0; i < measurements.size(); i++) { + if (schemaMap.containsKey(measurements.get(i))) { + throw new IllegalPathException("Path duplicated: " + measurements.get(i)); + } IMeasurementSchema schema = new MeasurementSchema( measurements.get(i), dataTypes.get(i), encodings.get(i), compressors.get(i)); -- GitLab