Support files with locations ending with whitespace #18206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
findepi
requested review from
electrum,
dain,
losipiuk,
ebyhr,
alexjo2144 and
findinpath
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
from
d36e8ec to
a9c4f5a
Compare
losipiuk
reviewed
Jul 10, 2023
lib/trino-filesystem-s3/src/test/java/io/trino/filesystem/s3/AbstractTestS3FileSystem.java
Outdated
Show resolved
Hide resolved
losipiuk
approved these changes
Jul 10, 2023
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
from
a9c4f5a to
e49af31
Compare
findinpath
approved these changes
Jul 10, 2023
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
from
e49af31 to
5d3d471
Compare
findepi
commented
Jul 10, 2023
lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java
Show resolved
Hide resolved
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
2 times, most recently
from
bfd42c1 to
abb0ece
Compare
losipiuk
approved these changes
Jul 10, 2023
|
Were you able to register locations ending in whitespace previously with the Hadoop If it was normalized before, maybe we don't need to support these types of locations. |
// prints "s3://bucket/whitespace x"
System.out.println(new org.apache.hadoop.fs.Path("s3://bucket/whitespace ") + "x"); |
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
2 times, most recently
from
30a067f to
199d84e
Compare
Use blob contents different than their locations.
Whitespace, and especially trailing whitespace, may have confusing consequences when used in file locations so it's not recommended. However, whitespace, including trailing whitespace, is valid part of file locations and Trino should be able to read such files (objects). The removed assertions are mostly covered by the new test method being added, except that this new test is run for S3-like filesystems only and does not cover renames. Both aspects will be addressed in a following commit.
findepi
force-pushed
the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
from
199d84e to
c66d37d
Compare
|
(rebased to resolve conflicts) |
findepi
deleted the
findepi/support-files-with-locations-ending-with-whitespace-701307
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Whitespace, and especially trailing whitespace, may have confusing consequences when used in file locations so it's not recommended. However, whitespace, including trailing whitespace, is valid part of file locations and Trino should be able to read such files (objects).