Skip to content

Conversation

@hqbhoho
Copy link
Contributor

@hqbhoho hqbhoho commented Nov 28, 2025

Description

Add missing defaultValue for ColumnMetadata.Builder

Release notes

(X) This is not user-visible or is docs only, and no release notes are required.

{
this.name = columnMetadata.getName();
this.type = columnMetadata.getType();
this.defaultValue = columnMetadata.getDefaultValue();
Copy link
Member

Choose a reason for hiding this comment

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

just this.defaultValue = columnMetadata.getDefaultValue(); is enough (pushed a fix to your branch)

can you please add a test that Builder(ColumnMetada).build() roundtrips?

Comment on lines 29 to 38
ColumnMetadata originColumnMetadata = ColumnMetadata.builder()
.setName("test_column")
.setType(INTEGER)
.setDefaultValue(Optional.of("1"))
.setNullable(false)
.setComment(Optional.ofNullable("test_comment"))
.setExtraInfo(Optional.of("test_extra_info"))
.setHidden(false)
.setProperties(ImmutableMap.of("test_key", "test_value"))
.build();
Copy link
Member

Choose a reason for hiding this comment

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

Make ColumnMetadata constructor package private, @VisibleForTesting and call here.
This would ensure the test stays up to date, should there any new fields be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, @VisibleForTesting will need guava dependency. I add a doc rather than add dependency. what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants