JackieTien97 commented on code in PR #14398:
URL: https://github.com/apache/iotdb/pull/14398#discussion_r1891288564
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java:
##########
@@ -458,27 +457,29 @@ public TSStatus setTimePartitionInterval(
*/
public synchronized void adjustMaxRegionGroupNum() {
// Get all DatabaseSchemas
- Map<String, TDatabaseSchema> databaseSchemaMap =
- getMatchedDatabaseSchemasByName(getDatabaseNames(null));
+ // TODO
Review Comment:
TODO for what? now we've already forbidden adding new todo in pr, there
should already exist compile check for this in CI.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -1331,15 +1355,10 @@ public TSStatus commitDeleteColumn(final
CommitDeleteColumnPlan plan) {
}
}
- private PartialPath getQualifiedDatabasePartialPath(final String database)
- throws IllegalPathException {
- return
PartialPath.getDatabasePath(PathUtils.qualifyDatabaseName(database));
- }
-
// endregion
@TestOnly
public void clear() {
- mTree.clear();
+ treeModelMTree.clear();
Review Comment:
better clear both
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -481,9 +485,18 @@ public TSStatus adjustMaxRegionGroupCount(final
AdjustMaxRegionGroupNumPlan plan
public List<String> getDatabaseNames(final Boolean isTableModel) {
databaseReadWriteLock.readLock().lock();
try {
- return mTree.getAllDatabasePaths(isTableModel).stream()
- .map(PartialPath::getFullPath)
- .collect(Collectors.toList());
+ final List<String> results = new ArrayList<>();
+ if (!Boolean.TRUE.equals(isTableModel)) {
Review Comment:
better add some comments or java doc for this method, it seems that if
isTableModel is null, both if will match, is that really you want? if so,
explain why
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -534,19 +549,15 @@ public TDatabaseSchema
getMatchedDatabaseSchemaByName(final String database)
* @return All DatabaseSchemas that matches to the specific Database patterns
*/
public Map<String, TDatabaseSchema> getMatchedDatabaseSchemasByName(
- final List<String> rawPathList) {
+ final List<String> rawPathList, final Boolean isTableModel) {
final Map<String, TDatabaseSchema> schemaMap = new HashMap<>();
databaseReadWriteLock.readLock().lock();
try {
- for (final String rawPath : rawPathList) {
- final PartialPath patternPath =
getQualifiedDatabasePartialPath(rawPath);
- final List<PartialPath> matchedPaths =
- mTree.getMatchedDatabases(patternPath, ALL_MATCH_SCOPE, false);
- for (final PartialPath path : matchedPaths) {
- schemaMap.put(
- path.getFullPath(),
-
mTree.getDatabaseNodeByPath(path).getAsMNode().getDatabaseSchema());
- }
+ if (!Boolean.FALSE.equals(isTableModel)) {
+ enrichSchemaMap(rawPathList, tableModelMTree, schemaMap);
+ }
+ if (!Boolean.TRUE.equals(isTableModel)) {
+ enrichSchemaMap(rawPathList, treeModelMTree, schemaMap);
Review Comment:
better add some comments or java doc for this method, it seems that if
`isTableModel` is null, both if will match, is that really you want? if so,
explain why
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/event/common/PipeInsertionEvent.java:
##########
@@ -97,10 +98,7 @@ public String getTreeModelDatabaseName() {
public String getTableModelDatabaseName() {
return tableModelDatabaseName == null
- ? tableModelDatabaseName =
- treeModelDatabaseName != null &&
treeModelDatabaseName.startsWith("root.")
- ? treeModelDatabaseName.substring(5)
- : treeModelDatabaseName
+ ? tableModelDatabaseName =
PathUtils.unQualifyDatabaseName(treeModelDatabaseName)
Review Comment:
need to be confirmed by @SteveYurongSu
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/utils/SchemaRegionSnapshotParserTest.java:
##########
@@ -488,42 +489,42 @@ public void testComplicatedSnapshotParser() throws
Exception {
// ----------------------------------------------------------------------
// Schema Tree
// ----------------------------------------------------------------------
- // This test will construct a complicated mtree. This tree will have
- // aligned timeseries, tags and attributes, normal timeseries device
template.
+ // This test will construct a complicated mTree. This tree will have
+ // aligned timeSeries, tags and attributes, normal timeSeries device
template.
//
//
//
- // status(BOOLEAN, RLE) alias(stat)
- // /
- // t2------temperature(INT64, TS_2DIFF,LZ4)
- // /
- // sg1------s1------t1(activate template: t1)
- // /
- // root ->|
- // \
- // sg2-------t1(aligned)------status(INT64, TS_2DIFF,
LZMA2){attr1:atr1}
- // \
- // t2-------level{tags:"tag1"="t1", attributes:
"attri1"="attr1"}
+ // status(BOOLEAN, RLE) alias(stat)
+ // /
+ // t2------temperature(INT64, TS_2DIFF,LZ4)
+ // /
+ // sg1------s1------t1(activate template: t1)
+ // /
+ // root -> db ->|
Review Comment:
why do this change?
--
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]
