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

feat: general variable substitution #196

Merged
merged 7 commits into from
Sep 19, 2023

Conversation

BugenZhao
Copy link
Collaborator

@BugenZhao BugenZhao commented Sep 18, 2023

Support environment variables and special variables (like $__TEST_DIR__) substitution with the crate subst. Basically, this is a general form of the previous replace_keywords.

This is helpful if we want to don't want to hardcode some information in the SQL, for example, authentication information.

control substitution on

query T
select $MY_USERNAME, ${MY_PASSWORD}, ${MY_INEXISTENT_PORT:11451}, ${MY_DATABASE:$MY_USERNAME-db}
----
sqllogictest, rust, 11451, sqllogictest-db

query T
select $__TEST_DIR__
----
/var/folders/h3/ky82klmd2ygfr58ppqm4js6m0000gn/T/.tmp63ZLdk

See substitution/basic.slt for more detailed usages.

In order to be compatible with other implementations, this feature is under the gate of control substitution and is by default disabled.

Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Copy link
Member

@wangrunji0408 wangrunji0408 left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines 326 to 327
/// Control whether or not to substitute variables in the SQL.
Substitution(OnOff),
Copy link
Member

Choose a reason for hiding this comment

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

Why not a simple bool? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, then I'll make try_from_str and as_str a trait.

tests/system_command/system_command.slt Show resolved Hide resolved
BugenZhao and others added 2 commits September 19, 2023 17:53
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: xxchan <[email protected]>
@xxchan xxchan enabled auto-merge (squash) September 19, 2023 12:12
@xxchan xxchan disabled auto-merge September 19, 2023 12:12
@xxchan xxchan enabled auto-merge (squash) September 19, 2023 12:13
@xxchan xxchan merged commit 263aa0c into risinglightdb:main Sep 19, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants