szehon-ho commented on code in PR #54247:
URL: https://github.com/apache/spark/pull/54247#discussion_r2791679687
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Implicits.scala:
##########
@@ -166,9 +166,10 @@ private[sql] object CatalogV2Implicits {
def asMultipartIdentifier: Seq[String] = (ident.namespace :+
ident.name).toImmutableArraySeq
def asTableIdentifier: TableIdentifier = ident.namespace match {
- case ns if ns.isEmpty => TableIdentifier(ident.name)
Review Comment:
curious here, is it necessary to remove these if ns is not empty? I see
that asTableIdentifierOpt still handles the empty ns case.
I assume in V1SessionCatalog case, the namespace is never empty, but just
checking if its necessary to remove, in case some v2 connector is using the
method
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala:
##########
@@ -1162,8 +1162,8 @@ class V2SessionCatalogNamespaceSuite extends
V2SessionCatalogBaseSuite {
val testIdent: IdentifierHelper = Identifier.of(Array("a", "b"), "c")
checkError(
exception = intercept[AnalysisException](testIdent.asTableIdentifier),
- condition = "IDENTIFIER_TOO_MANY_NAME_PARTS",
- parameters = Map("identifier" -> "`a`.`b`.`c`", "limit" -> "2")
+ condition = "REQUIRES_SINGLE_PART_NAMESPACE",
Review Comment:
just opinion that previously having the identifier was a bit easier to
debug. I wonder if we can extend the error message to have a version with
identifier?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala:
##########
@@ -698,12 +698,12 @@ class SparkSqlAstBuilder extends AstBuilder {
}
withIdentClause(ctx.identifierReference(), Seq(qPlan), (ident,
otherPlans) => {
- val tableIdentifier = ident.asTableIdentifier
- if (tableIdentifier.database.isDefined) {
+ if (ident.length > 1) {
Review Comment:
is this case covered by some test? (its easy to understand though, so not
blocking)
--
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]
