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.

Sign up for GitHub

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

Jump to bottom

Disallow glob paths with ** that is not a folder segment #1820

Closed
jaychia opened this issue Jan 26, 2024 · 3 comments · Fixed by #3100
Closed

Disallow glob paths with ** that is not a folder segment #1820

jaychia opened this issue Jan 26, 2024 · 3 comments · Fixed by #3100
Labels
good first issue Good for newcomers

Comments

@jaychia
Copy link
Contributor

jaychia commented Jan 26, 2024

Is your feature request related to a problem? Please describe.

Daft currently only correctly understands the ** recursive wildcard when it is applied to a folder segment. I.e.

  • s3://bucket/**.csv is not valid
  • s3://bucket/**/*.csv is valid and conveys the intended behavior

We should provide an appropriate error message when we detect the former, or perhaps sanitize the input when we find instances of ** not enclosed by /.
@jaychia jaychia added the good first issue Good for newcomers label Jan 26, 2024
@nemo-1999
Copy link

I would like to work on this

@nemo-1999
Copy link

take

@conradsoon
Copy link
Contributor

conradsoon commented Oct 22, 2024 •
edited
Loading

hey @jaychia, i'd like to give this a shot :)

@conradsoon conradsoon mentioned this issue Oct 22, 2024
Merged
sagiahrac pushed a commit to sagiahrac/Daft that referenced this issue Nov 4, 2024
@conradsoon @sagiahrac
[FEAT]: Throw error for invalid ** usage outside folder segments (e.g…
d753b44 
…. /tmp/**.csv) (Eventual-Inc#3100)

Closes Eventual-Inc#1820.

Main issue seems to be that the `globset` crate is permissive for what
kind of pattern it builds (no error is thrown when we try to build a
pattern for `/tmp/**.csv`, for instance, so we have to check ourselves
for any such patterns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[FEAT]: Throw error for invalid ** usage outside folder segments (e.g. /tmp/**.csv) conradsoon/Daft
3 participants
@jaychia @conradsoon @nemo-1999