Skip to content

Conversation

@hvanhovell
Copy link
Contributor

@hvanhovell hvanhovell commented Aug 15, 2016

What changes were proposed in this pull request?

This PR adds a field to subquery alias in order to make the usage of views in a resolved LogicalPlan more visible (and more understandable).

For example, the following view and query:

create view constants as select 1 as id union all select 1 union all select 42
select * from constants;

...now yields the following analyzed plan:

Project [id#39]
+- SubqueryAlias c, `default`.`constants`
   +- Project [gen_attr_0#36 AS id#39]
      +- SubqueryAlias gen_subquery_0
         +- Union
            :- Union
            :  :- Project [1 AS gen_attr_0#36]
            :  :  +- OneRowRelation$
            :  +- Project [1 AS gen_attr_1#37]
            :     +- OneRowRelation$
            +- Project [42 AS gen_attr_2#38]
               +- OneRowRelation$

How was this patch tested?

Added tests for the two code paths in SessionCatalogSuite (sql/core) and HiveMetastoreCatalogSuite (sql/hive)

@SparkQA
Copy link

SparkQA commented Aug 16, 2016

Test build #63811 has finished for PR 14657 at commit 6576496.

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

@hvanhovell hvanhovell changed the title [SPARK-17068][SQL] Make view-usage visible during analysis [WIP] [SPARK-17068][SQL] Make view-usage visible during analysis Aug 16, 2016
@hvanhovell
Copy link
Contributor Author

cc @rxin @sameeragarwal

@SparkQA
Copy link

SparkQA commented Aug 16, 2016

Test build #63841 has finished for PR 14657 at commit 7b39d2b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class HiveMetastoreCatalogSuite extends TestHiveSingleton with SQLTestUtils

// Skip projects and subquery aliases added by the Analyzer and the SQLBuilder.
def cleanQuery(p: LogicalPlan): LogicalPlan = p match {
case SubqueryAlias(_, child) => cleanQuery(child)
case SubqueryAlias(_, child, _) => cleanQuery(child)
Copy link
Contributor

Choose a reason for hiding this comment

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

not that big of a deal, but wouldn't it be easier and more robust if we just do

case s: SubqueryAlias => cleanQuery(s.child)
case p: Project => cleanQuery(p.child)
case child => child

@rxin
Copy link
Contributor

rxin commented Aug 16, 2016

Can you update the documentation of relevant classes to make sure we document this new contract clearly?

@SparkQA
Copy link

SparkQA commented Aug 16, 2016

Test build #63868 has finished for PR 14657 at commit ddd4ea0.

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

@rxin
Copy link
Contributor

rxin commented Aug 17, 2016

LGTM - merging in master.

@rxin
Copy link
Contributor

rxin commented Aug 17, 2016

(I didn't merge this in 2.0)

@asfgit asfgit closed this in f7c9ff5 Aug 17, 2016
val relationAlias = alias.getOrElse(table)
if (name.database.isDefined || !tempTables.contains(table)) {
val metadata = externalCatalog.getTable(db, table)
val view = Option(metadata.tableType).collect {
Copy link
Contributor

Choose a reason for hiding this comment

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

we are too conservative here, CatalogTable.tableType should never be null, and we use this assumption in a lot of places.

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.

4 participants