[SPARK-29513][SQL] REFRESH TABLE should look up catalog/table like v2 commands #26183

imback82 · 2019-10-20T14:55:58Z

What changes were proposed in this pull request?

Add RefreshTableStatement and make REFRESH TABLE go through the same catalog/table resolution framework of v2 commands.

Why are the changes needed?

It's important to make all the commands have the same table resolution behavior, to avoid confusing end-users. e.g.

USE my_catalog
DESC t // success and describe the table t from my_catalog
REFRESH TABLE t // report table not found as there is no table t in the session catalog

Does this PR introduce any user-facing change?

yes. When running REFRESH TABLE, Spark fails the command if the current catalog is set to a v2 catalog, or the table name specified a v2 catalog.

How was this patch tested?

New unit tests

imback82 · 2019-10-20T14:56:49Z

cc: @cloud-fan @rdblue @viirya

SparkQA · 2019-10-20T18:34:38Z

Test build #112343 has finished for PR 26183 at commit dc52212.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class RefreshTableStatement(tableName: Seq[String]) extends ParsedStatement

cloud-fan · 2019-10-21T06:07:52Z

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

        "MSCK REPAIR TABLE")
+
+    case RefreshTableStatement(tableName) =>
+      val v1TableName = parseV1Table(tableName, "REFRESH TABLE")


We have TableCatalog.invalidateTable to implement a v2 REFRESH TABLE command.

