Skip to content

[CALCITE-3676] VolcanoPlanner. dumpGraphviz should handle exception#1759

Closed
XuQianJin-Stars wants to merge 1 commit intoapache:masterfrom
XuQianJin-Stars:CALCITE-3676
Closed

[CALCITE-3676] VolcanoPlanner. dumpGraphviz should handle exception#1759
XuQianJin-Stars wants to merge 1 commit intoapache:masterfrom
XuQianJin-Stars:CALCITE-3676

Conversation

@XuQianJin-Stars
Copy link
Copy Markdown
Contributor

As illustrated in CALCITE-3676

dumpGraphviz(pw);
}
} catch (Exception ex) {
throw new AssertionError("Dumps the internal state ", ex);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change this way, we won't see CannotPlanException

Copy link
Copy Markdown
Member

@hsyuan hsyuan Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shouldn't throw an exception or error here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@vlsi
Copy link
Copy Markdown
Contributor

vlsi commented Jan 15, 2020

-1: the change ignores the exception, so it would be hard to analyze failures in production

pw.println();
pw.println("Graphviz:");
dumpGraphviz(pw);
pw.print(ex);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change to

pw.println("Error when dumping plan state: \n" + e);

Copy link
Copy Markdown
Member

@hsyuan hsyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@hsyuan hsyuan closed this in 0f09ef9 Jan 27, 2020
hsyuan pushed a commit that referenced this pull request Jan 28, 2020
wangxlong pushed a commit to wangxlong/calcite that referenced this pull request Feb 13, 2020
jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Aug 28, 2025
apache#91)

…acefully (Qianjin Xu)

Close apache#1759

Co-authored-by: XuQianJin-Stars <x1q1j1@163.com>
jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Mar 16, 2026
apache#91)

…acefully (Qianjin Xu)

Close apache#1759

Co-authored-by: XuQianJin-Stars <x1q1j1@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants