You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Shreepadma Venugopalan <sh...@cloudera.com> on 2013/01/12 01:27:44 UTC
Review Request: HIVE-3004: RegexSerDe should support other column types in
addition to STRING
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8931/
-----------------------------------------------------------
Review request for hive.
Description
-------
This patch enhances regex serde to parse column types other than STRING. Only primitive types are supported.
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
Re: Review Request: HIVE-3004: RegexSerDe should support other column types
in addition to STRING
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
> 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
>
>
Re: Review Request: HIVE-3004: RegexSerDe should support other column types
in addition to STRING
Posted by Mark Grover <gr...@gmail.com>.
> 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
>
>
Re: Review Request: HIVE-3004: RegexSerDe should support other column types
in addition to STRING
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8931/#review15305
-----------------------------------------------------------
serde/src/java/org/apache/hadoop/hive/serde2/RegexSerDe.java
<https://reviews.apache.org/r/8931/#comment32972>
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.
- Ashutosh Chauhan
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
>
>
Re: Review Request: HIVE-3004: RegexSerDe should support other column types
in addition to STRING
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-----
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
Re: Review Request: HIVE-3004: RegexSerDe should support other column types
in addition to STRING
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
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