morrySnow commented on code in PR #20716:
URL: https://github.com/apache/doris/pull/20716#discussion_r1268314793
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/cost/CostModelV1.java:
##########
@@ -65,7 +65,16 @@ class CostModelV1 extends PlanVisitor<Cost, PlanContext> {
// the penalty factor is no more than BROADCAST_JOIN_SKEW_PENALTY_LIMIT
static final double BROADCAST_JOIN_SKEW_RATIO = 30.0;
static final double BROADCAST_JOIN_SKEW_PENALTY_LIMIT = 2.0;
- private int beNumber = Math.max(1,
ConnectContext.get().getEnv().getClusterInfo().getBackendsNumber(true));
+ private int beNumber = 1;
+
+ public CostModelV1() {
+ if (ConnectContext.get().getSessionVariable().isPlayNereidsDump()) {
+ beNumber = ConnectContext.get().getSessionVariable().getBeNumber();
+ } else {
+ beNumber = Math.max(1,
ConnectContext.get().getEnv().getClusterInfo().getBackendsNumber(true));
+ ConnectContext.get().getSessionVariable().setBeNumber(beNumber);
Review Comment:
should not change session variable anytime
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -978,6 +984,9 @@ public void setMaxJoinNumBushyTree(int maxJoinNumBushyTree)
{
@VariableMgr.VarAttr(name = ENABLE_FOLD_NONDETERMINISTIC_FN)
public boolean enableFoldNondeterministicFn = true;
+ @VariableMgr.VarAttr(name = MINIDUMP_PATH)
+ public String minidumpPath = "default";
Review Comment:
nereids_minidump_path, btw use empty string to present default maybe better
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java:
##########
@@ -136,6 +136,9 @@ private LogicalPlan bindWithCurrentDb(CascadesContext
cascadesContext, UnboundRe
TableIf table = null;
if (cascadesContext.getTables() != null) {
table = cascadesContext.getTableByName(tableName);
+ if (ConnectContext.get().getSessionVariable().isPlayNereidsDump()
&& table == null) {
+ throw new AnalysisException("Can not find table:" + tableName);
+ }
Review Comment:
if getTableByName only used for minidump, please throw exception in it self
and change the method name to reflect it used for minidump
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -549,24 +548,26 @@ private Statistics computeFilter(Filter filter) {
}
private ColumnStatistic getColumnStatistic(TableIf table, String colName) {
- if (totalColumnStatisticMap.get(table.getName() + colName) != null) {
- return totalColumnStatisticMap.get(table.getName() + colName);
- } else if (isPlayNereidsDump) {
- return ColumnStatistic.UNKNOWN;
- } else {
- long catalogId;
- long dbId;
- try {
- catalogId = table.getDatabase().getCatalog().getId();
- dbId = table.getDatabase().getId();
- } catch (Exception e) {
- // Use -1 for catalog id and db id when failed to get them
from metadata.
- // This is OK because catalog id and db id is not in the
hashcode function of ColumnStatistics cache
- // and the table id is globally unique.
- LOG.debug(String.format("Fail to get catalog id and db id for
table %s", table.getName()));
- catalogId = -1;
- dbId = -1;
+ long catalogId;
+ long dbId;
+ try {
+ catalogId = table.getDatabase().getCatalog().getId();
+ dbId = table.getDatabase().getId();
+ } catch (Exception e) {
+ // Use -1 for catalog id and db id when failed to get them from
metadata.
+ // This is OK because catalog id and db id is not in the hashcode
function of ColumnStatistics cache
+ // and the table id is globally unique.
+ LOG.debug(String.format("Fail to get catalog id and db id for
table %s", table.getName()));
+ catalogId = -1;
+ dbId = -1;
+ }
+ if (isPlayNereidsDump) {
Review Comment:
i think the better way to do this is put all column stats in dump file into
cache first. and then we don't need to change any of the code here
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/TopicRewriteJob.java:
##########
@@ -51,4 +51,9 @@ public void execute(JobContext jobContext) {
public boolean isOnce() {
return true;
}
+
+ @Override
+ public String getRuleType() {
Review Comment:
why add this interface? u do not use it in anyplace
##########
minidump/nereids_ut.sh:
##########
Review Comment:
add readme
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/minidump/MinidumpUtTest.java:
##########
@@ -0,0 +1,43 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.minidump;
+
+import org.json.JSONObject;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+class MinidumpUtTest {
+
+ @Test
+ public void testMinidumpUt() {
Review Comment:
add comment to explain how to use it
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
