You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/06/23 15:42:51 UTC
[GitHub] [hive] kgyrtkirk commented on a change in pull request #2380: HIVE-25233: Removing deprecated unix_timestamp UDF
kgyrtkirk commented on a change in pull request #2380:
URL: https://github.com/apache/hive/pull/2380#discussion_r657233348
##########
File path: itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
##########
@@ -1098,11 +1098,11 @@ public void testHttpRetryOnServerIdleTimeout() throws Exception {
String userName = System.getProperty("user.name");
Connection conn = getConnection(miniHS2.getJdbcURL(testDbName), userName, "password");
Statement stmt = conn.createStatement();
- stmt.execute("select from_unixtime(unix_timestamp())");
+ stmt.execute("select from_unixtime(to_unix_timestamp())");
// Sleep for longer than server's idletimeout and execute a query
TimeUnit.SECONDS.sleep(10);
try {
- stmt.execute("select from_unixtime(unix_timestamp())");
+ stmt.execute("select from_unixtime(to_unix_timestamp())");
Review comment:
this method call looks odd - `to_unix_timestamp` looks like a conversion function ; but without arguments its just odd - I think we should keep `unix_timestamp` as an alias
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
##########
@@ -523,7 +523,6 @@
system.registerGenericUDF("from_utc_timestamp", GenericUDFFromUtcTimestamp.class);
system.registerGenericUDF("to_utc_timestamp", GenericUDFToUtcTimestamp.class);
- system.registerGenericUDF("unix_timestamp", GenericUDFUnixTimeStamp.class);
Review comment:
please don't remove this - instead reference the remaining funciton
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
##########
@@ -523,7 +523,6 @@
system.registerGenericUDF("from_utc_timestamp", GenericUDFFromUtcTimestamp.class);
system.registerGenericUDF("to_utc_timestamp", GenericUDFToUtcTimestamp.class);
- system.registerGenericUDF("unix_timestamp", GenericUDFUnixTimeStamp.class);
Review comment:
after this you could probably undo some of the unnecessary changes to `to_unix_timestamp` in the tests
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
##########
@@ -523,7 +523,6 @@
system.registerGenericUDF("from_utc_timestamp", GenericUDFFromUtcTimestamp.class);
system.registerGenericUDF("to_utc_timestamp", GenericUDFToUtcTimestamp.class);
- system.registerGenericUDF("unix_timestamp", GenericUDFUnixTimeStamp.class);
system.registerGenericUDF("to_unix_timestamp", GenericUDFToUnixTimeStamp.class);
Review comment:
instead of keeping `GenericUDFToUnixTimeStamp` I think we should rename `GenericUDFToUnixTimeStamp` to `GenericUDFUnixTimeStamp` ; because that's more logical
--
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.
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