Anonymous Coward (339) has posted comments on this change. ( http://gerrit.cloudera.org:8080/18430 )
Change subject: IMPALA-11233: Unset all query option ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/18430/6/fe/src/main/java/org/apache/impala/analysis/SetStmt.java File fe/src/main/java/org/apache/impala/analysis/SetStmt.java: http://gerrit.cloudera.org:8080/#/c/18430/6/fe/src/main/java/org/apache/impala/analysis/SetStmt.java@30 PS6, Line 30: private final boolean isSetAll_; : private boolean isUnsetAll_; Seems like the `SetStmt` can only be one of the following modes: - set one (key, value) - set all - unset all How about using an enum to indicate the modes, so that we can use `switch ... case ...` to replace the `if` statements in the `toSql()` and `toThrift()` functions. It can also help to improve the code readability and maintainability. The enum can be designed to have three options: - SetOne - SetAll - UnSetAll -- To view, visit http://gerrit.cloudera.org:8080/18430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iabf23622daab733ddab20dd3ca73af6c9bd5c250 Gerrit-Change-Number: 18430 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaoqing Gao <[email protected]> Gerrit-Reviewer: Anonymous Coward (339) Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xiaoqing Gao <[email protected]> Gerrit-Comment-Date: Fri, 27 May 2022 12:40:22 +0000 Gerrit-HasComments: Yes
