Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Jan 7, 2018

What changes were proposed in this pull request?

Previously running ANALYZE on temporary views produced NoSuchTableException even though the view was present. It is misleading. So it has to be changed to throw AnalysisException since it comes in the analysis part.

How was this patch tested?

Some unit tests were fixed to intercept AnalysisException instead of NoSuchTableException.

@cloud-fan
Copy link
Contributor

ok to test

@cloud-fan
Copy link
Contributor

what's the error message after this patch?

@SparkQA
Copy link

SparkQA commented Jan 8, 2018

Test build #85781 has finished for PR 20177 at commit 98b8711.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ghost
Copy link
Author

ghost commented Jan 8, 2018

The error message after the patch:
AnalysisException: ANALYZE TABLE is not supported on views

scala> sql("ANALYZE TABLE names COMPUTE STATISTICS")

org.apache.spark.sql.AnalysisException: ANALYZE TABLE is not supported on views.;
  at org.apache.spark.sql.execution.command.AnalyzeTableCommand.run(AnalyzeTableCommand.scala:38)
  at org.apache.spark.sql.execution.command.ExecutedCommandExec.sideEffectResult$lzycompute(commands.scala:70)
  at org.apache.spark.sql.execution.command.ExecutedCommandExec.sideEffectResult(commands.scala:68)
  at org.apache.spark.sql.execution.command.ExecutedCommandExec.executeCollect(commands.scala:79)
  at org.apache.spark.sql.Dataset$$anonfun$6.apply(Dataset.scala:186)
  at org.apache.spark.sql.Dataset$$anonfun$6.apply(Dataset.scala:186)
  at org.apache.spark.sql.Dataset$$anonfun$51.apply(Dataset.scala:3243)
  at org.apache.spark.sql.execution.SQLExecution$.withNewExecutionId(SQLExecution.scala:77)
  at org.apache.spark.sql.Dataset.withAction(Dataset.scala:3242)
  at org.apache.spark.sql.Dataset.<init>(Dataset.scala:186)
  at org.apache.spark.sql.Dataset$.ofRows(Dataset.scala:71)
  at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:638)

@cloud-fan
Copy link
Contributor

can you fix the test?

assertNoSuchTable(s"SHOW PARTITIONS $viewName")
assertNoSuchTable(s"ANALYZE TABLE $viewName COMPUTE STATISTICS")
assertNoSuchTable(s"ANALYZE TABLE $viewName COMPUTE STATISTICS FOR COLUMNS id")
assertAnalysisException(s"ANALYZE TABLE $viewName COMPUTE STATISTICS")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also check the error message to ensure the AnalysisException is not thrown from elsewhere.

@jiangxb1987
Copy link
Contributor

jiangxb1987 commented Jan 19, 2018

We shall also update AnalyzePartitionCommand and AnalyzeColumnCommand.

…ands - AnalyzeColum and AnalyzePartition. Fix tests to check if the error messages are proper.
@SparkQA
Copy link

SparkQA commented Jan 21, 2018

Test build #86424 has finished for PR 20177 at commit 0e873e5.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 21, 2018

Test build #86425 has finished for PR 20177 at commit 77e4d6d.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ghost
Copy link
Author

ghost commented Jan 21, 2018

I updated AnalyzePartitionCommand and AnalyzeColumnCommand. I noticed another one - According to SQLViewSuite, calling truncate on view must throw NoSuchTable, where as in TruncateTableCommand class I noticed this

    if (table.tableType == CatalogTableType.VIEW) {
      throw new AnalysisException(
        s"Operation not allowed: TRUNCATE TABLE on views: $tableIdentWithDB")
    }

A bug?

@SparkQA
Copy link

SparkQA commented Jan 21, 2018

Test build #86426 has finished for PR 20177 at commit 4d4e3fe.

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

@cloud-fan
Copy link
Contributor

actually it's not really useful to have NoSuchTableException, NoSuchFunctionException, etc. always using AnalysisException seems fine. CC @gatorsmile

@ghost
Copy link
Author

ghost commented Jan 23, 2018

I did't understand why the build is getting killed.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Jan 23, 2018

Test build #86506 has finished for PR 20177 at commit 4d4e3fe.

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

@jiangxb1987
Copy link
Contributor

Could you also add test cases that cover ANALYZE PARTITION and ANALYZE COLUMN queries?

…rtition on view throws AnalyzeException telling that it isn't supported on views.
@ghost
Copy link
Author

ghost commented Jan 30, 2018

Done. Added test cases to cover ANALYZE PARTITION and ANALYZE COLUMN queries.

@SparkQA
Copy link

SparkQA commented Jan 30, 2018

Test build #86791 has finished for PR 20177 at commit 4c86456.

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

@ghost ghost closed this May 18, 2018
@ghost ghost reopened this May 18, 2018
@gatorsmile
Copy link
Member

ok to test

@gatorsmile
Copy link
Member

cc @wzhfy

@SparkQA
Copy link

SparkQA commented May 19, 2018

Test build #90807 has finished for PR 20177 at commit 4c86456.

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

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants