okumin commented on code in PR #5643:
URL: https://github.com/apache/hive/pull/5643#discussion_r1957319071
##########
parser/src/java/org/apache/hadoop/hive/ql/parse/AlterClauseParser.g:
##########
@@ -493,22 +493,28 @@ alterStatementSuffixExecute
@after { gParent.popMsg(state); }
: KW_EXECUTE KW_ROLLBACK LPAREN (rollbackParam=(StringLiteral | Number))
RPAREN
-> ^(TOK_ALTERTABLE_EXECUTE KW_ROLLBACK $rollbackParam)
- | KW_EXECUTE KW_EXPIRE_SNAPSHOTS (LPAREN (expireParam=StringLiteral)
RPAREN)?
+ | KW_EXECUTE KW_EXPIRE_SNAPSHOTS (LPAREN (expireParam=expression) RPAREN)?
-> ^(TOK_ALTERTABLE_EXECUTE KW_EXPIRE_SNAPSHOTS $expireParam?)
| KW_EXECUTE KW_SET_CURRENT_SNAPSHOT LPAREN (snapshotParam=expression)
RPAREN
-> ^(TOK_ALTERTABLE_EXECUTE KW_SET_CURRENT_SNAPSHOT $snapshotParam)
| KW_EXECUTE KW_FAST_FORWARD sourceBranch=StringLiteral
(targetBranch=StringLiteral)?
-> ^(TOK_ALTERTABLE_EXECUTE KW_FAST_FORWARD $sourceBranch $targetBranch?)
| KW_EXECUTE KW_CHERRY_PICK snapshotId=Number
-> ^(TOK_ALTERTABLE_EXECUTE KW_CHERRY_PICK $snapshotId)
- | KW_EXECUTE KW_EXPIRE_SNAPSHOTS KW_BETWEEN (fromTimestamp=StringLiteral)
KW_AND (toTimestamp=StringLiteral)
+ | KW_EXECUTE KW_EXPIRE_SNAPSHOTS KW_BETWEEN
Review Comment:
```suggestion
| KW_EXECUTE KW_EXPIRE_SNAPSHOTS KW_BETWEEN
```
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/execute/AlterTableExecuteAnalyzer.java:
##########
@@ -158,19 +163,30 @@ private static AlterTableExecuteDesc
getExpireSnapshotDesc(TableName tableName,
} else if (children.size() == 3) {
ASTNode secondNode = (ASTNode) children.get(2);
String secondNodeText =
PlanUtils.stripQuotes(secondNode.getText().trim());
- TimestampTZ fromTime = TimestampTZUtil.parse(firstNodeText, timeZone);
- TimestampTZ toTime = TimestampTZUtil.parse(secondNodeText, timeZone);
+ TimestampTZ fromTime = TimestampTZUtil.parse(getTimeStampString(conf,
firstNode, firstNodeText), timeZone);
+ TimestampTZ toTime = TimestampTZUtil.parse(getTimeStampString(conf,
secondNode, secondNodeText), timeZone);
spec = new AlterTableExecuteSpec(EXPIRE_SNAPSHOT,
new ExpireSnapshotsSpec(fromTime.toEpochMilli(),
toTime.toEpochMilli()));
} else if (EXPIRE_SNAPSHOT_BY_ID_REGEX.matcher(firstNodeText).matches()) {
spec = new AlterTableExecuteSpec(EXPIRE_SNAPSHOT, new
ExpireSnapshotsSpec(firstNodeText));
} else {
- TimestampTZ time = TimestampTZUtil.parse(firstNodeText, timeZone);
+ TimestampTZ time = TimestampTZUtil.parse(getTimeStampString(conf,
firstNode, firstNodeText), timeZone);
spec = new AlterTableExecuteSpec(EXPIRE_SNAPSHOT, new
ExpireSnapshotsSpec(time.toEpochMilli()));
}
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
}
+ private static String getTimeStampString(HiveConf conf, ASTNode node, String
nodeText) throws SemanticException {
+ if (node.getChildCount() > 0) {
+ QueryState queryState = new
QueryState.Builder().withGenerateNewQueryId(false).withHiveConf(conf).build();
+ SemanticAnalyzer sem = (SemanticAnalyzer)
SemanticAnalyzerFactory.get(queryState, node);
+ ExprNodeDesc desc = sem.genExprNodeDesc(node, new RowResolver(), false,
true);
+ ExprNodeConstantDesc constantDesc = (ExprNodeConstantDesc) desc;
Review Comment:
I'd check if it is a constant or not. This throws ClassCastException.
```
shell.executeStatement("ALTER TABLE " + identifier.name() + " EXECUTE
EXPIRE_SNAPSHOTS(RAND())");
```
Additionally, we may check whether the type of `constantDesc` is acceptable
here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
