Todd Lipcon has posted comments on this change. Change subject: KUDU-1831. Java client does not check if the primary key columns are specified first ......................................................................
Patch Set 1: It seems like in the context of a projection, we don't want to label any columns as keys, and thus this probably passes because we don't propagate the "key" status from the table schema into the projection schema. If this were causing a problem, wouldn't the test added in the above-linked commit be failing now? If you think the coverage is lacking, can you propose a new unit test which fails with Jun's patch? Really the 'key(true)' vs 'key(false)' API is a bad one that we should try to deprecate over time in favor of a 'setPrimaryKey(List<String>)' type API (we've moved towards this in the C++ client). This paves the way for actually allowing key columns not listed first in the Schema object. -- To view, visit http://gerrit.cloudera.org:8080/5723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ca2a572801c964331ed65c630db28436fcaf86a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jun He <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: No
