Difference between TrinoFileSystem.listFiles contract and implementation

[findepi]

Trino version

479

Please describe the bug

TrinoFileSystem.listFiles(location) is documented as follows

Lists all files within the specified directory recursively. The location can be empty,
listing all files in the file system, otherwise the location must end with a slash.

At least the S3 implementation accepts locations not ending with a slash though:

trino/lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3FileSystem.java

Line 250 in ae6c57b

key += "/";

Either we should change implementation, or the contract.

cc @electrum

[findepi]

Side comment: it's unclear what exactly empty location means. Location.of("") or eg Location.of("s3://")

[ckstck]

Shall I make a PR which update the doc?

[findepi]

I would love to hear from David, but he might be busy. @ckstck sure, let's make a PR please.

[ckstck]

I would love to hear from David, but he might be busy. @ckstck sure, let's make a PR please.

Thanks, I have fixed it. Should I make a PR for the master branch?

-    * listing all files in the file system, otherwise the location must end with a slash. If the
-    * location does not exist, an empty iterator is returned.
+   * listing all files in the file system. The location is always interpreted as a directory,
+   * whether or not it ends with a slash. If the location does not exist, an empty iterator is returned.