924060929 commented on code in PR #47066:
URL: https://github.com/apache/doris/pull/47066#discussion_r1921736314
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -350,13 +346,35 @@ private boolean tablesOrDataChanged(Env env,
SqlCacheContext sqlCacheContext) {
return true;
}
OlapTable olapTable = (OlapTable) tableIf;
- Collection<Long> partitionIds = scanTable.getScanPartitions();
- olapTable.getVersionInBatchForCloudMode(partitionIds);
-
- for (Long scanPartitionId : scanTable.getScanPartitions()) {
- Partition partition = olapTable.getPartition(scanPartitionId);
+ List<Column> currentFullSchema = olapTable.getFullSchema();
+ List<SqlCacheContext.ColumnSection> cacheFullSchema =
scanTable.latestColumns;
+ if (currentFullSchema.size() != cacheFullSchema.size()) {
+ return true;
+ }
+ for (int i = 0; i < currentFullSchema.size(); i++) {
+ Column currentColumn = currentFullSchema.get(i);
+ SqlCacheContext.ColumnSection cacheColumn =
cacheFullSchema.get(i);
+ if ((currentColumn == null || cacheColumn == null)
+ || (currentColumn.hashCode() !=
cacheColumn.getHashCode())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getName(),
cacheColumn.getName())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getType().toString(),
cacheColumn.getType())) {
+ return true;
+ }
+ }
Review Comment:
1. You should move to the previous loop of `sqlCacheContext.getUsedTables()`
because
```
// the query maybe scan empty partition of the table, we should
check these table version too,
// but the table not exists in sqlCacheContext.getScanTables(), so
we need check here.
// check table type and version
```
2. If we already check the columns for table version `x`, can we skip check
columns next time if the table version is `x`?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/SqlCacheContext.java:
##########
@@ -484,4 +500,12 @@ public enum CacheKeyType {
// use MD5 as Cache key in FE
MD5
}
+
+ @lombok.Data
+ @lombok.AllArgsConstructor
+ public static class ColumnSection {
+ private int hashCode;
+ private String name;
+ private String type;
+ }
Review Comment:
Maybe you should direct use org.apache.doris.catalog.Column
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -350,13 +346,35 @@ private boolean tablesOrDataChanged(Env env,
SqlCacheContext sqlCacheContext) {
return true;
}
OlapTable olapTable = (OlapTable) tableIf;
- Collection<Long> partitionIds = scanTable.getScanPartitions();
- olapTable.getVersionInBatchForCloudMode(partitionIds);
-
- for (Long scanPartitionId : scanTable.getScanPartitions()) {
- Partition partition = olapTable.getPartition(scanPartitionId);
+ List<Column> currentFullSchema = olapTable.getFullSchema();
+ List<SqlCacheContext.ColumnSection> cacheFullSchema =
scanTable.latestColumns;
+ if (currentFullSchema.size() != cacheFullSchema.size()) {
+ return true;
+ }
+ for (int i = 0; i < currentFullSchema.size(); i++) {
+ Column currentColumn = currentFullSchema.get(i);
+ SqlCacheContext.ColumnSection cacheColumn =
cacheFullSchema.get(i);
+ if ((currentColumn == null || cacheColumn == null)
+ || (currentColumn.hashCode() !=
cacheColumn.getHashCode())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getName(),
cacheColumn.getName())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getType().toString(),
cacheColumn.getType())) {
+ return true;
+ }
+ }
+ Map<Long, Partition> partitionMap =
olapTable.getAllPartitions().stream()
+ .collect(Collectors.toMap(Partition::getId,
Function.identity()));
Review Comment:
it seems expensive, why not use OlapTable.idToPartition
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -350,13 +346,35 @@ private boolean tablesOrDataChanged(Env env,
SqlCacheContext sqlCacheContext) {
return true;
}
OlapTable olapTable = (OlapTable) tableIf;
- Collection<Long> partitionIds = scanTable.getScanPartitions();
- olapTable.getVersionInBatchForCloudMode(partitionIds);
-
- for (Long scanPartitionId : scanTable.getScanPartitions()) {
- Partition partition = olapTable.getPartition(scanPartitionId);
+ List<Column> currentFullSchema = olapTable.getFullSchema();
+ List<SqlCacheContext.ColumnSection> cacheFullSchema =
scanTable.latestColumns;
+ if (currentFullSchema.size() != cacheFullSchema.size()) {
+ return true;
+ }
+ for (int i = 0; i < currentFullSchema.size(); i++) {
+ Column currentColumn = currentFullSchema.get(i);
+ SqlCacheContext.ColumnSection cacheColumn =
cacheFullSchema.get(i);
+ if ((currentColumn == null || cacheColumn == null)
+ || (currentColumn.hashCode() !=
cacheColumn.getHashCode())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getName(),
cacheColumn.getName())) {
+ return true;
+ }
+ if (!Objects.equals(currentColumn.getType().toString(),
cacheColumn.getType())) {
+ return true;
+ }
+ }
+ Map<Long, Partition> partitionMap =
olapTable.getAllPartitions().stream()
+ .collect(Collectors.toMap(Partition::getId,
Function.identity()));
+ Map<Long, Long> scanPartitionIdToVersion =
scanTable.scanPartitionIdToVersion;
+ for (Entry<Long, Long> entry :
scanPartitionIdToVersion.entrySet()) {
+ Long scanPartitionId = entry.getKey();
+ Long cachePartitionVersion = entry.getValue();
+ Partition currentPartition = partitionMap.get(scanPartitionId);
// partition == null: is this partition truncated?
- if (partition == null) {
+ if (currentPartition == null
+ || currentPartition.getVisibleVersion() >
cachePartitionVersion) {
Review Comment:
change to `currentPartition.getVisibleVersion() != cachePartitionVersion`
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
