-
Notifications
You must be signed in to change notification settings - Fork 381
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
[#1769] feat(partition): Support listPartitions
for Hive catalog
#1799
Conversation
@mchades can you please fix the conflicts. |
@@ -50,15 +53,21 @@ public Response listPartitionNames( | |||
@PathParam("metalake") String metalake, | |||
@PathParam("catalog") String catalog, | |||
@PathParam("schema") String schema, | |||
@PathParam("table") String table) { | |||
@PathParam("table") String table, | |||
@QueryParam("verbose") @DefaultValue("false") boolean verbose) { |
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.
I would suggest to change the quey parameter to "details".
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.
ok
} else { | ||
String[] partitionNames = loadTable.supportPartitions().listPartitionNames(); | ||
return Utils.ok(new PartitionNameListResponse((partitionNames))); | ||
} |
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.
Also I think you miss the UTs here.
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.
fixed
@@ -211,6 +211,46 @@ public void testListPartitionNames() { | |||
Assertions.assertTrue(errorResp2.getMessage().contains("test exception")); | |||
} | |||
|
|||
@Test | |||
public void testListPartitions() { |
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.
Can you please add a test case returning empty partitions?
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.
added to IT
What changes were proposed in this pull request?
Support
listPartitions
for Hive catalogWhy are the changes needed?
Fix: #1769
Does this PR introduce any user-facing change?
no
How was this patch tested?
tests added