Skip to content

Commit dd5b799

Browse files
committed
Revert "[SPARK-26946][SQL][FOLLOWUP] Require lookup function"
This reverts commit 7e11ca6
1 parent 780f7ff commit dd5b799

File tree

5 files changed

+123
-105
lines changed

5 files changed

+123
-105
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalog/v2/LookupCatalog.scala

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,31 +26,35 @@ import org.apache.spark.sql.catalyst.TableIdentifier
2626
@Experimental
2727
trait LookupCatalog {
2828

29-
protected def lookupCatalog(name: String): CatalogPlugin
29+
def lookupCatalog: Option[(String) => CatalogPlugin] = None
3030

3131
type CatalogObjectIdentifier = (Option[CatalogPlugin], Identifier)
3232

3333
/**
3434
* Extract catalog plugin and identifier from a multi-part identifier.
3535
*/
3636
object CatalogObjectIdentifier {
37-
def unapply(parts: Seq[String]): Some[CatalogObjectIdentifier] = parts match {
38-
case Seq(name) =>
39-
Some((None, Identifier.of(Array.empty, name)))
40-
case Seq(catalogName, tail @ _*) =>
41-
try {
42-
Some((Some(lookupCatalog(catalogName)), Identifier.of(tail.init.toArray, tail.last)))
43-
} catch {
44-
case _: CatalogNotFoundException =>
45-
Some((None, Identifier.of(parts.init.toArray, parts.last)))
46-
}
37+
def unapply(parts: Seq[String]): Option[CatalogObjectIdentifier] = lookupCatalog.map { lookup =>
38+
parts match {
39+
case Seq(name) =>
40+
(None, Identifier.of(Array.empty, name))
41+
case Seq(catalogName, tail @ _*) =>
42+
try {
43+
val catalog = lookup(catalogName)
44+
(Some(catalog), Identifier.of(tail.init.toArray, tail.last))
45+
} catch {
46+
case _: CatalogNotFoundException =>
47+
(None, Identifier.of(parts.init.toArray, parts.last))
48+
}
49+
}
4750
}
4851
}
4952

5053
/**
5154
* Extract legacy table identifier from a multi-part identifier.
5255
*
53-
* For legacy support only. Please use [[CatalogObjectIdentifier]] instead on DSv2 code paths.
56+
* For legacy support only. Please use
57+
* [[org.apache.spark.sql.catalog.v2.LookupCatalog.CatalogObjectIdentifier]] in DSv2 code paths.
5458
*/
5559
object AsTableIdentifier {
5660
def unapply(parts: Seq[String]): Option[TableIdentifier] = parts match {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import scala.collection.mutable.ArrayBuffer
2424
import scala.util.Random
2525

2626
import org.apache.spark.sql.AnalysisException
27-
import org.apache.spark.sql.catalog.v2.{CatalogNotFoundException, CatalogPlugin, LookupCatalog}
27+
import org.apache.spark.sql.catalog.v2.{CatalogPlugin, LookupCatalog}
2828
import org.apache.spark.sql.catalyst._
2929
import org.apache.spark.sql.catalyst.catalog._
3030
import org.apache.spark.sql.catalyst.encoders.OuterScopes
@@ -96,15 +96,18 @@ object AnalysisContext {
9696
class Analyzer(
9797
catalog: SessionCatalog,
9898
conf: SQLConf,
99-
maxIterations: Int)
99+
maxIterations: Int,
100+
override val lookupCatalog: Option[(String) => CatalogPlugin] = None)
100101
extends RuleExecutor[LogicalPlan] with CheckAnalysis with LookupCatalog {
101102

102103
def this(catalog: SessionCatalog, conf: SQLConf) = {
103104
this(catalog, conf, conf.optimizerMaxIterations)
104105
}
105106

106-
override protected def lookupCatalog(name: String): CatalogPlugin =
107-
throw new CatalogNotFoundException("No catalog lookup function")
107+
def this(lookupCatalog: Option[(String) => CatalogPlugin], catalog: SessionCatalog,
108+
conf: SQLConf) = {
109+
this(catalog, conf, conf.optimizerMaxIterations, lookupCatalog)
110+
}
108111

109112
def executeAndCheck(plan: LogicalPlan, tracker: QueryPlanningTracker): LogicalPlan = {
110113
AnalysisHelper.markInAnalyzer {

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/v2/LookupCatalogSuite.scala

Lines changed: 0 additions & 88 deletions
This file was deleted.
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.spark.sql.catalyst.catalog.v2
18+
19+
import org.scalatest.Matchers._
20+
21+
import org.apache.spark.sql.catalog.v2.{CatalogNotFoundException, CatalogPlugin}
22+
import org.apache.spark.sql.catalyst.TableIdentifier
23+
import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, Analyzer}
24+
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
25+
import org.apache.spark.sql.internal.SQLConf
26+
import org.apache.spark.sql.util.CaseInsensitiveStringMap
27+
28+
private class TestCatalogPlugin(override val name: String) extends CatalogPlugin {
29+
30+
override def initialize(name: String, options: CaseInsensitiveStringMap): Unit = Unit
31+
}
32+
33+
class ResolveMultipartIdentifierSuite extends AnalysisTest {
34+
import CatalystSqlParser._
35+
36+
private val analyzer = makeAnalyzer(caseSensitive = false)
37+
38+
private val catalogs = Seq("prod", "test").map(name => name -> new TestCatalogPlugin(name)).toMap
39+
40+
private def lookupCatalog(catalog: String): CatalogPlugin =
41+
catalogs.getOrElse(catalog, throw new CatalogNotFoundException("Not found"))
42+
43+
private def makeAnalyzer(caseSensitive: Boolean) = {
44+
val conf = new SQLConf().copy(SQLConf.CASE_SENSITIVE -> caseSensitive)
45+
new Analyzer(Some(lookupCatalog _), null, conf)
46+
}
47+
48+
override protected def getAnalyzer(caseSensitive: Boolean) = analyzer
49+
50+
private def checkResolution(sqlText: String, expectedCatalog: Option[CatalogPlugin],
51+
expectedNamespace: Array[String], expectedName: String): Unit = {
52+
53+
import analyzer.CatalogObjectIdentifier
54+
val CatalogObjectIdentifier(catalog, ident) = parseMultipartIdentifier(sqlText)
55+
catalog shouldEqual expectedCatalog
56+
ident.namespace shouldEqual expectedNamespace
57+
ident.name shouldEqual expectedName
58+
}
59+
60+
private def checkTableResolution(sqlText: String,
61+
expectedIdent: Option[TableIdentifier]): Unit = {
62+
63+
import analyzer.AsTableIdentifier
64+
parseMultipartIdentifier(sqlText) match {
65+
case AsTableIdentifier(ident) =>
66+
assert(Some(ident) === expectedIdent)
67+
case _ =>
68+
assert(None === expectedIdent)
69+
}
70+
}
71+
72+
test("resolve multipart identifier") {
73+
checkResolution("tbl", None, Array.empty, "tbl")
74+
checkResolution("db.tbl", None, Array("db"), "tbl")
75+
checkResolution("prod.func", catalogs.get("prod"), Array.empty, "func")
76+
checkResolution("ns1.ns2.tbl", None, Array("ns1", "ns2"), "tbl")
77+
checkResolution("prod.db.tbl", catalogs.get("prod"), Array("db"), "tbl")
78+
checkResolution("test.db.tbl", catalogs.get("test"), Array("db"), "tbl")
79+
checkResolution("test.ns1.ns2.ns3.tbl",
80+
catalogs.get("test"), Array("ns1", "ns2", "ns3"), "tbl")
81+
checkResolution("`db.tbl`", None, Array.empty, "db.tbl")
82+
checkResolution("parquet.`file:/tmp/db.tbl`", None, Array("parquet"), "file:/tmp/db.tbl")
83+
checkResolution("`org.apache.spark.sql.json`.`s3://buck/tmp/abc.json`", None,
84+
Array("org.apache.spark.sql.json"), "s3://buck/tmp/abc.json")
85+
}
86+
87+
test("resolve table identifier") {
88+
checkTableResolution("tbl", Some(TableIdentifier("tbl")))
89+
checkTableResolution("db.tbl", Some(TableIdentifier("tbl", Some("db"))))
90+
checkTableResolution("prod.func", None)
91+
checkTableResolution("ns1.ns2.tbl", None)
92+
checkTableResolution("prod.db.tbl", None)
93+
checkTableResolution("`db.tbl`", Some(TableIdentifier("db.tbl")))
94+
checkTableResolution("parquet.`file:/tmp/db.tbl`",
95+
Some(TableIdentifier("file:/tmp/db.tbl", Some("parquet"))))
96+
checkTableResolution("`org.apache.spark.sql.json`.`s3://buck/tmp/abc.json`",
97+
Some(TableIdentifier("s3://buck/tmp/abc.json", Some("org.apache.spark.sql.json"))))
98+
}
99+
}

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceResolution.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ case class DataSourceResolution(
4242

4343
import org.apache.spark.sql.catalog.v2.CatalogV2Implicits._
4444

45-
override protected def lookupCatalog(name: String): CatalogPlugin = findCatalog(name)
45+
override def lookupCatalog: Option[String => CatalogPlugin] = Some(findCatalog)
4646

4747
def defaultCatalog: Option[CatalogPlugin] = conf.defaultV2Catalog.map(findCatalog)
4848

0 commit comments

Comments
 (0)