> On July 27, 2016, 3:26 p.m., Aihua Xu wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFStringToMap.java, > > line 95 > > <https://reviews.apache.org/r/50503/diff/1/?file=1455162#file1455162line95> > > > > Are line 90- 93 already dealing with null delimiters ? Seems unnessary > > to check here?
I could do a unit test which lead to the scenario that the "soi_de1" variable was not null, but the result of "soi_de1.convert(arguments[1].get())" call was null, so the delimiter1 variable ended up being null. This caused a NPE in the split call. I could reproduce this scenario only from the unit test (testStringToMapWithNullDelimiters test), but not from beeline. Do you think this scenario can happen outside from the unit test? If not, then these additional null checks are indeed not necessary and I will remove them. - Marta ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50503/#review143738 ----------------------------------------------------------- On July 27, 2016, 2:51 p.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50503/ > ----------------------------------------------------------- > > (Updated July 27, 2016, 2:51 p.m.) > > > Review request for hive, Aihua Xu and Sergio Pena. > > > Bugs: HIVE-12954 > https://issues.apache.org/jira/browse/HIVE-12954 > > > Repository: hive-git > > > Description > ------- > > HIVE-12954: NPE with str_to_map on null strings > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFStringToMap.java > ed60fbf > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFStringToMap.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/50503/diff/ > > > Testing > ------- > > Added new unit test for GenericUDFStringToMap, please see the patch for > details. > Also did manual testing. > > > Thanks, > > Marta Kuczora > >
