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]

Reply via email to