tustvold commented on code in PR #2394:
URL: https://github.com/apache/arrow-datafusion/pull/2394#discussion_r865114136
##########
data-access/src/object_store/mod.rs:
##########
@@ -102,3 +132,113 @@ pub trait ObjectStore: Sync + Send + Debug {
/// Get object reader for one file
fn file_reader(&self, file: SizedFile) -> Result<Arc<dyn ObjectReader>>;
}
+
+const GLOB_START_CHARS: [char; 3] = ['?', '*', '['];
+
+/// Determine whether the path contains a globbing character
+fn contains_glob_start_char(path: &str) -> bool {
+ path.chars().any(|c| GLOB_START_CHARS.contains(&c))
+}
+
+/// Filters the file_stream to only contain files that end with suffix
+async fn filter_suffix(
Review Comment:
I don't think this function needs to be async
##########
dev/pr-checks.sh:
##########
@@ -0,0 +1,22 @@
+#!/bin/bash
Review Comment:
Perhaps we could update the CI to run this script, to prevent it from
drifting?
##########
data-access/src/object_store/mod.rs:
##########
@@ -102,3 +132,113 @@ pub trait ObjectStore: Sync + Send + Debug {
/// Get object reader for one file
fn file_reader(&self, file: SizedFile) -> Result<Arc<dyn ObjectReader>>;
}
+
+const GLOB_START_CHARS: [char; 3] = ['?', '*', '['];
+
+/// Determine whether the path contains a globbing character
+fn contains_glob_start_char(path: &str) -> bool {
+ path.chars().any(|c| GLOB_START_CHARS.contains(&c))
+}
+
+/// Filters the file_stream to only contain files that end with suffix
+async fn filter_suffix(
+ file_stream: FileMetaStream,
+ suffix: &str,
+) -> Result<FileMetaStream> {
+ let suffix = suffix.to_owned();
+ Ok(Box::pin(file_stream.filter(move |fr| {
+ let has_suffix = match fr {
+ Ok(f) => f.path().ends_with(&suffix),
+ Err(_) => true,
+ };
+ async move { has_suffix }
+ })))
+}
+
+fn find_longest_search_path_without_glob_pattern(glob_pattern: &str) -> String
{
+ // in case the glob_pattern is not actually a glob pattern, take the
entire thing
+ if !contains_glob_start_char(glob_pattern) {
+ glob_pattern.to_string()
+ } else {
+ // take all the components of the path (left-to-right) which do not
contain a glob pattern
+ let components_in_glob_pattern =
+ Path::new(glob_pattern).components().collect::<Vec<_>>();
+ let mut path_buf_for_longest_search_path_without_glob_pattern =
PathBuf::new();
+ let mut encountered_glob = false;
+ for component_in_glob_pattern in components_in_glob_pattern {
+ let component_as_str =
+ component_in_glob_pattern.as_os_str().to_str().unwrap();
+ if !encountered_glob {
+ let component_str_is_glob =
contains_glob_start_char(component_as_str);
+ encountered_glob = component_str_is_glob;
+ if !encountered_glob {
+ path_buf_for_longest_search_path_without_glob_pattern
+ .push(component_in_glob_pattern);
+ }
+ }
+ }
+
+ let mut result = path_buf_for_longest_search_path_without_glob_pattern
+ .to_str()
+ .unwrap()
+ .to_string();
+ // when we're not at the root, append a separator
+ if path_buf_for_longest_search_path_without_glob_pattern
+ .components()
+ .count()
+ > 1
+ {
+ result.push(path::MAIN_SEPARATOR);
+ }
+ result
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[tokio::test]
+ async fn test_is_glob_path() -> Result<()> {
+ assert!(!contains_glob_start_char("/"));
+ assert!(!contains_glob_start_char("/test"));
+ assert!(!contains_glob_start_char("/test/"));
+ assert!(contains_glob_start_char("/test*"));
+ Ok(())
+ }
+
+ fn test_longest_base_path(input: &str, expected: &str) {
+ assert_eq!(
+ find_longest_search_path_without_glob_pattern(input),
Review Comment:
Does this mean that windows uses glob expressions with `/` as opposed to `\`?
##########
dev/pr-checks.sh:
##########
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+set -e
+cargo fmt --all -- --check
+#cargo clippy --all-targets --workspace -- -D warnings
Review Comment:
?
##########
data-access/src/object_store/mod.rs:
##########
@@ -102,3 +132,113 @@ pub trait ObjectStore: Sync + Send + Debug {
/// Get object reader for one file
fn file_reader(&self, file: SizedFile) -> Result<Arc<dyn ObjectReader>>;
}
+
+const GLOB_START_CHARS: [char; 3] = ['?', '*', '['];
+
+/// Determine whether the path contains a globbing character
+fn contains_glob_start_char(path: &str) -> bool {
+ path.chars().any(|c| GLOB_START_CHARS.contains(&c))
+}
+
+/// Filters the file_stream to only contain files that end with suffix
+async fn filter_suffix(
+ file_stream: FileMetaStream,
+ suffix: &str,
+) -> Result<FileMetaStream> {
+ let suffix = suffix.to_owned();
+ Ok(Box::pin(file_stream.filter(move |fr| {
+ let has_suffix = match fr {
+ Ok(f) => f.path().ends_with(&suffix),
+ Err(_) => true,
+ };
+ async move { has_suffix }
+ })))
+}
+
+fn find_longest_search_path_without_glob_pattern(glob_pattern: &str) -> String
{
+ // in case the glob_pattern is not actually a glob pattern, take the
entire thing
+ if !contains_glob_start_char(glob_pattern) {
+ glob_pattern.to_string()
+ } else {
+ // take all the components of the path (left-to-right) which do not
contain a glob pattern
+ let components_in_glob_pattern =
+ Path::new(glob_pattern).components().collect::<Vec<_>>();
+ let mut path_buf_for_longest_search_path_without_glob_pattern =
PathBuf::new();
+ let mut encountered_glob = false;
+ for component_in_glob_pattern in components_in_glob_pattern {
+ let component_as_str =
+ component_in_glob_pattern.as_os_str().to_str().unwrap();
+ if !encountered_glob {
+ let component_str_is_glob =
contains_glob_start_char(component_as_str);
+ encountered_glob = component_str_is_glob;
+ if !encountered_glob {
+ path_buf_for_longest_search_path_without_glob_pattern
+ .push(component_in_glob_pattern);
+ }
+ }
+ }
+
+ let mut result = path_buf_for_longest_search_path_without_glob_pattern
+ .to_str()
+ .unwrap()
+ .to_string();
+ // when we're not at the root, append a separator
+ if path_buf_for_longest_search_path_without_glob_pattern
+ .components()
+ .count()
+ > 1
+ {
+ result.push(path::MAIN_SEPARATOR);
+ }
+ result
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[tokio::test]
+ async fn test_is_glob_path() -> Result<()> {
+ assert!(!contains_glob_start_char("/"));
+ assert!(!contains_glob_start_char("/test"));
+ assert!(!contains_glob_start_char("/test/"));
+ assert!(contains_glob_start_char("/test*"));
+ Ok(())
+ }
+
+ fn test_longest_base_path(input: &str, expected: &str) {
+ assert_eq!(
+ find_longest_search_path_without_glob_pattern(input),
+ make_expected(input, expected),
+ "testing find_longest_search_path_without_glob_pattern with {}",
+ input
+ );
+ }
+
+ fn make_expected(input: &str, expected: &str) -> String {
Review Comment:
Some comments would help interpret what this is doing
--
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]
