[IOTDB-17798] Implement table model NEXT fill#17810
Conversation
JackieTien97
left a comment
There was a problem hiding this comment.
Thanks for the thorough work here — this is a clean, well-structured PR. It faithfully mirrors the existing PREVIOUS/LINEAR fill machinery end to end (grammar → AST/AstBuilder → StatementAnalyzer/Analysis → QueryPlanner → optimizer rules → NextFillNode + serde → operators + freemarker codegen), and the visitor coverage looks complete: the generic visitFill fallbacks (SortElimination keeps the sort for non-ValueFill, TableDistributedPlanGenerator/TableModelTypeProviderExtractor) all handle NextFillNode correctly through inheritance, and every rule that special-cases fill nodes (PruneFillColumns, PushLimitOffsetIntoTableScan, UnaliasSymbolReferences, PlanGraphPrinter, TableOperatorGenerator) was updated. The 1043 node-type id is free and the serde order matches the siblings. Test coverage (parser / serde / fill execution / IT) is solid.
I've left a few minor, non-blocking comments inline — mostly test-coverage hardening plus one defensive note. The only item I'd like confirmation on before merge is the grouped cross-TsBlock reset path in TableNextFillWithGroupOperator#resetFill (see inline comment): it's correct for the cross-group case that's already tested, but the symmetric same-group look-ahead case isn't covered yet and is the one most sensitive to that reset condition.
|
Thanks for the review. The issue has been fixed. @JackieTien97 |
|
@JackieTien97 This failure looks like a self-hosted runner environment issue: |
There was a problem hiding this comment.
Pull request overview
This PR adds support for the table-model FILL METHOD NEXT syntax and execution, enabling “backward/next-value” fill semantics (fill nulls using the nearest later non-null value in the operator input order), including optional TIME_BOUND, TIME_COLUMN, and FILL_GROUP handling. It threads the feature end-to-end through grammar → AST → analyzer/planner → plan nodes/serde/visitors/optimizers → execution operators and codegen, and adds unit + integration tests.
Changes:
- Extend relational SQL grammar + AST/building/formatting to parse and print
FILL METHOD NEXTwith optional modifiers. - Add
NextFillNodeand wire it through plan node types, serde, visitors, plan graph printing, and key optimizations (limit pushdown + pruning + unaliasing). - Implement NEXT-fill execution via new table operators and a generated per-type
NextFillimplementation, with tests covering grouped fill, time-bound behavior, and cross-TsBlocklook-ahead.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4 |
Add NEXT alternative to the fillMethod grammar rule. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/statement/component/FillPolicy.java |
Add FillPolicy.NEXT with byte id 3. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/relational/sql/util/CommonQuerySqlFormatter.java |
Format NEXT fill similarly to PREVIOUS (TIME_BOUND/TIME_COLUMN/FILL_GROUP). |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/relational/sql/ast/Fill.java |
Extend Fill AST to represent NEXT and add a constructor supporting PREVIOUS/NEXT. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/relational/planner/node/PreviousFillNode.java |
Fix equality/hash to include groupingKeys. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/relational/planner/node/NextFillNode.java |
Introduce NextFillNode plan node with serde and visitor support. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/planner/plan/node/PlanNodeType.java |
Register TABLE_NEXT_FILL_NODE type id 1043. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/planner/plan/node/ICoreQueryPlanVisitor.java |
Add visitNextFill default method. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/queryengine/plan/planner/plan/node/CommonPlanNodeDeserializer.java |
Deserialize NextFillNode for node type 1043. |
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/FillStatementTest.java |
Parser unit test for FILL METHOD NEXT with modifiers. |
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/planner/NextFillNodeSerdeTest.java |
Serde tests for NextFillNode + regression for PreviousFillNode equality. |
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/execution/operator/process/fill/NextFillTest.java |
Execution-level tests for NEXT fill, TIME_BOUND, object/binary behavior, and grouped fill. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java |
Build Fill AST for NEXT fill. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/QueryPlanner.java |
Plan FillPolicy.NEXT into a NextFillNode (and grouped path). |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/UnaliasSymbolReferences.java |
Support symbol unaliasing for NextFillNode. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/PushLimitOffsetIntoTableScan.java |
Disable limit/offset pushdown across NEXT fill. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/iterative/rule/PruneFillColumns.java |
Ensure helper/grouping symbols are retained for NEXT fill. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java |
Analyze FillPolicy.NEXT and resolve helper/time column and grouping keys as needed. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/Analysis.java |
Add Analysis.NextFillAnalysis. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/PlanGraphPrinter.java |
Render NextFillNode details in plan graphs. |
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/plan/planner/TableOperatorGenerator.java |
Generate physical operators for NextFillNode (grouped and non-grouped). |
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/plan/planner/CommonOperatorUtils.java |
Add getNextFill() and refactor fill-filter creation. |
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/process/TableNextFillWithGroupOperator.java |
New grouped NEXT fill operator built on linear-fill caching/slicing. |
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/process/TableNextFillOperator.java |
New non-grouped NEXT fill operator. |
iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/process/fill/next/NextFill.java |
NEXT fill algorithm implementation (right-to-left scan + cross-TsBlock look-ahead). |
iotdb-core/calc-commons/src/main/codegen/templates/nextFill.ftl |
Freemarker template generating per-type *NextFill implementations. |
integration-test/src/test/java/org/apache/iotdb/relational/it/query/recent/IoTDBFillTableIT.java |
Integration tests for NEXT fill semantics, modifiers, grouping, and error cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public Fill( | ||
| NodeLocation location, | ||
| FillPolicy fillMethod, | ||
| TimeDuration timeBound, | ||
| LongLiteral timeColumnIndex, | ||
| List<LongLiteral> fillGroupingElements) { | ||
| super(requireNonNull(location, "location is null")); | ||
| if (fillMethod != FillPolicy.PREVIOUS && fillMethod != FillPolicy.NEXT) { | ||
| throw new IllegalArgumentException("Unsupported fill method: " + fillMethod); | ||
| } |
| public class TableNextFillWithGroupOperator extends TableNextFillOperator { | ||
|
|
||
| private final List<Boolean> groupSplitter; | ||
|
|
||
| private final List<Boolean> noMoreTsBlockForCurrentGroup; | ||
|
|
||
| private final Comparator<SortKey> groupKeyComparator; |
|
Thanks for the review! I have addressed the latest Copilot comments in this PR:
I also verified the related test with:
Result: |
Description
Add table-model NEXT fill syntax and planning
This PR implements
FILL METHOD NEXTfor the table model.NEXTfills a null value with the nearest non-null value that appears later in the current operator input order. It directly copies the future value, does not interpolate, and does not search by timestamp order. If no next value exists, the value remains null.The implementation supports:
FILL METHOD NEXTTIME_BOUNDTIME_COLUMNFILL_GROUPPlain
NEXTdoes not require a helper/time column. A helper/time column is only required or inferred whenTIME_BOUNDorFILL_GROUPneeds it.FillPolicy.NEXTis appended after existing enum values to keep existing byte/ordinal compatibility, andTABLE_NEXT_FILL_NODEuses a new plan node type id.Add NEXT fill plan node and optimizer support
This PR adds
NextFillNodeand wires it through serde, plan visitors, plan graph printing, pruning, symbol rewrite, analyzer, and planner logic.For
TIME_BOUNDorFILL_GROUP, the analyzer resolves the required helper/time column similarly toPREVIOUS. For grouped fill, the planner reuses the existing fill-group sorting and grouping-symbol path.PushLimitOffsetIntoTableScantreatsNEXTlikeLINEARand avoids pushing downLIMIT/OFFSET, because NEXT fill may need future rows that would otherwise be removed before filling.Add NEXT fill execution path
This PR adds table NEXT fill operators and the generated fill implementation.
The execution layer reuses
AbstractLinearFillOperatorfor cross-TsBlocklook-ahead caching and grouped slicing. The NEXT fill algorithm scans the current value column from right to left, uses the nearest non-null candidate in the current block when available, and otherwise uses the first non-null candidate found in later cached blocks.For grouped fill, group boundaries reset the next-value state so values are not filled across groups.
Tests
This PR adds and updates tests for:
FILL METHOD NEXTNextFillNodeserdeTIME_BOUNDTIME_COLUMNFILL_GROUPTIME_BOUND + TIME_COLUMN + FILL_GROUPTsBlockfill behaviorLIMITbehavior without losing future next valuesCloses #17798
This PR has:
Key changed/added classes (or packages if there are too many classes) in this PR
RelationalSql.g4FillPolicyFillAstBuilderStatementAnalyzerAnalysis.NextFillAnalysisQueryPlannerNextFillNodePlanNodeTypeCommonPlanNodeDeserializerICoreQueryPlanVisitorPlanGraphPrinterPruneFillColumnsPushLimitOffsetIntoTableScanUnaliasSymbolReferencesCommonOperatorUtilsTableOperatorGeneratorTableNextFillOperatorTableNextFillWithGroupOperatorfill/next/NextFillnextFill.ftlFillStatementTestNextFillNodeSerdeTestNextFillTestIoTDBFillTableIT