[SPARK-29039][SQL] centralize the catalog and table lookup logic #25747

cloud-fan · 2019-09-10T17:08:01Z

What changes were proposed in this pull request?

Currently we deal with different ParsedStatement in many places and write duplicated catalog/table lookup logic. In general the lookup logic is

try look up the catalog by name. If no such catalog, and default catalog is not set, convert ParsedStatement to v1 command like ShowDatabasesCommand. Otherwise, convert ParsedStatement to v2 command like ShowNamespaces.
try look up the table by name. If no such table, fail. If the table is a V1Table, convert ParsedStatement to v1 command like CreateTable. Otherwise, convert ParsedStatement to v2 command like CreateV2Table.

However, since the code is duplicated we don't apply this lookup logic consistently. For example, we forget to consider the v2 session catalog in several places.

This PR centralizes the catalog/table lookup logic by 3 rules.

ResolveCatalogs (in catalyst). This rule resolves v2 catalog from the multipart identifier in SQL statements, and convert the statement to v2 command if the resolved catalog is not session catalog. If the command needs to resolve the table (e.g. ALTER TABLE), put an UnresolvedV2Table in the command.
ResolveTables (in catalyst). It resolves UnresolvedV2Table to DataSourceV2Relation.
ResolveSessionCatalog (in sql/core). This rule is only effective if the resolved catalog is session catalog. For commands that don't need to resolve the table, this rule converts the statement to v1 command directly. Otherwise, it converts the statement to v1 command if the resolved table is v1 table, and convert to v2 command if the resolved table is v2 table. Hopefully we can remove this rule eventually when v1 fallback is not needed anymore.

Why are the changes needed?

Reduce duplicated code and make the catalog/table lookup logic consistent.

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

cloud-fan · 2019-09-10T17:09:54Z

sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/utils/CatalogV2Util.scala

moved from https://github.com/apache/spark/pull/25747/files#diff-acba0f0ea96f6511882f4ead4fb33dc2L338

cloud-fan · 2019-09-10T17:17:11Z

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

-      val relation = UnresolvedRelation(delete.tableName)
-      val aliased = delete.tableAlias.map(SubqueryAlias(_, relation)).getOrElse(relation)
-      DeleteFromTable(aliased, delete.condition)
+    case ShowNamespacesStatement(None, pattern) if defaultCatalog.isEmpty =>


I found this problem while refactoring the catalog lookup logic. If no catalog is specified and default catalog is not set, we should fallback to v1 command which is ShowDatabasesCommand in this case.

OK, I may have misunderstood this discussion: #25601 (comment) - I had an impression that SHOW NAMESPACES is supported only in v2.

I don't think this is correct. The SHOW DATABASES command was not changed, so using SHOW DATABASES is the v1 fallback. SHOW NAMESPACES always uses v2. I think that's fine.

cloud-fan · 2019-09-10T17:18:47Z

cc @brkyvz @rdblue

also cc @imback82 . With this refactor, it should be much easier to refine the catalog/table lookup logic to fix https://issues.apache.org/jira/browse/SPARK-29014

SparkQA · 2019-09-10T18:34:48Z

