- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.7k
Add integration test for IO operations for listing tables queries #18229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b2924b7    to
    3db4b0e      
    Compare
  
    3db4b0e    to
    ba4e802      
    Compare
  
    ba4e802    to
    981bc5c      
    Compare
  
    | @r" | ||
| RequestCountingObjectStore() | ||
| Total Requests: 2 | ||
| - HEAD path=csv_table.csv | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the idea is here we can see what object store requests are made for various operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, we can add test if we have optimization to verify after this merged.
981bc5c    to
    4b4a9ec      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work, thank you @alamb !
| ------- Object Store Request Summary ------- | ||
| RequestCountingObjectStore() | ||
| Total Requests: 4 | ||
| - LIST prefix=data | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BlakeOrth here is where I envision we can add tests for the current (and potential changes) for LISTING with partitioned tables
| Let's start with this set of tests and we can iterate as we use them to show other features (like listing tables with partitions) | 
| Thanks @zhuqi-lucas | 
Which issue does this PR close?
Rationale for this change
As we spend more effort optimizing the number of IO requests made during various scenarios, we need to ensure we have test coverage to:
What changes are included in this PR?
Add a new integration test that verifies what IO operations happen when creating and querying listing tables
Are these changes tested?
It is all tests
Are there any user-facing changes?
No, only tests