Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Iceberg]Refactor validation for SHOW CREATE TABLE for relevant tests #24524

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

hantangwangd
Copy link
Member

@hantangwangd hantangwangd commented Feb 10, 2025

Description

This PR refactor the validation logic for SHOW CREATE TABLE in Iceberg relevant tests.

This refactoring can make these relevant test cases less likely to be affected on adding or modifying table properties. Besides, it can make newly added test cases which have to validate the result of SHOW CREATE TABLE more concise.

Motivation and Context

Make existing test cases less likely to be affected on adding or modifying table properties. See the discussion here

Impact

N/A

Test Plan

This is a test refactoring.

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== NO RELEASE NOTE ==

@hantangwangd hantangwangd force-pushed the refactor_validate_show_create_table branch from 18f61ca to fd7acb4 Compare February 10, 2025 18:27
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull branch, local docs build, looks good. Thanks!

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.

2 participants