[cdc-common] add PublicEvolving annotation to SchemaUtils.#2756
[cdc-common] add PublicEvolving annotation to SchemaUtils.#2756PatrickRen merged 2 commits intoapache:masterfrom lvyanquan0624:schema-util
Conversation
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(columns, primaryKeys, options, comment, nameToColumns); | ||
| return Objects.hash(columns, primaryKeys, options, comment); |
There was a problem hiding this comment.
remove it because it's initialized lazily.
PatrickRen
left a comment
PatrickRen
left a comment
There was a problem hiding this comment.
@lvyanquan Thanks for the PR! I left some comments.
| import java.util.stream.Collectors; | ||
|
|
||
| /** Utils for {@link Schema} to perform the ability of evolution. */ | ||
| @PublicEvolving |
There was a problem hiding this comment.
As we are promoting this utility to public, I think we need to be careful about the API, so I have concerns about the method applySchemaChangeEvent with parameter ignoreException. I prefer to remove this one, as I can't see any use cases for ignoring exceptions. Currently there's no usage of it, and the warning log is empty if the exception is ignored, which confuses me a bit.
A better behavior would be clearly defining the exception on the method (throws XXXException) and letting the caller catch and handle it.
|
Address it. And XXXException requires unified processing, so just keep to use IllegalArgumentException here. |
PatrickRen
left a comment
PatrickRen
left a comment
There was a problem hiding this comment.
@lvyanquan Thanks for the update! LGTM.
2 participants
add PublicEvolving annotation to SchemaUtils, add modify code of SchemaManager to reuse the method of SchemaUtils.
@PatrickRen CC.