[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user jiangxb1987 commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r220410022 --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala --- @@ -2348,4 +2348,17 @@ class HiveDDLSuite } } } + + test("desc formatted table should also show viewOriginalText for views") { +withView("v1") { + sql("CREATE VIEW v1 AS SELECT 1 AS value") + assert(sql("DESC FORMATTED v1").collect().containsSlice( +Seq( + Row("Type", "VIEW", ""), + Row("View Text", "SELECT 1 AS value", ""), + Row("View Original Text:", "SELECT 1 AS value", "") --- End diff -- @zheyuan28 This is intended, you shall create a view using previous versions of Spark, or create a view using Hive directly. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user zheyuan28 commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r220257132 --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala --- @@ -2348,4 +2348,17 @@ class HiveDDLSuite } } } + + test("desc formatted table should also show viewOriginalText for views") { +withView("v1") { + sql("CREATE VIEW v1 AS SELECT 1 AS value") + assert(sql("DESC FORMATTED v1").collect().containsSlice( +Seq( + Row("Type", "VIEW", ""), + Row("View Text", "SELECT 1 AS value", ""), + Row("View Original Text:", "SELECT 1 AS value", "") --- End diff -- @MaxGekk @gatorsmile Sorry for late, I was trying to use hive client to create the older view. But I find the [toHiveTable](https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala#L918) method will always use the expanded view text when I create view: https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala#L955. Is that reasonable we change that to: ```scala table.viewText.foreach { t => hiveTable.setViewExpandedText(t) } table.viewOriginalText.foreach { t => hiveTable.setViewOriginalText(t) } ``` Here is my new test case: ```scala test("SPARK-25459 desc formatted table for views created by older Spark") { withTable("hive_table") { withView("old_view") { spark.sql("CREATE TABLE hive_table AS SELECT 1 AS a, 2 AS b") val expandedView = "SELECT `gen_attr_0` AS `a`, `gen_attr_1` AS `b` FROM (SELECT " + "`gen_attr_0`, `gen_attr_1` FROM (SELECT `a` AS `gen_attr_0`, `b` AS " + "`gen_attr_1` FROM hive_table) AS gen_subquery_0) AS hive_table" val view = CatalogTable( identifier = TableIdentifier("old_view"), tableType = CatalogTableType.VIEW, storage = CatalogStorageFormat.empty, schema = new StructType().add("a", "int").add("b", "int"), viewText = Some(expandedView), viewOriginalText = Some("SELECT 1 AS a, 2 AS b") ) hiveContext.sessionState.catalog.createTable(view, ignoreIfExists = false) // Check the output rows. assert(sql("DESC FORMATTED old_view").collect().containsSlice( Seq( Row("Type", "VIEW", ""), Row("View Text", expandedView, ""), Row("View Original Text", "SELECT 1 AS a, 2 AS b", "") ) )) } } } ``` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r219727730 --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala --- @@ -2348,4 +2348,17 @@ class HiveDDLSuite } } } + + test("desc formatted table should also show viewOriginalText for views") { +withView("v1") { + sql("CREATE VIEW v1 AS SELECT 1 AS value") + assert(sql("DESC FORMATTED v1").collect().containsSlice( +Seq( + Row("Type", "VIEW", ""), + Row("View Text", "SELECT 1 AS value", ""), + Row("View Original Text:", "SELECT 1 AS value", "") --- End diff -- To do that, maybe using the Hive client to create a view, instead of Spark --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user MaxGekk commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r219670109 --- Diff: sql/core/src/test/resources/sql-tests/results/higher-order-functions.sql.out --- @@ -201,6 +201,7 @@ struct<> -- !query 20 output + --- End diff -- nit: unnecessary blank line --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user MaxGekk commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r219670142 --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala --- @@ -2348,4 +2348,17 @@ class HiveDDLSuite } } } + + test("desc formatted table should also show viewOriginalText for views") { +withView("v1") { + sql("CREATE VIEW v1 AS SELECT 1 AS value") + assert(sql("DESC FORMATTED v1").collect().containsSlice( +Seq( + Row("Type", "VIEW", ""), + Row("View Text", "SELECT 1 AS value", ""), + Row("View Original Text:", "SELECT 1 AS value", "") --- End diff -- Can you write a test where `View Text` and `View Original Text` are different. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user MaxGekk commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r219669978 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -331,6 +332,7 @@ case class CatalogTable( comment.foreach(map.put("Comment", _)) if (tableType == CatalogTableType.VIEW) { viewText.foreach(map.put("View Text", _)) + viewOriginalText.foreach(map.put("View Original Text:", _)) --- End diff -- Looking at line above and below, `:` should be removed for consistency. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22458: [SPARK-25459] Add viewOriginalText back to Catalo...
Github user jiangxb1987 commented on a diff in the pull request: https://github.com/apache/spark/pull/22458#discussion_r219370221 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala --- @@ -467,9 +467,9 @@ private[hive] class HiveClientImpl( properties = filteredProperties, stats = readHiveStats(properties), comment = comment, -// In older versions of Spark(before 2.2.0), we expand the view original text and store -// that into `viewExpandedText`, and that should be used in view resolution. So we get -// `viewExpandedText` instead of `viewOriginalText` for viewText here. --- End diff -- This comment is for `viewText`, please rephrase and keep it, thanks! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org