alamb commented on code in PR #9259:
URL: https://github.com/apache/arrow-datafusion/pull/9259#discussion_r1505850471
##########
datafusion/sql/src/planner.rs:
##########
@@ -502,7 +502,14 @@ pub fn object_name_to_table_reference(
let ObjectName(idents) = object_name;
idents_to_table_reference(idents, enable_normalization)
}
-
+fn substitute_tilde(cur: String) -> String {
Review Comment:
I am a little worried about the securoty implications of putting tilde
expansion into the sql planner itself as this will cause *all* identifiers that
tsart with `~` to be replaced
like `SELECT * from ~foo`
I think this tilde expansion should be done only in the datafusion-cli code
-- specifically I think somewhere like
https://github.com/apache/arrow-datafusion/blob/e62240969135e2236d100c8c0c01546a87950a80/datafusion-cli/src/catalog.rs#L163
(which is where it handles converting unknown table names to urls)
##########
datafusion/sql/src/planner.rs:
##########
@@ -563,3 +570,21 @@ pub fn object_name_to_qualifier(
.collect::<Vec<_>>()
.join(" AND ")
}
+#[test]
+fn test_substitute_tilde_with_home() {
+ std::env::set_var("HOME", "/home/user");
+ let input =
"~/Code/arrow-datafusion/benchmarks/data/tpch_sf1/part/part-0.parquet";
+ let expected =
+
"/home/user/Code/arrow-datafusion/benchmarks/data/tpch_sf1/part/part-0.parquet";
+ let actual = substitute_tilde(input.to_string());
+ assert_eq!(actual, expected);
+ std::env::remove_var("HOME");
Review Comment:
This test will leave the `HOME` variable unset, even if it set at the
beginning of the test.
##########
datafusion/sql/src/planner.rs:
##########
@@ -502,7 +502,14 @@ pub fn object_name_to_table_reference(
let ObjectName(idents) = object_name;
idents_to_table_reference(idents, enable_normalization)
}
-
+fn substitute_tilde(cur: String) -> String {
+ if let Ok(usr_dir) = env::var("HOME") {
Review Comment:
Since datafusion-cli already uses `dirs`
https://github.com/apache/arrow-datafusion/blob/e62240969135e2236d100c8c0c01546a87950a80/datafusion-cli/Cargo.toml#L48
What about using https://docs.rs/dirs/latest/dirs/fn.home_dir.html ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
