Skip to content

feat: Ensure explain format in config is valid - #17549

Merged
Jefffrey merged 4 commits into
apache:mainfrom
Weijun-H:17498-support-explain-format-validation
Sep 14, 2025
Merged

feat: Ensure explain format in config is valid#17549
Jefffrey merged 4 commits into
apache:mainfrom
Weijun-H:17498-support-explain-format-validation

Conversation

@Weijun-H

@Weijun-HWeijun-H commented Sep 13, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

  • move ExplainFormat to common crate
  • change explain.format from string to ExplainFormat

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actionsgithub-actionsBot added sql SQL Planner logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) common Related to common crate labels Sep 13, 2025
@Weijun-HWeijun-H changed the title feat: Validate explain formatfeat: Validate explain format in the configSep 13, 2025
@Weijun-HWeijun-H changed the title feat: Validate explain format in the configfeat: Ensure explain format in config is validSep 13, 2025
@github-actionsgithub-actionsBot added the documentation Improvements or additions to documentation label Sep 13, 2025
@github-actionsgithub-actionsBot removed the documentation Improvements or additions to documentation label Sep 13, 2025
Jefffrey
Jefffrey approved these changes Sep 13, 2025
View reviewed changes

@JefffreyJefffrey left a comment

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.

LGTM 👍

2010YOUY01
2010YOUY01 approved these changes Sep 14, 2025
View reviewed changes

@2010YOUY012010YOUY01 left a comment

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.

This is a really good pattern. I think this should be applied to all existing string type configurations.

@Jefffrey
Jefffrey merged commit e2c2d38 into apache:mainSep 14, 2025
28 checks passed
@xudong963

xudong963 commented Sep 15, 2025•
edited
Loading

Copy link
Copy Markdown
Member

@Weijun-H Thank you for the quick improvement

@petern48petern48 mentioned this pull request Sep 19, 2025
Open
This was referenced Dec 26, 2025
Closed
Merged
@shinzoxDshinzoxD mentioned this pull request Aug 15, 2026
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JefffreyJefffreyJefffrey approved these changes
@2010YOUY012010YOUY012010YOUY01 approved these changes

Assignees

No one assigned

Labels

commonRelated to common cratelogical-exprLogical plan and expressionssqlSQL PlannersqllogictestSQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Weijun-H@xudong963@Jefffrey@2010YOUY01