Skip to content

Refactor Hive S3 tests#18779

Merged
pettyjamesm merged 1 commit intoprestodb:masterfrom
dnanuti:s3-select-tests
Jan 3, 2023
Merged

Refactor Hive S3 tests#18779
pettyjamesm merged 1 commit intoprestodb:masterfrom
dnanuti:s3-select-tests

Conversation

@dnanuti
Copy link

@dnanuti dnanuti commented Dec 7, 2022

Move common methods for Hive File System tests setup to utility class.
Add more test cases in TestS3SelectPushdown.
More tests to be added, but I try to keep the PRs small for an easier review.

Test plan - (Please fill in how you tested your changes)
Locally run ./presto-hive-hadoop2/bin/run_hive_s3_tests.sh in Docker container and TestS3SelectPushdown unit tests.

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== NO RELEASE NOTE ==

@dnanuti dnanuti requested a review from a team as a code owner December 7, 2022 18:14
@dnanuti dnanuti requested a review from presto-oss December 7, 2022 18:14
@dnanuti dnanuti changed the title Refactor S3 tests Refactor Hive S3 tests Dec 7, 2022
@dnanuti
Copy link
Author

dnanuti commented Dec 8, 2022

@pettyjamesm please have a look on this when you find the time

@dnanuti dnanuti force-pushed the s3-select-tests branch 2 times, most recently from 160afd0 to bb6360a Compare December 8, 2022 17:28
@dnanuti dnanuti force-pushed the s3-select-tests branch 3 times, most recently from 58ae4de to 8fbc53f Compare December 9, 2022 15:44
@dnanuti dnanuti force-pushed the s3-select-tests branch 2 times, most recently from 452abb1 to 456b7d1 Compare December 13, 2022 16:29
Copy link
Contributor

@pettyjamesm pettyjamesm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat shortcut. I guess we don't really need the specific -copyFromLocal semantics enforced here

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

Move common methods for Hive File System tests setup to utility class.
Add more test cases for S3 Select Pushdown enabling.
@pettyjamesm pettyjamesm merged commit 974bbba into prestodb:master Jan 3, 2023
@pettyjamesm
Copy link
Contributor

Merged, thanks!

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.

3 participants