Lars Volker has posted comments on this change. Change subject: IMPALA-4403: Implement SHOW RANGE PARTITIONS for Kudu tables ......................................................................
Patch Set 2: (5 comments) Thanks for the reviews, please see PS3. http://gerrit.cloudera.org:8080/#/c/5390/2//COMMIT_MSG Commit Message: Line 1: Parent: 87fd39ad (IMPALA-4477: Bump Kudu version to latest master (60aa54e)) > can you add a test cases: Done. 1) by adding a "show range partitions" query to kudu_alter.test. I change the analysis to throw an exception for non-range partitioned tables. 2) added test AnalyzeDDLTest#TestShowRangePartitions() I'll work on 3) next while waiting for the next round of review comments. http://gerrit.cloudera.org:8080/#/c/5390/2/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: PS2, Line 187: PARTITION > PARTITIONS Done http://gerrit.cloudera.org:8080/#/c/5390/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: Line 421: resultSchema.addToColumns(new TColumn("Partition Specifier", Type.STRING.toThrift())); > Maybe Range Partition Specifier? Done Line 421: resultSchema.addToColumns(new TColumn("Partition Specifier", Type.STRING.toThrift())); > In the Kudu web UI we are calling it 'RANGE (col1, col2, col3, ...) PARTITI Thanks Dan. I changed it to RANGE (col1, col2, col3, ...). http://gerrit.cloudera.org:8080/#/c/5390/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: PS2, Line 364: // TODO Should we just pass params here? Or will this make the interfaces depend too : // much on thrift types? > we'll have to branch somewhere... I'm not sure it's clearly better to hand We could also branch here so we don't need an additional method there, but pass params to simplify the signatures. However it's stylistic and I'm also good with keeping this change minimal. -- To view, visit http://gerrit.cloudera.org:8080/5390 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf5b2fdd02938a42fa59ec98884e4ac915dd1f65 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-HasComments: Yes
