> On Jan. 13, 2013, 8:15 p.m., Ashutosh Chauhan wrote: > > serde/src/java/org/apache/hadoop/hive/serde2/RegexSerDe.java, line 121 > > <https://reviews.apache.org/r/8931/diff/1/?file=247630#file247630line121> > > > > Are we sure that reference equality here is guaranteed to work? In > > other words, we don't need .equals() instead? > > Same question for all == comparisons on subsequent line as well. > > Shreepadma Venugopalan wrote: > I agree we should do .equals() to compare value rather than reference. > However, it's interesting that the test didn't run into an issue with the > reference comparison. I've fixed the reference comparison to value comparison.
That's because type names are declared as static final's so == always returns the same result as equals() but equals() makes us think less so I agree with both of you to change it to equals(). - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8931/#review15305 ----------------------------------------------------------- On Jan. 14, 2013, 8:46 p.m., Shreepadma Venugopalan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8931/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2013, 8:46 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Description > ------- > > This patch enhances regex serde to parse column types other than STRING. Only > primitive types are supported. > > > This addresses bug HIVE-3004. > https://issues.apache.org/jira/browse/HIVE-3004 > > > Diffs > ----- > > ql/src/test/queries/clientnegative/serde_regex.q 6603b91 > ql/src/test/queries/clientpositive/serde_regex.q c6809cb > ql/src/test/results/clientnegative/serde_regex.q.out 03fe907 > ql/src/test/results/clientpositive/serde_regex.q.out a8ce604 > serde/src/java/org/apache/hadoop/hive/serde2/RegexSerDe.java e728244 > > Diff: https://reviews.apache.org/r/8931/diff/ > > > Testing > ------- > > New test cases have been added and they pass. > > > Thanks, > > Shreepadma Venugopalan > >
