Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3) ...................................................................... Patch Set 9: Code-Review+1 (3 comments) Looks good to me, just a question to clarify on the statement in one comment. http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java: http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558 PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN && : step.getAlterColumn().getDelta().hasImmutable())) { > should use 'hasImmutable()' but not 'getImmutable()'. Ah, great! Thanks for the clarification and the fix. http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2787 PS7, Line 2787: private static final class PingSupportsFeatureCallback implements Callback<Boolean, Object> { : @Override : public Boolean call(final Object resp) { : if (resp instanceof Exception) { : // The server returns an RpcRemoteException when the required feature is not supported. : // The exception should have an ERROR_INVALID_REQUEST error code and at least one : // unsupported feature flag. : if (resp instanceof RpcRemoteException && : ((RpcRemoteException) resp).getErrPB().getCode() == ERROR_INVALID_REQUEST && : ((RpcRemoteException) resp).getErrPB().getUnsupportedFeatureFlagsCount() >= 1) { : return false; : > Thanks for you information. Yes, that makes sense. Thanks! http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java: http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2549 PS9, Line 2549: we can : // test the function on the client side, the request will not be sent to server : // because it's validated in the client side. nit just for my education: Is that indeed so? I was under impression that the required feature flags from the RPC request are validated at the RPC level of a Kudu server. So, such a request wouldn't reach particular application code at the server side (e.g., AlterTable request requiring immutable columns feature wouldn't hit the code in CatalogManager::AlterTable()), but it will be processed by the generic RPC code running as a part of the kudu-master process. Am I missing something here? -- To view, visit http://gerrit.cloudera.org:8080/18993 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d Gerrit-Change-Number: 18993 Gerrit-PatchSet: 9 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Wed, 19 Oct 2022 17:57:13 +0000 Gerrit-HasComments: Yes
