-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Pipe: support multiple path patterns under tree model #16575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Enable Pipe to accept multiple tree path patterns by introducing a union pattern representation and updating parsing, selection, and tests.
- Add UnionTreePattern to represent a union of multiple TreePattern instances and extend parser to build unions from comma-separated inputs (backtick-aware).
- Update parsing semantics to unionize source.path and source.pattern when both are provided; adjust logic to avoid scan parsers for unions.
- Add integration and unit tests covering multiple prefix/IoTDB/hybrid patterns for historical and realtime data; minor test name fixes.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/source/IoTDBNonDataRegionSource.java | Temporarily unwraps UnionTreePattern to first pattern, affecting multi-pattern support. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/UnionTreePattern.java | New union pattern type and utilities (toString, matching, etc.). |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java | Refactor to parse multiple patterns, introduce helper methods, and move single-pattern state to SingleTreePattern. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/SingleTreePattern.java | New base class encapsulating single-pattern behavior and defaults. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/PrefixTreePattern.java | Switch to extend SingleTreePattern. |
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/IoTDBTreePattern.java | Switch to extend SingleTreePattern. |
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/source/IoTDBDataRegionSourceTest.java | Add assertions for comma-separated patterns and backtick-escaped commas. |
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/tsfile/parser/TsFileInsertionEventParserProvider.java | Avoid scan parser paths when a UnionTreePattern is used. |
integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/auto/basic/IoTDBTreePatternFormatIT.java | Add integration tests for multiple patterns; fix method names and semantics where both path and pattern are provided. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ode-commons/src/main/java/org/apache/iotdb/commons/pipe/source/IoTDBNonDataRegionSource.java
Outdated
Show resolved
Hide resolved
...e-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java
Show resolved
Hide resolved
...mons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/UnionTreePattern.java
Outdated
Show resolved
Hide resolved
...mons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/UnionTreePattern.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.../main/java/org/apache/iotdb/db/pipe/source/schemaregion/PipePlanTreePatternParseVisitor.java
Outdated
Show resolved
Hide resolved
...e-commons/src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/TreePattern.java
Show resolved
Hide resolved
|
||
// The statements do not contain AlterLogicalViewStatements | ||
// Here we apply the statements as many as possible | ||
// Even if there are failed statements |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This unchecked cast assumes the built TreePattern is always a UnionIoTDBTreePattern. To make this robust, either construct a UnionIoTDBTreePattern directly (e.g., map to List and new UnionIoTDBTreePattern(...)) or guard with an instanceof check and fail fast with a clear error if a non-IoTDB union is received.
// Even if there are failed statements | |
// Even if there are failed statements | |
if (!(treePattern instanceof UnionIoTDBTreePattern)) { | |
throw new IllegalArgumentException( | |
"Expected treePattern to be UnionIoTDBTreePattern, but got: " | |
+ treePattern.getClass().getName()); | |
} |
Copilot uses AI. Check for mistakes.
...java/org/apache/iotdb/confignode/manager/pipe/receiver/protocol/IoTDBConfigNodeReceiver.java
Show resolved
Hide resolved
...src/main/java/org/apache/iotdb/commons/pipe/datastructure/pattern/UnionIoTDBTreePattern.java
Outdated
Show resolved
Hide resolved
...anode/src/test/java/org/apache/iotdb/db/pipe/source/PipePlanTreePatternParseVisitorTest.java
Show resolved
Hide resolved
...de/src/test/java/org/apache/iotdb/db/pipe/sink/PipeStatementTreePatternParseVisitorTest.java
Show resolved
Hide resolved
.../iotdb/confignode/manager/pipe/source/PipeConfigPhysicalPlanTreePatternParseVisitorTest.java
Show resolved
Hide resolved
/ PipeDataNodeResourceManager.memory().getTotalNonFloatingMemorySizeInBytes() | ||
> PipeTsFilePublicResource.MEMORY_SUFFICIENT_THRESHOLD | ||
&& treePattern.isSingle()) { | ||
return new TsFileInsertionEventScanParser( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "ScanParser" must use single?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether the implementation of scan parser will conflict with multi patterns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely not. Just check whether the device or measurement fits the pattern one by one.
...-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipeInclusionIT.java
Outdated
Show resolved
Hide resolved
...-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipeInclusionIT.java
Outdated
Show resolved
Hide resolved
...anode/src/test/java/org/apache/iotdb/db/pipe/source/PipePlanTreePatternParseVisitorTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
reopen #16435