morrySnow commented on code in PR #49415:
URL: https://github.com/apache/doris/pull/49415#discussion_r2024181635
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java:
##########
@@ -121,11 +123,23 @@ public class ExpressionEstimation extends
ExpressionVisitor<ColumnStatistic, Sta
* returned columnStat is newly created or a copy of stats
*/
public static ColumnStatistic estimate(Expression expression, Statistics
stats) {
- ColumnStatistic columnStatistic = expression.accept(INSTANCE, stats);
- if (columnStatistic == null) {
- return ColumnStatistic.UNKNOWN;
+ try {
+ ColumnStatistic columnStatistic = expression.accept(INSTANCE,
stats);
+ if (columnStatistic == null) {
+ return ColumnStatistic.UNKNOWN;
Review Comment:
why here not use withAvgSizeByte?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/StatsCalculator.java:
##########
@@ -301,13 +301,33 @@ public static void estimate(GroupExpression
groupExpression, CascadesContext con
private void estimate() {
Plan plan = groupExpression.getPlan();
- Statistics newStats = plan.accept(this, null);
+ Statistics newStats;
+ try {
+ newStats = plan.accept(this, null);
+ } catch (Exception e) {
+ // throw exception in debug mode
+ if (ConnectContext.get() != null &&
ConnectContext.get().getSessionVariable().feDebug) {
+ throw e;
+ }
+ LOG.warn("stats calculation failed, plan " + plan.treeString(), e);
Review Comment:
why need to print all plan tree? it is very expensive
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java:
##########
@@ -121,11 +123,23 @@ public class ExpressionEstimation extends
ExpressionVisitor<ColumnStatistic, Sta
* returned columnStat is newly created or a copy of stats
*/
public static ColumnStatistic estimate(Expression expression, Statistics
stats) {
- ColumnStatistic columnStatistic = expression.accept(INSTANCE, stats);
- if (columnStatistic == null) {
- return ColumnStatistic.UNKNOWN;
+ try {
+ ColumnStatistic columnStatistic = expression.accept(INSTANCE,
stats);
+ if (columnStatistic == null) {
+ return ColumnStatistic.UNKNOWN;
+ }
+ return columnStatistic;
+ } catch (Exception e) {
+ // in regression test, feDebug is true so that the exception is
thrown in order to detect problems.
+ if (ConnectContext.get() != null &&
ConnectContext.get().getSessionVariable().feDebug) {
Review Comment:
log here
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java:
##########
Review Comment:
could we add some ut to check return column statistic is as expected when
exception thrown?
--
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]
