ayushtkn commented on code in PR #5643:
URL: https://github.com/apache/hive/pull/5643#discussion_r1958700459
##########
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 added a check that desc is of type ExprNodeConstantDesc, whether
`constantDesc` is acceptable or not I think the original logic should take
care, It should throw some DateTimeParsing exception, same as in case some one
provides non timestamp string
--
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]