You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "morrySnow (via GitHub)" <gi...@apache.org> on 2026/01/07 10:38:56 UTC
Re: [PR] [opt](mtmv) Optimize pre rbo query rewrite by materialized view performance to avoid fe oom and slow [doris]
morrySnow commented on code in PR #59154:
URL: https://github.com/apache/doris/pull/59154#discussion_r2667902528
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java:
##########
@@ -128,12 +132,28 @@ public int getGroupExpressionsSize() {
return groupExpressions.size();
}
- public long getRefreshVersion() {
- return refreshVersion.get();
+ /** get the refresh version map*/
+ public Map<Integer, AtomicInteger> getRefreshVersion() {
+ return refreshVersion;
}
- public long incrementAndGetRefreshVersion() {
- return refreshVersion.incrementAndGet();
+ /** return the incremented refresh version for the given commonTableId*/
+ public long incrementAndGetRefreshVersion(int commonTableId) {
+ return refreshVersion.compute(commonTableId, (k, v) -> {
+ if (v == null) {
+ return new AtomicInteger(1);
+ }
+ v.incrementAndGet();
+ return v;
Review Comment:
why not `return v.incrementAndGet()`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/StructInfoMap.java:
##########
@@ -51,80 +52,156 @@
public class StructInfoMap {
public static final Logger LOG = LogManager.getLogger(StructInfoMap.class);
+ // 2166136261
+ private static final int FNV32_OFFSET_BASIS = 0x811C9DC5;
+ // 16777619
+ private static final int FNV32_PRIME = 0x01000193;
/**
* The map key is the relation id bit set to get corresponding plan accurately
*/
- private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMap = new HashMap<>();
+ private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMapByRelationId = new HashMap<>();
/**
* The map key is the relation id bit set to get corresponding plan accurately
*/
- private final Map<BitSet, StructInfo> infoMap = new HashMap<>();
- private long refreshVersion = 0;
+ private final Map<BitSet, StructInfo> infoMapByRelationId = new HashMap<>();
+
+ /**
+ * The map key is the common table id bit set to get corresponding plan accurately
+ */
+ private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMapByTableId = new HashMap<>();
+ /**
+ * The map key is the common table id bit set to get corresponding plan accurately
+ */
+ private final Map<BitSet, StructInfo> infoMapByTableId = new HashMap<>();
+
+ // The key is the tableIds query used, the value is the refresh version when last refresh
+ private final Map<BitSet, Integer> refreshVersion = new HashMap<>();
/**
* get struct info according to table map
*
- * @param tableMap the original table map
+ * @param targetIdMap the original table map
* @param group the group that the mv matched
* @return struct info or null if not found
*/
- public @Nullable StructInfo getStructInfo(CascadesContext cascadesContext, BitSet tableMap, Group group,
- Plan originPlan, boolean forceRefresh) {
- StructInfo structInfo = infoMap.get(tableMap);
- if (structInfo != null) {
- return structInfo;
- }
- if (groupExpressionMap.isEmpty() || !groupExpressionMap.containsKey(tableMap)) {
- refresh(group, cascadesContext, tableMap, new HashSet<>(),
- forceRefresh);
- group.getStructInfoMap().setRefreshVersion(cascadesContext.getMemo().getRefreshVersion());
- }
- if (groupExpressionMap.containsKey(tableMap)) {
- Pair<GroupExpression, List<BitSet>> groupExpressionBitSetPair = getGroupExpressionWithChildren(tableMap);
- // NOTICE: During the transition from physicalAggregate to logical aggregation,
- // the original function signature needs to remain unchanged because the constructor of LogicalAggregation
- // will recalculate the signature of the aggregation function.
- // When the calculated signature is inconsistent with the original signature
- // (e.g. due to the influence of the session variable enable_decimal256),
- // a problem will arise where the output type of the rewritten plan is inconsistent with
- // the output type of the upper-level operator.
- structInfo = MoreFieldsThread.keepFunctionSignature(() ->
- constructStructInfo(groupExpressionBitSetPair.first, groupExpressionBitSetPair.second,
- originPlan, cascadesContext));
- infoMap.put(tableMap, structInfo);
+ public @Nullable StructInfo getStructInfo(CascadesContext cascadesContext, BitSet targetIdMap, Group group,
+ Plan originPlan, boolean forceRefresh, boolean tableIdMode) {
+ StructInfo structInfo;
+ if (tableIdMode) {
Review Comment:
maybe split two mode into two function is better to maintain and test
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/StructInfoMap.java:
##########
@@ -51,80 +52,156 @@
public class StructInfoMap {
public static final Logger LOG = LogManager.getLogger(StructInfoMap.class);
+ // 2166136261
+ private static final int FNV32_OFFSET_BASIS = 0x811C9DC5;
+ // 16777619
+ private static final int FNV32_PRIME = 0x01000193;
/**
* The map key is the relation id bit set to get corresponding plan accurately
*/
- private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMap = new HashMap<>();
+ private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMapByRelationId = new HashMap<>();
/**
* The map key is the relation id bit set to get corresponding plan accurately
*/
- private final Map<BitSet, StructInfo> infoMap = new HashMap<>();
- private long refreshVersion = 0;
+ private final Map<BitSet, StructInfo> infoMapByRelationId = new HashMap<>();
+
+ /**
+ * The map key is the common table id bit set to get corresponding plan accurately
+ */
+ private final Map<BitSet, Pair<GroupExpression, List<BitSet>>> groupExpressionMapByTableId = new HashMap<>();
+ /**
+ * The map key is the common table id bit set to get corresponding plan accurately
+ */
+ private final Map<BitSet, StructInfo> infoMapByTableId = new HashMap<>();
+
+ // The key is the tableIds query used, the value is the refresh version when last refresh
+ private final Map<BitSet, Integer> refreshVersion = new HashMap<>();
/**
* get struct info according to table map
*
- * @param tableMap the original table map
+ * @param targetIdMap the original table map
* @param group the group that the mv matched
* @return struct info or null if not found
*/
- public @Nullable StructInfo getStructInfo(CascadesContext cascadesContext, BitSet tableMap, Group group,
- Plan originPlan, boolean forceRefresh) {
- StructInfo structInfo = infoMap.get(tableMap);
- if (structInfo != null) {
- return structInfo;
- }
- if (groupExpressionMap.isEmpty() || !groupExpressionMap.containsKey(tableMap)) {
- refresh(group, cascadesContext, tableMap, new HashSet<>(),
- forceRefresh);
- group.getStructInfoMap().setRefreshVersion(cascadesContext.getMemo().getRefreshVersion());
- }
- if (groupExpressionMap.containsKey(tableMap)) {
- Pair<GroupExpression, List<BitSet>> groupExpressionBitSetPair = getGroupExpressionWithChildren(tableMap);
- // NOTICE: During the transition from physicalAggregate to logical aggregation,
- // the original function signature needs to remain unchanged because the constructor of LogicalAggregation
- // will recalculate the signature of the aggregation function.
- // When the calculated signature is inconsistent with the original signature
- // (e.g. due to the influence of the session variable enable_decimal256),
- // a problem will arise where the output type of the rewritten plan is inconsistent with
- // the output type of the upper-level operator.
- structInfo = MoreFieldsThread.keepFunctionSignature(() ->
- constructStructInfo(groupExpressionBitSetPair.first, groupExpressionBitSetPair.second,
- originPlan, cascadesContext));
- infoMap.put(tableMap, structInfo);
+ public @Nullable StructInfo getStructInfo(CascadesContext cascadesContext, BitSet targetIdMap, Group group,
+ Plan originPlan, boolean forceRefresh, boolean tableIdMode) {
+ StructInfo structInfo;
+ if (tableIdMode) {
+ structInfo = infoMapByTableId.get(targetIdMap);
+ if (structInfo != null) {
+ return structInfo;
+ }
+ if (groupExpressionMapByTableId.isEmpty() || !groupExpressionMapByTableId.containsKey(targetIdMap)) {
+ refresh(group, cascadesContext, targetIdMap, new HashSet<>(),
+ forceRefresh, Integer.MAX_VALUE, tableIdMode);
+ group.getStructInfoMap().setRefreshVersion(targetIdMap, cascadesContext.getMemo().getRefreshVersion());
+ }
+ if (groupExpressionMapByTableId.containsKey(targetIdMap)) {
+ Pair<GroupExpression, List<BitSet>> groupExpressionBitSetPair =
+ getGroupExpressionWithChildren(targetIdMap, tableIdMode);
+ // NOTICE: During the transition from physicalAggregate to logical aggregation,
+ // the original function signature needs to remain unchanged because the constructor
+ // of LogicalAggregation
+ // will recalculate the signature of the aggregation function.
+ // When the calculated signature is inconsistent with the original signature
+ // (e.g. due to the influence of the session variable enable_decimal256),
+ // a problem will arise where the output type of the rewritten plan is inconsistent with
+ // the output type of the upper-level operator.
+ structInfo = MoreFieldsThread.keepFunctionSignature(() ->
+ constructStructInfo(groupExpressionBitSetPair.first, groupExpressionBitSetPair.second,
+ originPlan, cascadesContext, tableIdMode));
+ infoMapByTableId.put(targetIdMap, structInfo);
+ }
+ } else {
+ structInfo = infoMapByRelationId.get(targetIdMap);
+ if (structInfo != null) {
+ return structInfo;
+ }
+ if (groupExpressionMapByRelationId.isEmpty() || !groupExpressionMapByRelationId.containsKey(targetIdMap)) {
+ int memoVersion = getMemoVersion(targetIdMap, cascadesContext.getMemo().getRefreshVersion());
+ refresh(group, cascadesContext, targetIdMap, new HashSet<>(), forceRefresh, memoVersion, tableIdMode);
+ group.getStructInfoMap().setRefreshVersion(targetIdMap, cascadesContext.getMemo().getRefreshVersion());
+ }
+ if (groupExpressionMapByRelationId.containsKey(targetIdMap)) {
+ Pair<GroupExpression, List<BitSet>> groupExpressionBitSetPair =
+ getGroupExpressionWithChildren(targetIdMap, tableIdMode);
+ // NOTICE: During the transition from physicalAggregate to logical aggregation,
+ // the original function signature needs to remain unchanged because the constructor
+ // of LogicalAggregation
+ // will recalculate the signature of the aggregation function.
+ // When the calculated signature is inconsistent with the original signature
+ // (e.g. due to the influence of the session variable enable_decimal256),
+ // a problem will arise where the output type of the rewritten plan is inconsistent with
+ // the output type of the upper-level operator.
+ structInfo = MoreFieldsThread.keepFunctionSignature(() ->
+ constructStructInfo(groupExpressionBitSetPair.first, groupExpressionBitSetPair.second,
+ originPlan, cascadesContext, tableIdMode));
+ infoMapByRelationId.put(targetIdMap, structInfo);
+ }
}
return structInfo;
}
- public Set<BitSet> getTableMaps() {
- return groupExpressionMap.keySet();
+ public Set<BitSet> getTableMaps(boolean tableIdMode) {
+ return tableIdMode ? groupExpressionMapByTableId.keySet() : groupExpressionMapByRelationId.keySet();
+ }
+
+ public Pair<GroupExpression, List<BitSet>> getGroupExpressionWithChildren(BitSet tableMap, boolean tableIdMode) {
+ if (tableIdMode) {
+ return groupExpressionMapByTableId.get(tableMap);
+ }
+ return groupExpressionMapByRelationId.get(tableMap);
+ }
+
+ // Set the refresh version for the given queryRelationIdSet
+ public void setRefreshVersion(BitSet queryRelationIdSet, Map<Integer, AtomicInteger> memoRefreshVersionMap) {
+ this.refreshVersion.put(queryRelationIdSet, getMemoVersion(queryRelationIdSet, memoRefreshVersionMap));
}
- public Pair<GroupExpression, List<BitSet>> getGroupExpressionWithChildren(BitSet tableMap) {
- return groupExpressionMap.get(tableMap);
+ // Set the refresh version for the given queryRelationIdSet
+ public void setRefreshVersion(BitSet queryRelationIdSet, int memoRefreshVersion) {
+ this.refreshVersion.put(queryRelationIdSet, memoRefreshVersion);
}
- public void setRefreshVersion(long refreshVersion) {
- this.refreshVersion = refreshVersion;
+ // Get the refresh version for the given queryRelationIdSet, if not exist, return 0
+ public long getRefreshVersion(BitSet queryRelationIdSet) {
+ return refreshVersion.computeIfAbsent(queryRelationIdSet, k -> 0);
+ }
+
+ /** Get the memo version among the relation ids in queryRelationIdSet*/
+ public static int getMemoVersion(BitSet queryRelationIdSet, Map<Integer, AtomicInteger> memoRefreshVersionMap) {
+ int hash = FNV32_OFFSET_BASIS;
Review Comment:
could u explain the hash algorithm in comment?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/memo/StructInfoMap.java:
##########
@@ -186,41 +273,72 @@ public void refresh(Group group, CascadesContext cascadesContext,
eachGroupExpressionTableSet.or(bitSet);
}
}
- if (groupExpressionMap.containsKey(eachGroupExpressionTableSet)) {
- // for the group expressions of group, only need to refresh any of the group expression
- // when they have the same group expression table set
- continue;
+ if (tableIdMode) {
+ if (groupExpressionMapByTableId.containsKey(eachGroupExpressionTableSet)) {
+ // for the group expressions of group, only need to refresh any of the group expression
+ // when they have the same group expression table set
+ continue;
+ }
+ } else {
+ if (groupExpressionMapByRelationId.containsKey(eachGroupExpressionTableSet)) {
+ // for the group expressions of group, only need to refresh any of the group expression
+ // when they have the same group expression table set
+ continue;
+ }
}
// if cumulative child table map is different from current
// or current group expression map is empty, should update the groupExpressionMap currently
Collection<Pair<BitSet, List<BitSet>>> bitSetWithChildren = cartesianProduct(childrenTableMap);
- for (Pair<BitSet, List<BitSet>> bitSetWithChild : bitSetWithChildren) {
- groupExpressionMap.putIfAbsent(bitSetWithChild.first,
- Pair.of(groupExpression, bitSetWithChild.second));
+ if (tableIdMode) {
+ for (Pair<BitSet, List<BitSet>> bitSetWithChild : bitSetWithChildren) {
+ groupExpressionMapByTableId.putIfAbsent(bitSetWithChild.first,
+ Pair.of(groupExpression, bitSetWithChild.second));
+ }
+ } else {
+ for (Pair<BitSet, List<BitSet>> bitSetWithChild : bitSetWithChildren) {
+ groupExpressionMapByRelationId.putIfAbsent(bitSetWithChild.first,
+ Pair.of(groupExpression, bitSetWithChild.second));
+ }
}
-
}
}
private BitSet constructLeaf(GroupExpression groupExpression, CascadesContext cascadesContext,
- boolean forceRefresh) {
+ boolean forceRefresh, boolean tableIdMode) {
Plan plan = groupExpression.getPlan();
BitSet tableMap = new BitSet();
- if (plan instanceof LogicalCatalogRelation) {
- LogicalCatalogRelation logicalCatalogRelation = (LogicalCatalogRelation) plan;
- TableIf table = logicalCatalogRelation.getTable();
- // If disable materialized view nest rewrite, and mv already rewritten successfully once, doesn't construct
- // table id map for nest mv rewrite
- if (!forceRefresh && cascadesContext.getStatementContext()
- .getMaterializationRewrittenSuccessSet().contains(table.getFullQualifiers())) {
- return tableMap;
- }
- tableMap.set(logicalCatalogRelation.getRelationId().asInt());
- }
- // one row relation / CTE consumer
- if (plan instanceof LogicalCTEConsumer || plan instanceof LogicalEmptyRelation
- || plan instanceof LogicalOneRowRelation) {
- tableMap.set(((LogicalRelation) plan).getRelationId().asInt());
+ if (tableIdMode) {
Review Comment:
there are too many functions have the if else for tableIdMode, i think u should refactor these class for better maintainability
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org