Followed by TableCatalog.loadTable? OrTableCatalog.loadTable is called lazily (you don't call it inside RefreshTableExec - in this case, same behavior as UNCACHE TABLE)?

Not followed by loadTable. It should invalidate and load the table lazily.

Thanks. I adde v2 command in this PR.

rdblue · 2019-10-21T20:48:09Z

Looks fine to me. We can add the implementation here or in a separate PR.

imback82 · 2019-10-22T05:07:28Z

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

+    val t = "testcat.ns1.ns2.tbl"
+    withTable(t) {
+      sql(s"CREATE TABLE $t (id bigint, data string) USING foo")
+      sql(s"REFRESH TABLE $t")


Any suggestion for a reasonable test here?

ALTER TABLE doesn't require REFRESH TABLE in the following existing test, correct?

sql(s"CREATE TABLE $t (id int, data string) USING $v2Format") sql(s"ALTER TABLE $t DROP COLUMN data") val table = getTableMetadata(t) // <- this calls TableCatalog.loadTable

I think a reasonable test is to create a custom v2 implementation which implements invalidateTable.

We can implement InMemoryTable.invalidateTable which simply set a flag on a global object to indicate it has been called.

Thanks for the suggestion! Exactly what I was planning to do. :)

SparkQA · 2019-10-22T07:05:01Z

Test build #112439 has finished for PR 26183 at commit fcc31f2.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class RefreshTable(
case class RefreshTableExec(

imback82 · 2019-10-22T13:44:32Z

retest this please

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

SparkQA · 2019-10-22T17:31:19Z

Test build #112462 has finished for PR 26183 at commit 7b884e8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
skeleton_class = type_constructor(name, bases, type_kwargs)
enum_class = metacls.__new__(metacls, name, bases, classdict)
case class BitAndAgg(child: Expression) extends DeclarativeAggregate with ExpectsInputTypes
case class BitOrAgg(child: Expression) extends DeclarativeAggregate with ExpectsInputTypes
abstract class AbstractSqlParser(conf: SQLConf) extends ParserInterface with Logging
class CatalystSqlParser(conf: SQLConf) extends AbstractSqlParser(conf)
case class TruncateTableStatement(
class SparkSqlParser(conf: SQLConf) extends AbstractSqlParser(conf)

SparkQA · 2019-10-22T17:36:27Z

Test build #112466 has finished for PR 26183 at commit 7b884e8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
skeleton_class = type_constructor(name, bases, type_kwargs)
enum_class = metacls.__new__(metacls, name, bases, classdict)
case class BitAndAgg(child: Expression) extends DeclarativeAggregate with ExpectsInputTypes
case class BitOrAgg(child: Expression) extends DeclarativeAggregate with ExpectsInputTypes
abstract class AbstractSqlParser(conf: SQLConf) extends ParserInterface with Logging
class CatalystSqlParser(conf: SQLConf) extends AbstractSqlParser(conf)
case class TruncateTableStatement(
class SparkSqlParser(conf: SQLConf) extends AbstractSqlParser(conf)

SparkQA · 2019-10-22T20:37:24Z

Test build #112477 has finished for PR 26183 at commit fe577f5.

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

viirya

looks good.

SparkQA · 2019-10-23T03:07:54Z

Test build #112492 has finished for PR 26183 at commit 3a1da85.

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

cloud-fan · 2019-10-23T04:22:12Z

LGTM, but we need to resolve conflicts again...

imback82 · 2019-10-23T04:31:37Z

Just resolved conflicts. Thanks!

SparkQA · 2019-10-23T07:05:01Z

Test build #112507 has finished for PR 26183 at commit 31bfc6f.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CreateNamespaceStatement(
case class CreateNamespace(
case class CreateNamespaceExec(

cloud-fan · 2019-10-23T07:10:14Z

retest this please

SparkQA · 2019-10-23T10:38:17Z

Test build #112519 has finished for PR 26183 at commit 31bfc6f.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CreateNamespaceStatement(
case class CreateNamespace(
case class CreateNamespaceExec(

cloud-fan · 2019-10-23T10:45:06Z

retest this please

SparkQA · 2019-10-23T11:19:31Z

Test build #112522 has finished for PR 26183 at commit 31bfc6f.

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

SparkQA · 2019-10-23T14:55:15Z

Test build #112535 has finished for PR 26183 at commit 31bfc6f.

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

viirya · 2019-10-23T15:26:04Z

Thanks! Merging to master.

imback82 · 2019-10-23T15:46:08Z

Thanks all!

initial checkin

dc52212

imback82 mentioned this pull request Oct 20, 2019

[SPARK-29521][SQL] LOAD DATA INTO TABLE should look up catalog/table like v2 commands #26178

Closed

imback82 changed the title ~~[SPARK-29512][SQL] REFRESH TABLE should look up catalog/table like v2 commands~~ [SPARK-29513][SQL] REFRESH TABLE should look up catalog/table like v2 commands Oct 20, 2019

dongjoon-hyun added the SQL label Oct 20, 2019

cloud-fan reviewed Oct 21, 2019

View reviewed changes

implement v2 command

fcc31f2

imback82 commented Oct 22, 2019

View reviewed changes

cloud-fan approved these changes Oct 22, 2019

View reviewed changes

Merge branch 'master' into refresh_table

7b884e8

viirya reviewed Oct 22, 2019

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/RefreshTableExec.scala Show resolved Hide resolved

Address PR comments

fe577f5

Merge branch 'master' into refresh_table

3a1da85

viirya approved these changes Oct 22, 2019

View reviewed changes

Merge branch 'master' into refresh_table

31bfc6f

viirya mentioned this pull request Oct 23, 2019

[SPARK-29563][SQL] CREATE TABLE LIKE should look up catalog/table like v2 commands #26219

Closed

viirya closed this in 53a5f17 Oct 23, 2019

imback82 deleted the refresh_table branch October 23, 2019 15:45

[SPARK-29513][SQL] REFRESH TABLE should look up catalog/table like v2 commands #26183

[SPARK-29513][SQL] REFRESH TABLE should look up catalog/table like v2 commands #26183

Uh oh!

Conversation

imback82 commented Oct 20, 2019

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!

imback82 commented Oct 20, 2019

Uh oh!

SparkQA commented Oct 20, 2019

Uh oh!

cloud-fan Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

imback82 Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

rdblue Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

imback82 Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

rdblue commented Oct 21, 2019

Uh oh!

imback82 Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

cloud-fan Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imback82 Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

imback82 commented Oct 22, 2019

Uh oh!

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

viirya left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

cloud-fan commented Oct 23, 2019

Uh oh!

imback82 commented Oct 23, 2019

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

cloud-fan commented Oct 23, 2019

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

cloud-fan commented Oct 23, 2019

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

viirya commented Oct 23, 2019

Uh oh!

imback82 commented Oct 23, 2019

Uh oh!

Reviewers

Assignees

Labels

cloud-fan Oct 22, 2019 •

edited

Loading