[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/18709 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r132094303 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -205,6 +205,9 @@ case class BucketSpec( * configured. * @param ignoredProperties is a list of table properties that are used by the underlying table * but ignored by Spark SQL yet. + * @param createVersion records the version of Spark that created this table metadata. The default + * is '2.2 or prior'. We expect it will be read from the catalog or filled by --- End diff -- the default is empty string. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r131995780 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +218,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = "", --- End diff -- The default will also impact the temporary view. How about just keeping it unchanged? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r131995378 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala --- @@ -700,6 +704,9 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat table = restoreDataSourceTable(table, provider) } +// Restore version info +val version: String = table.properties.getOrElse(CREATED_SPARK_VERSION, "") --- End diff -- I will change this to `2.2 or prior` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r131964192 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +218,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = "", --- End diff -- The reason why I did not set the current version by default is that I am afraid some bugs might use the newer version to overwrite the original value that is read from catalog. Thus, I am a little bit conservative here. I like your first proposal. `2.2 or prior` looks good to me. It is better than an empty string. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r131943455 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +218,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = "", --- End diff -- actually, can we use current version as default value, and when reading table metadata in `HiveExternalCatalog`, set the version to the value from table properties or `Prior to 2.3` if no such entry in table properties. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r131942350 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +218,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = "", --- End diff -- use `Unknown` as default value? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r129259658 --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala --- @@ -932,6 +934,7 @@ private[hive] object HiveClientImpl { table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe")) table.storage.properties.foreach { case (k, v) => hiveTable.setSerdeParam(k, v) } table.properties.foreach { case (k, v) => hiveTable.setProperty(k, v) } +hiveTable.setProperty(CREATED_SPARK_VERSION, table.createVersion) --- End diff -- seems better to do it in `HiveExternalCatalog`? this version string stuff is not related to hive. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128925557 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +217,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = org.apache.spark.SPARK_VERSION, --- End diff -- Done. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128925552 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -304,6 +305,7 @@ case class CatalogTable( if (owner.nonEmpty) map.put("Owner", owner) map.put("Created", new Date(createTime).toString) --- End diff -- Plan to backport this to the previous version. Thus, will not change it in this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128890400 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -345,6 +347,7 @@ object CatalogTable { val VIEW_QUERY_OUTPUT_PREFIX = "view.query.out." val VIEW_QUERY_OUTPUT_NUM_COLUMNS = VIEW_QUERY_OUTPUT_PREFIX + "numCols" val VIEW_QUERY_OUTPUT_COLUMN_NAME_PREFIX = VIEW_QUERY_OUTPUT_PREFIX + "col." + val SCHEMA_SPARK_VERSION = "spark.sql.create.version" --- End diff -- `CREATED_SPARK_VERSION`? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128890390 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -304,6 +305,7 @@ case class CatalogTable( if (owner.nonEmpty) map.put("Owner", owner) map.put("Created", new Date(createTime).toString) --- End diff -- not related, but seems `Created Time` is better? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128890385 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -304,6 +305,7 @@ case class CatalogTable( if (owner.nonEmpty) map.put("Owner", owner) map.put("Created", new Date(createTime).toString) map.put("Last Access", new Date(lastAccessTime).toString) +map.put("Create Version", createVersion) --- End diff -- Created Version? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128890382 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala --- @@ -217,6 +217,7 @@ case class CatalogTable( owner: String = "", createTime: Long = System.currentTimeMillis, lastAccessTime: Long = -1, +createVersion: String = org.apache.spark.SPARK_VERSION, --- End diff -- add parameter doc? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
Github user gatorsmile commented on a diff in the pull request: https://github.com/apache/spark/pull/18709#discussion_r128881870 --- Diff: sql/core/src/test/resources/sql-tests/results/describe-table-after-alter-table.sql.out --- @@ -25,6 +25,7 @@ Database default Table table_with_comment Created [not included in comparison] Last Access [not included in comparison] +Create Version [not included in comparison] --- End diff -- I manually verified all these version values are right. To avoid the unnecessary test result updates, hide the values. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...
GitHub user gatorsmile opened a pull request: https://github.com/apache/spark/pull/18709 [SPARK-21504] [SQL] Add spark version info into table metadata ## What changes were proposed in this pull request? This PR is to add the spark version info in the table metadata. When creating the table, this value is assigned. It can help users find which version of Spark was used to create the table. ## How was this patch tested? N/A You can merge this pull request into a Git repository by running: $ git pull https://github.com/gatorsmile/spark addVersion Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/18709.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #18709 commit ccbd3a96e5d9fe154f8adec172179fd0021eada2 Author: gatorsmile Date: 2017-07-21T22:22:02Z fix --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org