Skip to content

Comments

Handle empty directories with content-type application/octet-stream also as directories#20603

Merged
imjalpreet merged 1 commit intoprestodb:masterfrom
imjalpreet:Issue20310
Aug 23, 2023
Merged

Handle empty directories with content-type application/octet-stream also as directories#20603
imjalpreet merged 1 commit intoprestodb:masterfrom
imjalpreet:Issue20310

Conversation

@imjalpreet
Copy link
Member

@imjalpreet imjalpreet commented Aug 18, 2023

Description

Certain systems create empty directories with content-type as application/octet-stream rather than application/x-directory due to which we see failures when trying to create an external table on top of such directories as Presto does not recognize them as directories.

Motivation and Context

Fixes #20310

Impact

This PR will allow Presto to recognize such directories

Test Plan

Tested the fix with Minio.
Before this fix, got the below error while trying to create an external table with the location as an empty directory in Minio

presto:imjalpreet_db> create table imjalpreet_content_type(a int, b int) WITH (external_location='s3a://imjalpreet-minio/imjalpreet-content-type/');
Query 20230818_102509_00006_8ppz6 failed: External location must be a directory

After the fix, create table ran successfully on the same empty directory:

presto:imjalpreet_db> create table imjalpreet_content_type(a int, b int) WITH (external_location='s3a://imjalpreet-minio/imjalpreet-content-type/');
CREATE TABLE

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Hive Changes
* Handle empty directories with content-type application/octet-stream also as directories (:issue:`20310`)

@imjalpreet imjalpreet self-assigned this Aug 18, 2023
@imjalpreet imjalpreet requested a review from a team as a code owner August 18, 2023 13:43
@imjalpreet imjalpreet requested a review from presto-oss August 18, 2023 13:43
Copy link
Contributor

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and confirmed that this fixes the issue - LGTM!

@imjalpreet
Copy link
Member Author

Thank you @tdcmeehan and @kiersten-stokes for reviewing the PR. I will go ahead and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strict check of the s3 directory object content type.

3 participants