GitHub user pfcoperez opened a pull request:
https://github.com/apache/spark/pull/17040
CatalogTableType constructor access modifier renders useless when case
class factory is public
## 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:
```
scala> case class C private(x: Int)
defined class C
scala> C(1)
res0: C = C(1) // Should users be able to do this?
scala> new C(1) //This is the expected behaviour.
<console>:13: error: constructor C in class C cannot be accessed in object
$iw
new C(1)
```
This PR makes `CatalogTableType` a regular class, if matching the name was
necessary, its companion object could implement `unapply`. Alternatives ways of
solving this could be just using Scala's enumeration or model
`CatalogTableType` using algebraic data types based on extending an interface,
e.g:
```
object CatalogTableType {
object EXTERNAL extends CatalogTableType { val name = "EXTERNAL" }
object INTERNAL extends CatalogTableType { val name = "INTERNAL" }
object VIEW extends CatalogTableType { val name = "VIEW" }
}
```
## How was this patch tested?
This PR does not add new functionality and has thus been tested by your
current test suite.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pfcoperez/spark
catalogtabletpe_private_factory
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/17040.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #17040
----
commit daae901d331293144af8dd125ea59892f5ec3c3d
Author: Pablo Fco. Pérez Hidalgo <[email protected]>
Date: 2017-02-23T12:18:06Z
Even though the case class constructor has been made private, the default
`apply` factory for the case class remains public so this constraint is useless
since anyone can use that factory to create customized instances.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
