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 2020/09/02 20:05:50 UTC
[GitHub] [hive] rajkrrsingh opened a new pull request #1460: Hive 24113
rajkrrsingh opened a new pull request #1460:
URL: https://github.com/apache/hive/pull/1460
### What changes were proposed in this pull request?
Following query will trigger the getPartitionsByExpr call at HMS, HMS will try to evaluate the filter based on the PartitionExpressionForMetastore proxy, this proxy uses the QL packages to evaluate the filter and call GenericUDFToUnixTimeStamp.
select * from table_name where hour between from_unixtime(unix_timestamp('2020090120', 'yyyyMMddHH') - 1*60*60, 'yyyyMMddHH') and from_unixtime(unix_timestamp('2020090122', 'yyyyMMddHH') + 2*60*60, 'yyyyMMddHH');
I think SessionState in the code path will always be NULL thats why it hit the NPE. to avoid NPE I will be checking if SessionState is null and based on that will initialize it.
### Why are the changes needed?
This is BUG when SessionState is accessed in HMS.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
I tried apply the changes on my local repro and it worked as expected.
----------------------------------------------------------------
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
[GitHub] [hive] rajkrrsingh commented on a change in pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
rajkrrsingh commented on a change in pull request #1460:
URL: https://github.com/apache/hive/pull/1460#discussion_r490607794
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFromUnixTime.java
##########
@@ -107,6 +107,10 @@ public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumen
}
if (timeZone == null) {
+ if (SessionState.get() == null) {
+ SessionState ss = new SessionState(new HiveConf());
+ SessionState.setCurrentSessionState(ss);
Review comment:
* sorry I did not follow this correctly, I think SessionState will not be available in HMS, can you please guide me how can I retrieve it from new HiveConf()
* yes that's a rare case while filtering the partition by expression, a query with filter condition having timestamp/date as predicate e.g. following query
select * from table_name where hour between from_unixtime(unix_timestamp('2020090120', 'yyyyMMddHH') - 1*60*60, 'yyyyMMddHH') and from_unixtime(unix_timestamp('2020090122', 'yyyyMMddHH') + 2*60*60, 'yyyyMMddHH');
in case of PartitionExpressionForMetastore proxy, the filter expression is based on ql class/package and thats where user will see such exception.
* I think its difficult to reproduce with embedded HMS.
----------------------------------------------------------------
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
[GitHub] [hive] rajkrrsingh commented on a change in pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
rajkrrsingh commented on a change in pull request #1460:
URL: https://github.com/apache/hive/pull/1460#discussion_r490607794
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFromUnixTime.java
##########
@@ -107,6 +107,10 @@ public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumen
}
if (timeZone == null) {
+ if (SessionState.get() == null) {
+ SessionState ss = new SessionState(new HiveConf());
+ SessionState.setCurrentSessionState(ss);
Review comment:
* sorry I did not follow this correctly, I think SessionState will not be available in HMS, can you please guide me how can I retrieve it from new HiveConf(), do you want me do something like new HiveConf().getLocalTimeZone() ?
* yes that's a rare case while filtering the partition by expression, a query with filter condition having timestamp/date as predicate e.g. following query
select * from table_name where hour between from_unixtime(unix_timestamp('2020090120', 'yyyyMMddHH') - 1*60*60, 'yyyyMMddHH') and from_unixtime(unix_timestamp('2020090122', 'yyyyMMddHH') + 2*60*60, 'yyyyMMddHH');
in case of PartitionExpressionForMetastore proxy, the filter expression is based on ql class/package and thats where user will see such exception.
* I think its difficult to reproduce with embedded HMS.
----------------------------------------------------------------
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
[GitHub] [hive] rajkrrsingh commented on pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
rajkrrsingh commented on pull request #1460:
URL: https://github.com/apache/hive/pull/1460#issuecomment-714908537
> I had a second look at this change, and instead of creating a new SessionState to get the timezone I would do the following:
> if (timeZone == null) {
> timeZone = SessionState.get() == null ? new HiveConf().getLocalTimeZone() : SessionState.get().getConf()
> .getLocalTimeZone();
> formatter.setTimeZone(TimeZone.getTimeZone(timeZone));
> }
updated the pull request, please review. Thanks
----------------------------------------------------------------
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
[GitHub] [hive] lcspinter closed pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
lcspinter closed pull request #1460:
URL: https://github.com/apache/hive/pull/1460
----------------------------------------------------------------
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
[GitHub] [hive] lcspinter commented on pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
lcspinter commented on pull request #1460:
URL: https://github.com/apache/hive/pull/1460#issuecomment-715392816
@rajkrrsingh Merged to master. Thanks for the patch
----------------------------------------------------------------
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
[GitHub] [hive] lcspinter merged pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
lcspinter merged pull request #1460:
URL: https://github.com/apache/hive/pull/1460
----------------------------------------------------------------
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
[GitHub] [hive] kgyrtkirk commented on a change in pull request #1460: Hive 24113
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1460:
URL: https://github.com/apache/hive/pull/1460#discussion_r488578302
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFromUnixTime.java
##########
@@ -107,6 +107,10 @@ public ObjectInspector initialize(ObjectInspector[] arguments) throws UDFArgumen
}
if (timeZone == null) {
+ if (SessionState.get() == null) {
+ SessionState ss = new SessionState(new HiveConf());
+ SessionState.setCurrentSessionState(ss);
Review comment:
please don't start a new session here
* in case you don't have a session - you may read this info from a `new HiveConf()`
* bumping into this issue from the HMS side is kinda unexpected - could you please leave a note about why this is needed here
* could you cover this issue with a testcase - or that's not possible because we are running embedded metastore in the tests?
----------------------------------------------------------------
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