[GitHub] spark pull request #18709: [SPARK-21504] [SQL] Add spark version info into t...

[asfgit]

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...

[cloud-fan]

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...

[gatorsmile]

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...

[gatorsmile]

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...

[gatorsmile]

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...

[cloud-fan]

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...

[cloud-fan]

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...

[cloud-fan]

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...

[gatorsmile]

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...

[gatorsmile]

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...

[cloud-fan]

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...

[cloud-fan]

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...

[cloud-fan]

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...

[cloud-fan]

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...

[gatorsmile]

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...

[gatorsmile]

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