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.

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

[function] SELECT VERSION() #782

Merged
merged 7 commits into from
Jun 9, 2021
Merged

Conversation

PsiACE
Copy link
Member

@PsiACE PsiACE commented Jun 8, 2021

Signed-off-by: Chojan Shang psiace@outlook.com

I hereby agree to the terms of the CLA available at: https://datafuse.rs/policies/cla/

Summary

try add version() function.

mysql> SELECT VERSION();
+-----------------------+
| VERSION(FuseQuery v-) |
+-----------------------+
| FuseQuery v-          |
+-----------------------+
1 row in set (0.00 sec)

Changelog

  • New Feature

Related Issues

Fixes #762

Test Plan

Unit Tests

Stateless Tests

Signed-off-by: Chojan Shang <psiace@outlook.com>
@databend-bot databend-bot added the pr-feature this PR introduces a new feature to the codebase label Jun 8, 2021
@databend-bot
Copy link
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@PsiACE
Copy link
Member Author

PsiACE commented Jun 8, 2021

Basically finished, just need some tests and documentation.

@BohuTANG Could you please review it?

cc @drmingdrmer , I checked the output file, oops, vargen can't gen FUSE_COMMIT_VERSION. Just get cargo:rustc-env=FUSE_COMMIT_VERSION=.

@codecov-commenter
Copy link

Codecov Report

Merging #782 (c8acd95) into master (af6f4ad) will decrease coverage by 0%.
The diff coverage is 22%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #782    +/-   ##
=======================================
- Coverage      78%     77%    -1%     
=======================================
  Files         314     317     +3     
  Lines       17215   17362   +147     
=======================================
+ Hits        13481   13537    +56     
- Misses       3734    3825    +91     
Impacted Files Coverage Δ
common/datavalues/src/data_value.rs 30% <0%> (+<1%) ⬆️
common/datavalues/src/data_value_kernel.rs 40% <0%> (ø)
common/datavalues/src/macros.rs 60% <ø> (ø)
common/functions/src/udfs/version.rs 0% <0%> (ø)
common/planners/src/plan_expression_chain.rs 54% <0%> (+1%) ⬆️
common/planners/src/plan_expression_function.rs 87% <0%> (-5%) ⬇️
common/planners/src/plan_rewriter.rs 45% <0%> (-1%) ⬇️
...ery/src/datasources/system/functions_table_test.rs 93% <ø> (-1%) ⬇️
fusequery/query/src/sql/plan_parser.rs 77% <12%> (-3%) ⬇️
common/datavalues/src/data_group_value.rs 15% <15%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9c6fa1...c8acd95. Read the comment docs.

@@ -0,0 +1,53 @@
// Copyright 2020-2021 The Datafuse Authors.
Copy link
Member

Choose a reason for hiding this comment

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

Cool, but why we need the udfs function?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, got it.
It's ok to me.

@drmingdrmer
Copy link
Member

drmingdrmer commented Jun 9, 2021

cc @drmingdrmer , I checked the output file, oops, vargen can't gen FUSE_COMMIT_VERSION. Just get cargo:rustc-env=FUSE_COMMIT_VERSION=.

@PsiACE
Sorry but I do not get it. Have you had a problem using this const var? What do you expect and what did you actually get?

The build.rs generates FUSE_COMMIT_VERSION as an env var. Config loads the env and saves it in a const var.

https://github.com/datafuselabs/datafuse/blob/c2cc5952dd9fd7e6adad19389db8bfdf654a4c0c/common/building/src/lib.rs#L49-L61

https://github.com/datafuselabs/datafuse/blob/c2cc5952dd9fd7e6adad19389db8bfdf654a4c0c/fusequery/query/src/configs/config.rs#L10

@PsiACE
Copy link
Member Author

PsiACE commented Jun 9, 2021

What do you expect and what did you actually get?

I may need a value, but get nothing.

@BohuTANG
Copy link
Member

BohuTANG commented Jun 9, 2021

