Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-29554][SQL] Add version SQL function #26209

Closed
wants to merge 5 commits into from
Closed

[SPARK-29554][SQL] Add version SQL function #26209

wants to merge 5 commits into from

Conversation

yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Oct 22, 2019 •
edited by dongjoon-hyun
Loading

What changes were proposed in this pull request?

hive> select version();
OK
3.1.1 rf4e0529634b6231a0072295da48af466cf2f10b7
Time taken: 2.113 seconds, Fetched: 1 row(s)

Why are the changes needed?

From hive behavior and I guess it is useful for debugging and developing etc.

Does this PR introduce any user-facing change?

add a misc func

How was this patch tested?

add ut

@SparkQA
Copy link

SparkQA commented Oct 22, 2019

Test build #112458 has finished for PR 26209 at commit 40126cc.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class Version() extends LeafExpression with CodegenFallback

srowen
srowen reviewed Oct 22, 2019
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Likewise seems niche, but easy enough to add. How about docs or Pyspark?

@SparkQA
Copy link

SparkQA commented Oct 23, 2019

Test build #112500 has finished for PR 26209 at commit dc4e6f1.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yaooqinn
Copy link
Member Author

Not familiar with pyspark, but, does it already has its own implementation?

spark/python/pyspark/version.py

Line 19 in 9bf397c

__version__ = "3.0.0.dev0"

@SparkQA
Copy link

SparkQA commented Oct 23, 2019

Test build #112501 has finished for PR 26209 at commit 3fb8199.

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

MaxGekk
MaxGekk reviewed Oct 23, 2019
View reviewed changes
Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

The same and more info have been available to users already:

scala> println(org.apache.spark.SPARK_VERSION)
3.0.0-SNAPSHOT

scala> println(org.apache.spark.SPARK_REVISION)
b4280705cb98a5e03529b638b2c1cf4213db6197

scala> println(org.apache.spark.SPARK_BRANCH)
interval-mul-div

scala> println(org.apache.spark.SPARK_BUILD_DATE)
2019-10-18T16:51:42Z

@srowen
Copy link
Member

srowen commented Oct 23, 2019

True, I guess the point is to expose it in SQL for Hive parity.

@yaooqinn yaooqinn requested review from maropu and srowen October 25, 2019 09:17
maropu
maropu approved these changes Oct 25, 2019
View reviewed changes
Copy link
Member

@maropu maropu left a comment

Choose a reason for hiding this comment

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

LGTM @cloud-fan @srowen

dongjoon-hyun
dongjoon-hyun requested changes Oct 25, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi, All.
Sorry, but let's not add a new expression because we need only SQL support.
We can add a new function without adding the expressions.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29554][SQL] Add suppport for misc function - version [SPARK-29554][SQL] Add version SQL function Oct 25, 2019
dongjoon-hyun
dongjoon-hyun requested changes Oct 25, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Hi, @yaooqinn . I left a few additional comments.

@yaooqinn
Copy link
Member Author

@dongjoon-hyun Thanks a lot. Updated as you request

dongjoon-hyun
dongjoon-hyun approved these changes Oct 26, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

+1, LGTM. (Pending Jenkins).
Thank you for updating, @yaooqinn .

@SparkQA
Copy link

SparkQA commented Oct 26, 2019

Test build #112702 has finished for PR 26209 at commit 48c667f.

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

@dongjoon-hyun
Copy link
Member

Thank you for your contribution, @yaooqinn .
Thank you for your reviews, @srowen , @maropu , @MaxGekk .
Merged to master.

@HyukjinKwon
Copy link
Member

+1

@yaooqinn yaooqinn deleted the SPARK-29554 branch October 28, 2019 09:23
gatorsmile
gatorsmile reviewed Nov 23, 2019
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
usage = """_FUNC_() - Returns the Spark version. The string contains 2 fields, the first being a release version and the second being a git revision.""",
since = "3.0.0")
// scalastyle:on line.size.limit
case class Version() extends LeafExpression with CodegenFallback {
Copy link
Member

Choose a reason for hiding this comment

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

Could we change the class name to SparkVersion()? Version() is too general as a class name.

Copy link
Member

Choose a reason for hiding this comment

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

cc @cloud-fan @Ngone51

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

+1

Copy link
Member

Choose a reason for hiding this comment

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

+1.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@dongjoon-hyun dongjoon-hyun mentioned this pull request Nov 24, 2019
Closed
dongjoon-hyun added a commit that referenced this pull request Nov 24, 2019
@dongjoon-hyun
[SPARK-29554][SQL][FOLLOWUP] Rename Version to SparkVersion
13338ea 
### What changes were proposed in this pull request?

This is a follow-up of #26209 .
This renames class `Version` to class `SparkVersion`.

### Why are the changes needed?

According to the review comment, this uses more specific class name.

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Pass the Jenkins with the existing tests.

Closes #26647 from dongjoon-hyun/SPARK-29554.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maropu maropu maropu approved these changes

@MaxGekk MaxGekk MaxGekk left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

@gatorsmile gatorsmile gatorsmile left review comments

@srowen srowen Awaiting requested review from srowen

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@yaooqinn @SparkQA @srowen @dongjoon-hyun @HyukjinKwon @maropu @MaxGekk @gatorsmile