[GitHub] [hive] StefanXiepj commented on pull request #1209: HIVE-22412: StatsUtils throw NPE when explain

[GitBox <gi...@apache.org>]

StefanXiepj commented on pull request #1209:
URL: https://github.com/apache/hive/pull/1209#issuecomment-654557578


   > Thanks for the contribution.
   > 
   > I need to look at it more, but if 'null' is not a valid option to pass into the constructors, then it should be prohibited: `Objects#requireNull` Please check where the null value is being passed in and instead have the calling class pass in an `Collections#EmptyList` if possible, or a new `ArrayList` otherwise.
   
   Thanks for your review. The details about this exeception stack is here:
    About Map, you can find it at `org.apache.hadoop.hive.ql.stats.StatsUtils#getSizeOfMap`:
   ```
     public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
       // This map is null
       Map<?, ?> map = scmoi.getWritableConstantValue();
       ObjectInspector koi = scmoi.getMapKeyObjectInspector();
       ObjectInspector voi = scmoi.getMapValueObjectInspector();
       long result = 0;
       // NPE will thrown at here
       for (Map.Entry<?, ?> entry : map.entrySet()) {
         result += getWritableSize(koi, entry.getKey());
         result += getWritableSize(voi, entry.getValue());
       }
       ...
     }
   ```
   Maybe we can fix it like this:
   ```
   public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
       // This map is null
       Map<?, ?> map = scmoi.getWritableConstantValue();
       // return zero when map is null
       if (null == map) {
           return 0;
       }
       ObjectInspector koi = scmoi.getMapKeyObjectInspector();
       ObjectInspector voi = scmoi.getMapValueObjectInspector();
       long result = 0;
       // NPE will thrown at here
       for (Map.Entry<?, ?> entry : map.entrySet()) {
         result += getWritableSize(koi, entry.getKey());
         result += getWritableSize(voi, entry.getValue());
       }
       ...
     }
   ```
   We might want to fix this in StatsUtils, but i don't think it is a good idea, it is better to fix it by initializing value for StandardConstantMapObjectInspector, StandardConstantListObjectInspector and StandardConstantStructObjectInspector.


----------------------------------------------------------------
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