dtenedor commented on code in PR #41549:
URL: https://github.com/apache/spark/pull/41549#discussion_r1227099188
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1401,12 +1399,29 @@ trait TableSpec {
case class UnresolvedTableSpec(
properties: Map[String, String],
provider: Option[String],
+ optionExpression: OptionList,
location: Option[String],
comment: Option[String],
serde: Option[SerdeInfo],
- external: Boolean) extends TableSpec {
+ external: Boolean) extends UnaryExpression with Unevaluable with TableSpec
{
+
+ override def dataType: DataType =
+ throw new UnsupportedOperationException("UnresolvedTableSpec doesn't have
a data type")
+
+ override def options: Map[String, String] =
+ throw new UnsupportedOperationException("Can't access options of
UnresolvedTableSpec")
Review Comment:
This PR generally LGTM! My only question is, do we need the `options` to
exist in the base class `TableSpec` and override that here in the
`UnresolvedTableSpec` to throw an error? From this PR it looks like the only
place we call the base class `TableSpec` `options` list is here:
```
/** Returns a string representing the arguments to this node, minus any
children */
def argString(maxFields: Int): String = stringArgs.flatMap {
...
case t: ResolvedTableSpec =>
t.copy(properties = Utils.redact(t.properties).toMap,
options = Utils.redact(t.options).toMap) :: Nil
...
```
If we restore these two deleted lines in this PR, could we also remove the
base `options` and this override?
```
case t: UnresolvedTableSpec =>
t.copy(properties = Utils.redact(t.properties).toMap) :: Nil
```
I am just wondering because this may be safer to prevent accidental calls to
this base class `options` list in the future when we think that the `TableSpec`
is always going to be resolved, but in some corner case it is not, and we get a
confusing error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
