You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "deniskuzZ (via GitHub)" <gi...@apache.org> on 2023/09/07 10:26:07 UTC
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4641: HIVE-27654: Iceberg: Cherry-Pick commit to a branch.
deniskuzZ commented on code in PR #4641:
URL: https://github.com/apache/hive/pull/4641#discussion_r1318407226
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/execute/AlterTableExecuteAnalyzer.java:
##########
@@ -73,59 +75,85 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
inputs.add(new ReadEntity(table));
AlterTableExecuteDesc desc = null;
if (HiveParser.KW_ROLLBACK == executeCommandType.getType()) {
- AlterTableExecuteSpec<AlterTableExecuteSpec.RollbackSpec> spec;
- // the second child must be the rollback parameter
- ASTNode child = (ASTNode) command.getChild(1);
-
- if (child.getType() == HiveParser.StringLiteral) {
- ZoneId timeZone = SessionState.get() == null ? new HiveConf().getLocalTimeZone() : SessionState.get().getConf()
- .getLocalTimeZone();
- TimestampTZ time = TimestampTZUtil.parse(PlanUtils.stripQuotes(child.getText()), timeZone);
- spec = new AlterTableExecuteSpec(ROLLBACK, new RollbackSpec(TIME, time.toEpochMilli()));
- } else {
- spec = new AlterTableExecuteSpec(ROLLBACK, new RollbackSpec(VERSION,
- Long.valueOf(child.getText())));
- }
- desc = new AlterTableExecuteDesc(tableName, partitionSpec, spec);
+ desc = getRollbackDesc(tableName, partitionSpec, (ASTNode) command.getChild(1));
Review Comment:
could we use `switch` instead on if-else?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/execute/AlterTableExecuteAnalyzer.java:
##########
@@ -73,59 +75,85 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
inputs.add(new ReadEntity(table));
AlterTableExecuteDesc desc = null;
if (HiveParser.KW_ROLLBACK == executeCommandType.getType()) {
- AlterTableExecuteSpec<AlterTableExecuteSpec.RollbackSpec> spec;
- // the second child must be the rollback parameter
- ASTNode child = (ASTNode) command.getChild(1);
-
- if (child.getType() == HiveParser.StringLiteral) {
- ZoneId timeZone = SessionState.get() == null ? new HiveConf().getLocalTimeZone() : SessionState.get().getConf()
- .getLocalTimeZone();
- TimestampTZ time = TimestampTZUtil.parse(PlanUtils.stripQuotes(child.getText()), timeZone);
- spec = new AlterTableExecuteSpec(ROLLBACK, new RollbackSpec(TIME, time.toEpochMilli()));
- } else {
- spec = new AlterTableExecuteSpec(ROLLBACK, new RollbackSpec(VERSION,
- Long.valueOf(child.getText())));
- }
- desc = new AlterTableExecuteDesc(tableName, partitionSpec, spec);
+ desc = getRollbackDesc(tableName, partitionSpec, (ASTNode) command.getChild(1));
Review Comment:
could we use `switch` instead of if-else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org