ARROW-9837: [Rust][DataFusion] Add provider for variable#8033
ARROW-9837: [Rust][DataFusion] Add provider for variable#8033wqc200 wants to merge 20 commits intoapache:masterfrom wqc200:master_sqlparser_variable
Conversation
There was a problem hiding this comment.
When the user USES, to create a new system and user_defined method, here is only for the user to understand.
The @@ variable is defined by the external system
rust/datafusion/src/logicalplan.rs
Outdated
There was a problem hiding this comment.
this naming is already being used by columns (one line above). Doesn't this make it confusing?
There was a problem hiding this comment.
Yes, i remove the code '#', the variable name itself start with an @ sign
|
Hi @wqc200 please rebase against master rather than merge from master once this is ready for review. |
| use crate::variable::VarProvider; | ||
|
|
||
| /// user defined variable | ||
| pub struct UserDefinedVar {} |
There was a problem hiding this comment.
This seems to be identical to SystemVar. I understand that we want to register separate variables for user-defined and system, but I don't think we need two separate implementations of the trait?
There was a problem hiding this comment.
Yes, I move it to test dir
| impl VarProvider for SystemVar { | ||
| /// get system variable value | ||
| fn get_value(&self, var_names: Vec<String>) -> Result<ScalarValue> { | ||
| let s = format!("{}-{}", "test".to_string(), var_names.concat()); |
There was a problem hiding this comment.
This seems to be test code but is not marked with #[test] as far as I can see.
There was a problem hiding this comment.
Yes, I move it to test dir
|
@wqc200 Thanks for making those changes. However, I still see merges in the commit history for this PR. I would suggest creating a new PR from master with these approved changes if you are not comfortable with rebasing this PR. See this article for more information about rebasing: https://andygrove.io/apache_arrow_git_tips/ |
Select @@Version;
@@Version is a variable, and if we want to get its value, we should get it from outside the system,