hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shreepadma Venugopalan" <shreepa...@cloudera.com>
Subject Re: Review Request: HIVE-3004: RegexSerDe should support other column types in addition to STRING
Date Mon, 14 Jan 2013 20:45:38 GMT


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

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.


- Shreepadma


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8931/#review15305
-----------------------------------------------------------


On Jan. 12, 2013, 12:28 a.m., Shreepadma Venugopalan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8931/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2013, 12:28 a.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message