Test build #110429 has finished for PR 25747 at commit b4f46d3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
implicit class NamePartsHelper(nameParts: Seq[String])
case class DescribeTable(table: Table, isExtended: Boolean) extends Command
trait AlterTableStatement extends StatementRequiringCatalogAndTable
abstract class ParsedStatement extends LogicalPlan
abstract class StatementRequiringCatalog extends ParsedStatement
abstract class StatementRequiringCatalogAndTable extends ParsedStatement
case class ShowNamespacesStatement(

imback82 · 2019-09-10T23:51:03Z

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

+  object CatalogAndRestNameParts {
+    def unapply(nameParts: Seq[String]): Some[(Option[CatalogPlugin], Seq[String])] = {
+      if (nameParts.isEmpty) {
+        Some((defaultCatalog, Nil))


Shouldn't we introduce currentCatalog in CatalogManager and use it instead of defaultCatalog as @rdblue suggested? I was thinking doing something like the following: master...imback82:current_ns

I'd like to make this PR a pure refactor and leave it to followups. For example, we should add more tests when changing this to currentCatalog.

rdblue · 2019-09-11T16:06:55Z

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

  }

+  implicit class NamePartsHelper(nameParts: Seq[String]) {
+    def toIdentifier: Identifier = Identifier.of(nameParts.init.toArray, nameParts.last)


Why not add this to MultipartIdentifierHelper?

MultipartIdentifierHelper takes a parameter called namespace. I'm a little worried about semantic mismatch here so I created a new one. How about we merge these two and call it NamePartsHelper? nameParts is very general.

I think we could just change that one to use nameParts and combine the two.

brkyvz · 2019-09-12T17:27:36Z

...alyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala

    catalog: TableCatalog,
    ident: Identifier,
-    table: NamedRelation,
+    table: Table,


This was previously NamedRelation so that it could be converted from a statement when the catalog is resolved using UnresolvedRelation. Then the normal table resolution rule would run to resolve this as to a DSv2 relation. Why change that?

As I think about this more, it seems like a bad idea.

This ties the SQL logical plans to a particular type of relation. There is no need to do that. What if we decide to add a v3 connector API? Then we would have to change all of these logical plans instead of using a new type of relation.

I think we should move this back to NamedRelation to keep logical plans and the connector API separate. That also makes rules cleaner, because catalog resolution and table resolution are done in independent rules.

brkyvz · 2019-09-12T17:29:32Z

sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/utils/CatalogV2Util.scala

+  def loadV2Table(catalog: CatalogPlugin, ident: Identifier): Option[Table] =
    try {
-      Option(catalog.asTableCatalog.loadTable(ident))
+      Option(catalog.asTableCatalog.loadTable(ident)).filterNot(_.isInstanceOf[V1Table])


while this does seem to simplify code a little bit, I think it's going to hamper the small incremental work that we can do to migrate to V2 from V1. Having this one centralized thing would cause PRs to have humongous changes when we want ALTER TABLE ADD COLUMNS behavior go fully through V2SessionCatalog for example

rdblue · 2019-09-13T23:14:49Z

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

    }

+    if (catalogTable.tableType == CatalogTableType.VIEW) {
+      throw new NoSuchTableException(ident)


Should this be a separate bug fix?

rdblue · 2019-09-13T23:15:10Z

sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2SQLSuite.scala


  test("DropTable: if exists") {
-    intercept[NoSuchTableException] {
+    intercept[AnalysisException] {


Why did this change?

rdblue · 2019-09-13T23:57:24Z

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

      case _ => plan
    }

+    private def tryResolveV2Relation(u: UnresolvedRelation): Option[DataSourceV2Relation] = {


I think this should continue to use extractors instead of inline logic. The rules are cleaner when written with extractors. We might need to update them, but I'd like to avoid methods like this one that do basically the same thing.

rdblue · 2019-09-13T23:57:45Z

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

+      case i @ InsertIntoStatement(u: UnresolvedRelation, _, _, _, _) if i.query.resolved =>
+        tryResolveV2Relation(u).map(v2Relation => i.copy(table = v2Relation)).getOrElse {
+          if (u.multipartIdentifier.length <= 2) {
+            val ident = u.multipartIdentifier match {


This should get the identifier value from an extractor instead.

rdblue · 2019-09-14T00:12:54Z

@cloud-fan, overall I like some parts of this PR quite a bit. It is really nice how the rules are collapsed in this PR. The problem is that I'm not sure that the trade-off is worth it. I don't think it is good for SQL statements to be responsible for converting themselves into DSv2 plans. We want to keep the statements that are parsed and the plan implementations separate. And it seems odd to move lots of rule logic into plan nodes themselves.

I think that the right approach for keeping this logic in one place is to use extractors in rules. Recent changes have moved further away from that, but I think that's the cleanest way to keep this logic in one place.

The reason we have changed this recently is the need to handle v1 tables. But I think we should take a different approach to support those. The only reason why catalog/identifier resolution and table resolution happen at the same time is to support fallback to v1. But we don't need fallback to v1 outside of the DataSourceResolution rules.

Here's what I would do:

Use an extractor to resolve catalog and identifier that matches if and only if the catalog must be v2 -- that is, when the catalog is not spark_catalog (built-in session catalog). Rules in analyzer should use this extractor. Note that when v1 goes away, these rules don't need to change.
If the spark_catalog is responsible for an identifier, the above extractor doesn't match because v1 might be used, depending on the table. Then rule should be in DataSourceResolution and will load the table and check its provider or check the provider for create plans.

With this approach, catalog/identifier resolution is separate from table resolution. Table resolution would happen in ResolveTables. Identifier resolution would happen in the extractor. Both are cleanly separated.

What do you think?

cloud-fan · 2019-09-16T03:27:59Z

Hi @rdblue , thanks for taking a look! What I want to get consensus on is the categories of catalog/table resolution I mentioned in the PR description:

try look up the catalog by name. If no such catalog, and default catalog is not set, convert ParsedStatement to v1 command like ShowDatabasesCommand. Otherwise, convert ParsedStatement to v2 command like ShowNamespaces.
try look up the table by name. If no such table, fail. If the table is a V1Table, convert ParsedStatement to v1 command like CreateTable. Otherwise, convert ParsedStatement to v2 command like CreateV2Table.

If we agree with this, I don't mind to use different extractors to implement this. I'll update this PR to use extractors.

cloud-fan · 2019-09-16T12:09:09Z

I've updated this PR to use extractors and updated the PR description.

SparkQA · 2019-09-16T21:26:29Z

Test build #110642 has finished for PR 25747 at commit 14aff3e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-17T05:15:49Z

Test build #110724 has finished for PR 25747 at commit 4d714ad.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-09-17T07:05:02Z

Test build #110740 has finished for PR 25747 at commit 47eaf14.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2019-09-17T07:30:19Z

retest this please

SparkQA · 2019-09-17T10:04:16Z

Test build #110752 has finished for PR 25747 at commit 47eaf14.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2019-09-17T14:51:19Z

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveTables.scala

+      DescribeTableCommand(tblName.toV1Identifier, partitionSpec, isExtended)
+  }
+
+  private def assertTopLeveColumn(colName: Seq[String], command: String): Unit = {


This method and below methods are moved from the old DataSourceResolution.

SparkQA · 2019-10-02T15:54:25Z

Test build #111693 has finished for PR 25747 at commit ae9efee.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class _GaussianMixtureParams(HasMaxIter, HasFeaturesCol, HasSeed, HasPredictionCol,
class GaussianMixtureModel(JavaModel, _GaussianMixtureParams, JavaMLWritable, JavaMLReadable,
class GaussianMixture(JavaEstimator, _GaussianMixtureParams, JavaMLWritable, JavaMLReadable):
class _KMeansParams(HasMaxIter, HasFeaturesCol, HasSeed, HasPredictionCol, HasTol,
class KMeansModel(JavaModel, _KMeansParams, GeneralJavaMLWritable, JavaMLReadable,
class KMeans(JavaEstimator, _KMeansParams, JavaMLWritable, JavaMLReadable):
class _BisectingKMeansParams(HasMaxIter, HasFeaturesCol, HasSeed, HasPredictionCol,
class BisectingKMeansModel(JavaModel, _BisectingKMeansParams, JavaMLWritable, JavaMLReadable,
class BisectingKMeans(JavaEstimator, _BisectingKMeansParams, JavaMLWritable, JavaMLReadable):
class _LDAParams(HasMaxIter, HasFeaturesCol, HasSeed, HasCheckpointInterval):
class LDAModel(JavaModel, _LDAParams):
class LDA(JavaEstimator, _LDAParams, JavaMLReadable, JavaMLWritable):
class _PowerIterationClusteringParams(HasMaxIter, HasWeightCol):
class PowerIterationClustering(_PowerIterationClusteringParams, JavaParams, JavaMLReadable,
case class SecondWithFraction(child: Expression, timeZoneId: Option[String] = None)
case class PostgreCastStringToBoolean(child: Expression)
abstract class Optimizer(catalogManager: CatalogManager)
case class GetCurrentDatabase(catalogManager: CatalogManager) extends Rule[LogicalPlan]
case class SetCatalogAndNamespace(
case class UseStatement(isNamespaceSet: Boolean, nameParts: Seq[String]) extends ParsedStatement
case class AlterDatabaseSetLocationCommand(databaseName: String, location: String)
case class SetCatalogAndNamespaceExec(

SparkQA · 2019-10-02T18:27:46Z

Test build #111695 has finished for PR 25747 at commit a0429ec.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

rdblue · 2019-10-02T20:44:45Z

There are a few problems with mixing identifier resolution with table lookup.

First, there is only one case where we need to do them at the same time: for v1 fallback tables. But there are other cases where loading and inspecting a table has been suggested. For example, if we are looking up cat.tab and the catalog cat doesn't contain table tab, then we could check for cat.tab in the session catalog. Adding that second lookup inadvertently creates lots of problems; for example, you can create a table using CTAS that will replace an existing table with the same name (cat.tab). We want to avoid bad behavior changes later by separating concerns now.

Second, by separating the resolution rules in the Analyzer, we won't need to rewrite them later when we remove v1 fallback. When we remove v1 tables, we want to simply remove the DataSourceResolution rule because the session catalog is a v2 catalog like any other. Also, if users want to use a v2 catalog as the default and to not use session at all, they can blacklist the DataSourceResolution rule without breaking anything.

I see the code duplication argument, but we can separate conversion into a utility class that is used in both places.

SparkQA · 2019-10-03T12:49:46Z

Test build #111732 has finished for PR 25747 at commit c0bdaab.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-10-03T13:03:33Z

Test build #111733 has finished for PR 25747 at commit 9d1c73c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rdblue · 2019-10-03T17:57:16Z

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

+      DeleteFromTable(aliased, condition)
+
+    case update: UpdateTableStatement =>
+      throw new AnalysisException(s"Update table is not supported temporarily.")


Nit: in other cases, SQL keywords use all caps to distinguish from text. Should be "UPDATE TABLE is not ..."

rdblue · 2019-10-03T18:00:13Z

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

+    case DescribeTableStatement(
+         nameParts @ NonSessionCatalog(catalog, tableName), partitionSpec, isExtended) =>
+      if (partitionSpec.nonEmpty) {
+        throw new AnalysisException("DESC TABLE does not support partition for v2 tables.")


Nit: should we use the full keyword instead of an abbreviation? "DESCRIBE TABLE does not ..."

rdblue · 2019-10-03T22:20:04Z

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

+        writeOptions = c.options.filterKeys(_ != "path"),
+        orCreate = c.orCreate)
+
+    case DropTableStatement(NonSessionCatalog(catalog, tableName), ifExists, purge) =>


Nit: purge isn't used so it should be _.

rdblue · 2019-10-03T22:21:01Z

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

+      }
+  }
+
+  object NonSessionCatalog {


+1 for this. We may want to move this into the lookup methods and use it as a common extractor pattern. Looks like we probably need to clean those up and simplify after all the planned updates are done.

rdblue · 2019-10-03T22:22:23Z

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

+      val namespace = if (nameParts.isEmpty) None else Some(nameParts)
+      ShowNamespaces(catalog.asNamespaceCatalog, namespace, pattern)
+
+    // TODO (SPARK-29014): we should check if the current catalog is not session catalog here.


Yes, this should use the current catalog. After ShowNamespaces and ShowDatabases are merged, we won't need the fallback.

rdblue · 2019-10-03T22:24:19Z

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

+ * @param catalog The catalog which the table should be looked up from.
+ * @param tableName The name of the table to look up.
+ */
+case class UnresolvedV2Table(


Minor: I think we may want to use UnresolvedV2Relation instead. The only thing we know is that this uses a v2 catalog. It could be a v2 view in the future.

rdblue · 2019-10-03T22:31:12Z

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala


-    assert(exception.getMessage.contains("No default v2 catalog is set"))
+    assert(exception.getMessage.contains(
+      "SHOW NAMESPACES is not supported with the session catalog"))


Why is this not supported?

This is the existing behavior. We will support it once we unify SHOW NAMESPACE and SHOW DATABASE. AFAIK @imback82 already has a PR for it.

Yes, I will update this as soon as this PR is merged.

rdblue · 2019-10-03T22:32:06Z

sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala


+  private def assertViewNotSupported(query: String): Unit = {
+    val e = intercept[AnalysisException](sql(query))
+    assert(e.message.contains("'testView' is a view not table"))


Nit: should be not a table.

rdblue · 2019-10-03T23:12:09Z

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

+          AlterTableAddColumnsCommand(tableName.asTableIdentifier, cols.map(convertToStructField))
+      }.getOrElse {
+        val changes = cols.map { col =>
+          TableChange.addColumn(col.name.toArray, col.dataType, true, col.comment.orNull)


Nit: boolean arguments should be named.

Or is this not allowed since it's a Java method?

yes, the method parameter name is not kept in Java.

rdblue · 2019-10-03T23:12:53Z

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

+         nameParts @ SessionCatalog(catalog, tableName), colName, dataType, comment) =>
+      loadTable(catalog, tableName.asIdentifier).collect {
+        case v1Table: V1Table =>
+          // TODO: we should fallback to the v1 `AlterTableChangeColumnCommand`.


Should we open an issue for this?

ticket created: https://issues.apache.org/jira/browse/SPARK-29353

rdblue · 2019-10-03T23:26:19Z

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

+    }
+  }
+
+  def createV2AlterTable(


Is this used?

rdblue · 2019-10-03T23:29:04Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala

        |LOCATION '/user/external/page_view'
        |TBLPROPERTIES ('p1'='v1', 'p2'='v2')
-        |AS SELECT * FROM src
+        |AS SELECT 1


Why did this change? To avoid needing to look up src?

These test cases originally came directly from the SparkSql parser tests. Not changing them was a way to ensure compatibility.

rdblue · 2019-10-03T23:33:45Z

@cloud-fan, looks good. The only thing other than nits is that I think we should probably rename UnresolvedV2Table to UnresolvedV2Relation because it could resolve to a view and is not necessarily a table. Otherwise, +1.

SparkQA · 2019-10-04T06:39:51Z

Test build #111763 has finished for PR 25747 at commit 9ad516e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedV2Relation(

cloud-fan · 2019-10-04T08:21:27Z

thanks for the review, merging to master!

waterlx · 2019-12-05T07:26:23Z

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

+        orCreate = c.orCreate)
+
+    case DropTableStatement(NonSessionCatalog(catalog, tableName), ifExists, purge) =>
+      DropTable(catalog.asTableCatalog, tableName.asIdentifier, ifExists)


hi @cloud-fan , I got your message on TableCatalog # dropTable() not having purge as its option. It seems purge is not passed on from here (not passed into DropTable). Do I get it right?

Here we are converting to v2 DropTable. We can fail if purge = true

ulysses-you · 2019-12-06T02:29:07Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala

-    comparePlans(parsed1_table, expected1_table)
-    comparePlans(parsed2_table, expected2_table)
-    comparePlans(parsed3_table, expected3_table)
+    // For non-existing tables, we convert it to v2 command with `UnresolvedV2Table`


Hi @cloud-fan I'm curious why we need convert non-existing table to v2 command. In my idea, we can just handle this in v1 command.

we use v2 command for v2 table, and v1 command for v1 table. If table not exists, I think v2 command is better, as we want to migrate everything to v2 command in the future.

I do agree v2 command is better than v1. But it will make some things unexpected. when load table return none, ResolveSessionCatalog will check if v2 support it, and throw not support msg when not support and throw table not found when support. I do not determine which msg is the high priority. Here is a example:

create table test(c int); // throw Table not found desc tes; // throw Describing columns is not supported for v2 tables.; desc tes c;

Shall we keep this consistent when table is non-existing ?

If we use v1 command directly, things will be more simple. We even not need load table(it will affect perf), just use v1 command when catalog is session catalog.
E.G.

case DescribeTableStatement(SessionCatalog(_, tableName), partitionSpec, isExtended) => DescribeTableCommand(tableName.asTableIdentifier, partitionSpec, isExtended) case DescribeColumnStatement(SessionCatalog(_, tableName), colNameParts, isExtended) => DescribeColumnCommand(tableName.asTableIdentifier, colNameParts, isExtended)

I think it's fine. fail earlier is better than looking up the table and report table not found.

That's OK. Thanks.

cloud-fan commented Sep 10, 2019

View reviewed changes

cloud-fan force-pushed the lookup branch from da6edf9 to b4f46d3 Compare September 10, 2019 17:11

cloud-fan commented Sep 10, 2019

View reviewed changes

dongjoon-hyun added the SQL label Sep 10, 2019

imback82 reviewed Sep 10, 2019

View reviewed changes

rdblue reviewed Sep 11, 2019

View reviewed changes

imback82 mentioned this pull request Sep 12, 2019

[SPARK-28970][SQL] Implement USE CATALOG/NAMESPACE for Data Source V2 #25771

Closed

brkyvz reviewed Sep 12, 2019

View reviewed changes

rdblue reviewed Sep 13, 2019

View reviewed changes

cloud-fan force-pushed the lookup branch 3 times, most recently from d08e930 to 14aff3e Compare September 16, 2019 12:05

cloud-fan force-pushed the lookup branch from 14aff3e to 4d714ad Compare September 17, 2019 02:54

cloud-fan force-pushed the lookup branch from 4d714ad to 47eaf14 Compare September 17, 2019 05:51

cloud-fan force-pushed the lookup branch from 47eaf14 to 372ba48 Compare September 17, 2019 14:48

cloud-fan commented Sep 17, 2019

View reviewed changes

imback82 mentioned this pull request Oct 3, 2019

[SPARK-29279][SQL] Merge SHOW NAMESPACES and SHOW DATABASES code path #26006

Closed

address comment

9d1c73c

cloud-fan force-pushed the lookup branch from c0bdaab to 9d1c73c Compare October 3, 2019 08:49

rdblue reviewed Oct 3, 2019

View reviewed changes

address comments

9ad516e

cloud-fan closed this in 275e044 Oct 4, 2019

waterlx reviewed Dec 5, 2019

View reviewed changes

ulysses-you reviewed Dec 6, 2019

View reviewed changes

[SPARK-29039][SQL] centralize the catalog and table lookup logic #25747

[SPARK-29039][SQL] centralize the catalog and table lookup logic #25747

Uh oh!

Conversation

cloud-fan commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Sep 10, 2019

Uh oh!

SparkQA commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdblue commented Sep 14, 2019

Uh oh!

cloud-fan commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Sep 16, 2019

Uh oh!

SparkQA commented Sep 16, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

cloud-fan commented Sep 17, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 2, 2019

Uh oh!

SparkQA commented Oct 2, 2019

Uh oh!

rdblue commented Oct 2, 2019

Uh oh!

SparkQA commented Oct 3, 2019

Uh oh!

SparkQA commented Oct 3, 2019

Uh oh!

cloud-fan commented Sep 10, 2019 •

edited

Loading

cloud-fan commented Sep 16, 2019 •

edited

Loading