Support User Defined time column name #17102
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mns, modify the AccumulatorFactory to explicitly verify if the sort key in aggregations(last, first, last_by, first_by) is actually the time column (#17074)
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces support for user-defined time column names in IoTDB's table model, representing a significant architectural change. Previously, tables had an implicit "time" column that was always present. Now, users can explicitly define time columns with custom names and positions within the table schema.
Changes:
- Removed automatic "time" column creation from TsTable constructor and InformationSchema tables
- Added TIME column category handling to TsFileTableSchemaUtil for proper serialization/deserialization
- Moved time argument handling from AST parsing (AstBuilder) to semantic analysis (ExpressionAnalyzer) with priority-based time column resolution
- Enhanced aggregation functions (first, last, first_by, last_by) to work with user-defined time columns
- Added comprehensive integration tests for user-defined time columns and aggregation function behavior
- Updated TsFileTableGenerator test utility to properly handle TIME column categories
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Updated tsfile dependency version to 2.2.1-260120-SNAPSHOT |
| TsTableColumnCategory.java | Added TIME category conversion methods for TsFile compatibility |
| TsTable.java | Removed automatic TIME column initialization; enhanced column renaming logic |
| TsFileTableSchemaUtil.java | Added TIME column handling in schema conversion with special case for default "time" column |
| InformationSchema.java | Removed TIME column removal operations (no longer auto-created) |
| AstBuilder.java | Removed syntactic sugar for automatic time argument insertion in aggregation functions |
| ExpressionAnalyzer.java | Implemented priority-based time column resolution (TIME category > TIMESTAMP type) |
| StatementAnalyzer.java | Added validation for TIME column presence in INSERT operations |
| TableHeaderSchemaValidator.java | Added time column validation and difference detection for TsFile loading |
| AccumulatorFactory.java | Updated accumulator creation to consider order column for optimization decisions |
| TableOperatorGenerator.java | Added time column validation for aggregation optimizations |
| Multiple test files | Added comprehensive integration tests for user-defined time columns |
| TsFileTableGenerator.java | Updated to exclude TIME columns from data generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ++inputTimeIndex; | ||
| } | ||
| if (inputWithoutTimeColumn != realWithoutTimeColumn | ||
| || !inputWithoutTimeColumn && inputTimeIndex != realTimeIndex) { |
There was a problem hiding this comment.
The condition on line 202 is missing parentheses for the logical AND operation. The expression !inputWithoutTimeColumn && inputTimeIndex != realTimeIndex should be wrapped in parentheses for clarity and to ensure correct precedence. While the current code works due to operator precedence in Java (! binds tighter than &&), adding parentheses would make the logic clearer: (!inputWithoutTimeColumn) && (inputTimeIndex != realTimeIndex) or (inputWithoutTimeColumn != realWithoutTimeColumn) || (!inputWithoutTimeColumn && inputTimeIndex != realTimeIndex).
Suggested change
| || !inputWithoutTimeColumn && inputTimeIndex != realTimeIndex) { | |
| || (!inputWithoutTimeColumn && inputTimeIndex != realTimeIndex)) { |
Contributor
There was a problem hiding this comment.
well, the code logic works
Comment on lines
56
to
87
| @Test | ||
| public void lastAggTest() { | ||
| final String sql = null; | ||
| DataNodeQueryContext dataNodeQueryContext = new DataNodeQueryContext(1); | ||
|
|
||
| SessionInfo sessionInfo = | ||
| new SessionInfo( | ||
| 0, "root", ZoneId.systemDefault(), "last_agg_db", IClientSession.SqlDialect.TABLE); | ||
| QueryId queryId = new QueryId("test"); | ||
| MPPQueryContext context = | ||
| new MPPQueryContext( | ||
| sql, | ||
| queryId, | ||
| sessionInfo, | ||
| new TEndPoint("127.0.0.1", 6667), | ||
| new TEndPoint("127.0.0.1", 6667)); | ||
| Analyzer analyzer = | ||
| new Analyzer(context, new FakePartitionFetcherImpl(), new FakeSchemaFetcherImpl()); | ||
| Statement statement = StatementGenerator.createStatement(sql, ZonedDateTime.now().getOffset()); | ||
| Analysis analysis = analyzer.analyze(statement); | ||
| LogicalPlanner logicalPlanner = new LogicalPlanner(context); | ||
| LogicalQueryPlan logicalPlan = logicalPlanner.plan(analysis); | ||
| DistributionPlanner distributionPlanner = new DistributionPlanner(analysis, logicalPlan); | ||
| FragmentInstance instance = distributionPlanner.planFragments().getInstances().get(0); | ||
|
|
||
| LocalExecutionPlanner localExecutionPlanner = LocalExecutionPlanner.getInstance(); | ||
| localExecutionPlanner.plan( | ||
| instance.getFragment().getPlanNodeTree(), | ||
| instance.getFragment().getTypeProvider(), | ||
| mockFIContext(queryId), | ||
| dataNodeQueryContext); | ||
| } |
There was a problem hiding this comment.
The test method lastAggTest has a null SQL statement (line 58) which will cause a NullPointerException when passed to StatementGenerator on line 74. This test appears to be incomplete or a placeholder. If this test is intended to be functional, it needs a valid SQL statement. If it's a work in progress, consider marking it with @ignore or removing it from this PR.
Contributor
There was a problem hiding this comment.
fixed it, the lastAggTest() is removed
Comment on lines
1
to
233
| /* | ||
| * 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.schema; | ||
|
|
||
| import org.apache.iotdb.db.it.utils.TestUtils; | ||
| 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.itbase.env.BaseEnv; | ||
|
|
||
| import org.junit.AfterClass; | ||
| import org.junit.BeforeClass; | ||
| import org.junit.Test; | ||
| import org.junit.experimental.categories.Category; | ||
| import org.junit.runner.RunWith; | ||
|
|
||
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
| import java.sql.Statement; | ||
| import java.util.Arrays; | ||
|
|
||
| import static org.apache.iotdb.db.it.utils.TestUtils.prepareTableData; | ||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.fail; | ||
|
|
||
| @RunWith(IoTDBTestRunner.class) | ||
| @Category({TableLocalStandaloneIT.class, TableClusterIT.class}) | ||
| public class IoTDBUserDefinedTimeIT { | ||
|
|
||
| private static final String TABLE_DATABASE = "user_defined_time"; | ||
| private static final String VIEW_DATABASE = "user_defined_time_for_view"; | ||
| private static final String SELECT_DATABASE = "select_agg_function"; | ||
| private static final String[] SQLS = | ||
| new String[] { | ||
| "CREATE DATABASE " + TABLE_DATABASE, | ||
| "CREATE DATABASE " + VIEW_DATABASE, | ||
| "CREATE DATABASE " + SELECT_DATABASE | ||
| }; | ||
| private final String header = "ColumnName,DataType,Category,"; | ||
|
|
||
| @BeforeClass | ||
| public static void setUp() throws Exception { | ||
| EnvFactory.getEnv().initClusterEnvironment(); | ||
| prepareTableData(SQLS); | ||
| } | ||
|
|
||
| @AfterClass | ||
| public static void tearDown() throws Exception { | ||
| EnvFactory.getEnv().cleanClusterEnvironment(); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCreateTable() { | ||
| try (final Connection connection = | ||
| EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT); | ||
| final Statement statement = connection.createStatement()) { | ||
| statement.execute("use " + TABLE_DATABASE); | ||
|
|
||
| // create table and do not assign the time column name | ||
| try { | ||
| statement.execute( | ||
| "create table default_not_assign_time(device string tag, s1 int32 field)"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe default_not_assign_time"), | ||
| header, | ||
| Arrays.asList("time,TIMESTAMP,TIME,", "device,STRING,TAG,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("create table without time info fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create table and assign the time column name | ||
| try { | ||
| statement.execute( | ||
| "create table time_in_first(date_time timestamp time, device string tag, s1 int32 field)"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe time_in_first"), | ||
| header, | ||
| Arrays.asList("date_time,TIMESTAMP,TIME,", "device,STRING,TAG,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("assign the name of time column fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create table which of the time column not at the first column | ||
| try { | ||
| statement.execute( | ||
| "create table time_not_in_first(device string tag, date_time timestamp time, s1 int32 field)"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe time_not_in_first"), | ||
| header, | ||
| Arrays.asList("device,STRING,TAG,", "date_time,TIMESTAMP,TIME,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("assign the name of time column fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create table with multi time-column | ||
| try { | ||
| statement.execute( | ||
| "create table with_multi_time(time_type timestamp time, device string tag, date_time timestamp time, s1 int32 field)"); | ||
| fail("Creating table is not be allowed to assign two time columns"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: A table cannot have more than one time column", e.getMessage()); | ||
| } | ||
|
|
||
| // create table with time column that is not timestamp data type | ||
| try { | ||
| statement.execute( | ||
| "create table time_other_type(device string tag, date_time int64 time, s1 int32 field)"); | ||
| fail("The time column has to be assigned a timestamp data type when creating table"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: The time column's type shall be 'timestamp'.", e.getMessage()); | ||
| } | ||
|
|
||
| // Columns in table shall not share the same name time when creating table | ||
| try { | ||
| statement.execute( | ||
| "create table shared_time_name(device string tag, time int64 field, s1 int32 field)"); | ||
| fail("Columns in table shall not share the same name time when creating table"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: Columns in table shall not share the same name time.", e.getMessage()); | ||
| } | ||
|
|
||
| } catch (SQLException e) { | ||
| e.printStackTrace(); | ||
| fail(e.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| private void prepareTreeData() { | ||
| try (final Connection connection = EnvFactory.getEnv().getConnection(BaseEnv.TREE_SQL_DIALECT); | ||
| final Statement statement = connection.createStatement()) { | ||
| statement.execute("create timeseries root.tt.device.s1 with datatype=int32"); | ||
| } catch (SQLException e) { | ||
| e.printStackTrace(); | ||
| fail(e.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testCreateView() { | ||
| prepareTreeData(); | ||
|
|
||
| try (final Connection connection = | ||
| EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT); | ||
| final Statement statement = connection.createStatement()) { | ||
| statement.execute("use " + VIEW_DATABASE); | ||
|
|
||
| // create view and do not assign the time column name | ||
| try { | ||
| statement.execute( | ||
| "create view default_not_assign_time(device string tag, s1 int32 field) as root.tt.**"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe default_not_assign_time"), | ||
| header, | ||
| Arrays.asList("time,TIMESTAMP,TIME,", "device,STRING,TAG,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("create table without time info fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create view which of the time column at the first column | ||
| try { | ||
| statement.execute( | ||
| "create view time_in_first(date_time timestamp time, device string tag, s1 int32 field) as root.tt.**"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe time_in_first"), | ||
| header, | ||
| Arrays.asList("date_time,TIMESTAMP,TIME,", "device,STRING,TAG,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("assign the name of time column fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create view which of the time column not at the first column | ||
| try { | ||
| statement.execute( | ||
| "create view time_not_in_first(device string tag, date_time timestamp time, s1 int32 field) as root.tt.**"); | ||
| TestUtils.assertResultSetEqual( | ||
| statement.executeQuery("describe time_not_in_first"), | ||
| header, | ||
| Arrays.asList("device,STRING,TAG,", "date_time,TIMESTAMP,TIME,", "s1,INT32,FIELD,")); | ||
| } catch (SQLException e) { | ||
| fail("assign the name of time column fails, the specific message: " + e.getMessage()); | ||
| } | ||
|
|
||
| // create view with multi time-column | ||
| try { | ||
| statement.execute( | ||
| "create view with_multi_time(time_type timestamp time, device string tag, date_time timestamp time, s1 int32 field) as root.tt.**"); | ||
| fail("Creating view is not be allowed to assign two time columns"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: A table cannot have more than one time column", e.getMessage()); | ||
| } | ||
|
|
||
| // create table with time column that is not timestamp data type | ||
| try { | ||
| statement.execute( | ||
| "create view time_other_type(device string tag, date_time int64 time, s1 int32 field) as root.tt.**"); | ||
| fail("The time column has to be assigned a timestamp data type when creating view"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: The time column's type shall be 'timestamp'.", e.getMessage()); | ||
| } | ||
|
|
||
| // Columns in table shall not share the same name time when creating table | ||
| try { | ||
| statement.execute( | ||
| "create view shared_time_time(device string tag, time int64 field, s1 int32 field) as root.tt.**"); | ||
| fail("Columns in view shall not share the same name time when creating table"); | ||
| } catch (SQLException e) { | ||
| assertEquals("701: Columns in table shall not share the same name time.", e.getMessage()); | ||
| } | ||
|
|
||
| } catch (SQLException e) { | ||
| e.printStackTrace(); | ||
| fail(e.getMessage()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The test classes IoTDBUserDefinedTimeIT and IoTDBTableWithDefinedTimeIT appear to be nearly identical duplicates. The only significant difference is that IoTDBUserDefinedTimeIT defines an additional database constant SELECT_DATABASE (line 50) that is never used. This duplication makes maintenance more difficult and increases the risk of inconsistencies. Consider consolidating these into a single test class or clearly differentiating their purposes if they are intended to test different scenarios.
Comment on lines
218
to
243
| public void renameColumnSchema(final String oldName, final String newName) { | ||
| executeWrite( | ||
| () -> { | ||
| // Ensures idempotency | ||
| if (columnSchemaMap.containsKey(oldName)) { | ||
| final TsTableColumnSchema schema = columnSchemaMap.remove(oldName); | ||
| final Map<String, String> oldProps = schema.getProps(); | ||
| oldProps.computeIfAbsent(TreeViewSchema.ORIGINAL_NAME, k -> schema.getColumnName()); | ||
| switch (schema.getColumnCategory()) { | ||
| case TAG: | ||
| columnSchemaMap.put( | ||
| newName, new TagColumnSchema(newName, schema.getDataType(), oldProps)); | ||
| break; | ||
| case FIELD: | ||
| columnSchemaMap.put( | ||
| newName, | ||
| new FieldColumnSchema( | ||
| newName, | ||
| schema.getDataType(), | ||
| ((FieldColumnSchema) schema).getEncoding(), | ||
| ((FieldColumnSchema) schema).getCompressor(), | ||
| oldProps)); | ||
| break; | ||
| case ATTRIBUTE: | ||
| columnSchemaMap.put( | ||
| newName, new AttributeColumnSchema(newName, schema.getDataType(), oldProps)); | ||
| break; | ||
| case TIME: | ||
| default: | ||
| // Do nothing | ||
| columnSchemaMap.put(oldName, schema); | ||
| if (!columnSchemaMap.containsKey(oldName)) { | ||
| return; | ||
| } | ||
|
|
||
| // Capture the current strict order of current columns | ||
| List<Map.Entry<String, TsTableColumnSchema>> snapshotOfColumns = | ||
| new ArrayList<>(columnSchemaMap.entrySet()); | ||
| columnSchemaMap.clear(); | ||
|
|
||
| // Re-insert all entries in their original sequence, substituting the renamed column at | ||
| // its exact original index | ||
| for (Map.Entry<String, TsTableColumnSchema> entry : snapshotOfColumns) { | ||
| String currentKey = entry.getKey(); | ||
| TsTableColumnSchema currentColumnSchema = entry.getValue(); | ||
|
|
||
| if (currentKey.equals(oldName)) { | ||
| TsTableColumnSchema newSchema = createRenamedSchema(currentColumnSchema, newName); | ||
| columnSchemaMap.put(newName, newSchema); | ||
| } else { | ||
| columnSchemaMap.put(currentKey, currentColumnSchema); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
When renaming a TAG column in renameColumnSchema, the method does not update the tagColumnIndexMap which maps tag column names to their indices. This will cause the old column name to remain in the map, and the new name to be missing. This could lead to incorrect behavior when looking up tag column indices using getTagColumnOrdinal(). The fix should include updating the tagColumnIndexMap when a TAG column is renamed.
Comment on lines
764
to
765
| // throw new SemanticException("Add column statement shall not specify column category | ||
| // TIME"); |
There was a problem hiding this comment.
The commented-out code on lines 764-765 should be removed instead of being left as a comment. If this exception was intentionally removed to allow TIME column creation, the comment serves no purpose and should be deleted. If there are concerns about this change, they should be documented in commit messages or code documentation, not as commented-out code.
Suggested change
| // throw new SemanticException("Add column statement shall not specify column category | |
| // TIME"); |
Comment on lines
1191
to
1204
| .orElseThrow(() -> new SemanticException("the input field do not exists")) | ||
| .toString() | ||
| : argument.toString(); | ||
|
|
||
| for (Field field : visibleFields) { | ||
| if (field | ||
| .getName() | ||
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) | ||
| .equalsIgnoreCase(argumentName)) { | ||
| return field.getType() == TIMESTAMP; | ||
| } | ||
| } | ||
| // should never reach here | ||
| throw new SemanticException("the input argument do not exists"); |
There was a problem hiding this comment.
Typo in error message: "do not exists" should be "does not exist".
Suggested change
| .orElseThrow(() -> new SemanticException("the input field do not exists")) | |
| .toString() | |
| : argument.toString(); | |
| for (Field field : visibleFields) { | |
| if (field | |
| .getName() | |
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) | |
| .equalsIgnoreCase(argumentName)) { | |
| return field.getType() == TIMESTAMP; | |
| } | |
| } | |
| // should never reach here | |
| throw new SemanticException("the input argument do not exists"); | |
| .orElseThrow(() -> new SemanticException("the input field does not exist")) | |
| .toString() | |
| : argument.toString(); | |
| for (Field field : visibleFields) { | |
| if (field | |
| .getName() | |
| .orElseThrow(() -> new SemanticException("the field in table does not have a name")) | |
| .equalsIgnoreCase(argumentName)) { | |
| return field.getType() == TIMESTAMP; | |
| } | |
| } | |
| // should never reach here | |
| throw new SemanticException("the input argument does not exist"); |
| for (Field field : visibleFields) { | ||
| if (field | ||
| .getName() | ||
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) |
There was a problem hiding this comment.
Typo in error message: "do not hava the name" should be "does not have a name".
Suggested change
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) | |
| .orElseThrow(() -> new SemanticException("the field in table does not have a name")) |
Comment on lines
1191
to
1204
| .orElseThrow(() -> new SemanticException("the input field do not exists")) | ||
| .toString() | ||
| : argument.toString(); | ||
|
|
||
| for (Field field : visibleFields) { | ||
| if (field | ||
| .getName() | ||
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) | ||
| .equalsIgnoreCase(argumentName)) { | ||
| return field.getType() == TIMESTAMP; | ||
| } | ||
| } | ||
| // should never reach here | ||
| throw new SemanticException("the input argument do not exists"); |
There was a problem hiding this comment.
Typo in error message: "do not exists" should be "does not exist".
Suggested change
| .orElseThrow(() -> new SemanticException("the input field do not exists")) | |
| .toString() | |
| : argument.toString(); | |
| for (Field field : visibleFields) { | |
| if (field | |
| .getName() | |
| .orElseThrow(() -> new SemanticException("the field in table do not hava the name")) | |
| .equalsIgnoreCase(argumentName)) { | |
| return field.getType() == TIMESTAMP; | |
| } | |
| } | |
| // should never reach here | |
| throw new SemanticException("the input argument do not exists"); | |
| .orElseThrow(() -> new SemanticException("the input field does not exist")) | |
| .toString() | |
| : argument.toString(); | |
| for (Field field : visibleFields) { | |
| if (field | |
| .getName() | |
| .orElseThrow( | |
| () -> new SemanticException("the field in table does not have a name")) | |
| .equalsIgnoreCase(argumentName)) { | |
| return field.getType() == TIMESTAMP; | |
| } | |
| } | |
| // should never reach here | |
| throw new SemanticException("the input argument does not exist"); |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request enhances the integration tests for the
firstandfirst_byaggregation functions, especially around handling time columns and timestamp-type fields. It adds new test cases to verify priority and fallback logic for time columns, ensures proper error handling when no valid time column is present, and improves the robustness of test coverage for both base and joined tables. Additionally, a minor refactor in the data generation utility ensures that time columns are excluded where appropriate.Key changes include:
Test Coverage and Logic Improvements
Added new test cases in
IoTDBAggregationFirstIT.javaandIoTDBAggregationFirstByIT.javato verify:Extended test scenarios to cover queries on both base tables and joined tables, including cases with explicit and implicit time columns, and grouped queries. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]
Data Generation Utility Refactor
TsFileTableGenerator.javato exclude the time column from measurement schemas and column categories when generating data, ensuring that test data correctly reflects the intended table structure for time-related queries. [1] [2] [3]Test Utility Enhancements
runTest2,runTest3) to streamline repeated test logic for different query forms inIoTDBAggregationFirstByIT.java.These changes collectively improve the reliability and completeness of integration tests for time-aware aggregation functions in IoTDB.