CatalogTableType constructor access modifier renders useless when case class factory is public #17040
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
CatalogTableType
seems to implement an union data type enumerating all possible catalog table types.It is built using a case class with a single value attribute "name". Wisely, it seems to be intended to be set only by the case class private constructor within the companion object with the idea of users not being able to create
CatalogTableType
instances but using those enumerated within that object.However, making the case class constructor private does't forbid users creating whichever instances they want to create since the case class factory is still public. e.g:
This PR makes
CatalogTableType
a regular class, if matching the name was necessary, its companion object could implementunapply
. Alternatives ways of solving this could be just using Scala's enumeration or modelCatalogTableType
using algebraic data types based on extending an interface, e.g:How was this patch tested?
This PR does not add new functionality and has thus been tested by your current test suite.