HyunWooZZ commented on code in PR #46416:
URL: https://github.com/apache/arrow/pull/46416#discussion_r2085847671
##########
cpp/src/arrow/filesystem/gcsfs.cc:
##########
@@ -353,7 +353,9 @@ class GcsFileSystem::Impl {
// matches the prefix we assume it is a directory.
std::string canonical = internal::EnsureTrailingSlash(path.object);
auto list_result = client_.ListObjects(path.bucket,
gcs::Prefix(canonical));
- if (list_result.begin() != list_result.end()) {
+
+ // Check that the result is valid before determining whether the list is
empty.
+ if (list_result.begin() != list_result.end() && *list_result.begin()) {
Review Comment:
>Our empty result usage is wrong, right?
Yes you are right.
Basically without any permission of GCS, this part is true.
`list_result.begin() != list_result.end()`
> Could you add a test for this case?
I will add some testcases. :)
##########
cpp/src/arrow/filesystem/gcsfs.cc:
##########
@@ -353,7 +353,9 @@ class GcsFileSystem::Impl {
// matches the prefix we assume it is a directory.
std::string canonical = internal::EnsureTrailingSlash(path.object);
auto list_result = client_.ListObjects(path.bucket,
gcs::Prefix(canonical));
- if (list_result.begin() != list_result.end()) {
+
+ // Check that the result is valid before determining whether the list is
empty.
+ if (list_result.begin() != list_result.end() && *list_result.begin()) {
Review Comment:
>Our empty result usage is wrong, right?
Yes you are right.
Basically without any permission of GCS, this part is true.
`list_result.begin() != list_result.end()`
> Could you add a test for this case?
I will add some testcases. :)
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