What do you expect and what did you actually get?

I may need a value, but get nothing.

Here we should use:
config::FUSE_COMMIT_VERSION
https://github.com/datafuselabs/datafuse/blob/master/fusequery/query/src/bin/fuse-query.rs#L33-L36

@drmingdrmer
Copy link
Member

What do you expect and what did you actually get?

I may need a value, but get nothing.

Confirmed It's my fault. Working on it.

@sundy-li
Copy link
Member

sundy-li commented Jun 9, 2021

Is it possible to register the feature simd to the version description?

Currently, the arrow is not using simd feature, even we have

[features]
simd = ["arrow/simd"]

Or using

arrow = { git="https://github.com/apache/arrow-rs", rev = "0a165748c924b3752cc00ff86e56917427985872", features = ["simd", "prettyprint", "avx512"] }

Query:

mysql> SELECT sum(number % 3) FROM numbers_mt(10000000000);
+-------------------+
| sum((number % 3)) |
+-------------------+
|        9999999999 |
+-------------------+
1 row in set (12.58 sec)

Profile flamegraph:

image

Need to investigate that.

@PsiACE
Copy link
Member Author

PsiACE commented Jun 9, 2021

Is it possible to register the feature simd to the version description?

yep, i guess we can detect this feature. and gen version with #[cfg(feature="simd")]

Currently, the arrow is not using simd feature, even we have

[features]
simd = ["arrow/simd"]

now, this feature is not enabled by default, try --features simd.

@BohuTANG
Copy link
Member

BohuTANG commented Jun 9, 2021

Show SIMD feature in the version, we want it.

Chojan Shang and others added 2 commits June 9, 2021 15:21
@PsiACE
Copy link
Member Author

PsiACE commented Jun 9, 2021

Now, we can get version info like:

+----------------------------------------------------------------------------------------+
| VERSION(FuseQuery v-0.1.0-623cfb1(1.54.0-nightly-2021-06-09T07:40:55.026718153+00:00)) |
+----------------------------------------------------------------------------------------+
| FuseQuery v-0.1.0-623cfb1(1.54.0-nightly-2021-06-09T07:40:55.026718153+00:00)          |
+----------------------------------------------------------------------------------------+

Or simd version like:

+---------------------------------------------------------------------------------------------+
| VERSION(FuseQuery v-0.1.0-623cfb1-simd(1.54.0-nightly-2021-06-09T07:46:11.570439919+00:00)) |
+---------------------------------------------------------------------------------------------+
| FuseQuery v-0.1.0-623cfb1-simd(1.54.0-nightly-2021-06-09T07:46:11.570439919+00:00)          |
+---------------------------------------------------------------------------------------------+

thanks @drmingdrmer

cc @BohuTANG @sundy-li

@drmingdrmer
Copy link
Member

@PsiACE nice shot man!

PsiACE added 4 commits June 9, 2021 16:18
Signed-off-by: Chojan Shang <psiace@outlook.com>
Signed-off-by: Chojan Shang <psiace@outlook.com>
Signed-off-by: Chojan Shang <psiace@outlook.com>
Signed-off-by: Chojan Shang <psiace@outlook.com>
@PsiACE PsiACE marked this pull request as ready for review June 9, 2021 09:44
+------------+
| default |
+------------+
+-------------------+
Copy link
Member

Choose a reason for hiding this comment

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

💯

@@ -152,6 +152,7 @@ nav:
- SIPHASH: sqlstatement/hash-functions/siphash.md
- Information Functions:
- DATABASE: sqlstatement/information-functions/database.md
- VERSION: sqlstatement/information-functions/version.md
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

@BohuTANG BohuTANG left a comment

Choose a reason for hiding this comment

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

LGTM

@databend-bot
Copy link
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot databend-bot merged commit 5015d73 into databendlabs:master Jun 9, 2021
@PsiACE PsiACE deleted the version() branch June 9, 2021 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[function] SELECT VERSION()
6 participants