You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Lars Francke <as...@lars-francke.de> on 2012/06/25 07:29:51 UTC

Review Request: HBase Handler doesn't handle NULLs properly

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

Review request for hive.


Description
-------

This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 


This addresses bug HIVE-3179.
    https://issues.apache.org/jira/browse/HIVE-3179


Diffs
-----

  hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
  hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 

Diff: https://reviews.apache.org/r/5542/diff/


Testing
-------

Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests


Thanks,

Lars Francke


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Brock Noland <br...@cloudera.com>.

> On Feb. 8, 2013, 3:33 p.m., Mark Grover wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java, line 127
> > <https://reviews.apache.org/r/5542/diff/1/?file=116176#file116176line127>
> >
> >     Lars, I noticed that the Javadoc here is inconsistent with the method signature. This is obviously unrelated to your change but if you have a chance, could you please correct the Javadoc here?

Mark,

I agree we should update the javadoc, however I don't think that should hold up a commit since it's unrelated to this change and this issue is critical. That is if Lars doesn't update the patch we can address that issue in a new JIRA. Would you agree?  

Brock


- Brock


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


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Mark Grover <gr...@gmail.com>.

> On Feb. 8, 2013, 3:33 p.m., Mark Grover wrote:
> > hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java, line 127
> > <https://reviews.apache.org/r/5542/diff/1/?file=116176#file116176line127>
> >
> >     Lars, I noticed that the Javadoc here is inconsistent with the method signature. This is obviously unrelated to your change but if you have a chance, could you please correct the Javadoc here?
> 
> Brock Noland wrote:
>     Mark,
>     
>     I agree we should update the javadoc, however I don't think that should hold up a commit since it's unrelated to this change and this issue is critical. That is if Lars doesn't update the patch we can address that issue in a new JIRA. Would you agree?  
>     
>     Brock

Fair enough, that's fine by me.


- Mark


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


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Mark Grover <gr...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5542/#review16351
-----------------------------------------------------------



hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java
<https://reviews.apache.org/r/5542/#comment34816>

    Lars, I noticed that the Javadoc here is inconsistent with the method signature. This is obviously unrelated to your change but if you have a chance, could you please correct the Javadoc here?


- Mark Grover


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5542/#review16304
-----------------------------------------------------------

Ship it!


Ship It!

- Shreepadma Venugopalan


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Brock Noland <br...@cloudera.com>.

> On Feb. 7, 2013, 10:11 p.m., Brock Noland wrote:
> > Ship It!

I am not a committer.


- Brock


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


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5542/#review16302
-----------------------------------------------------------

Ship it!


Ship It!

- Brock Noland


On June 25, 2012, 5:30 a.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5542/
> -----------------------------------------------------------
> 
> (Updated June 25, 2012, 5:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Description
> -------
> 
> This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 
> 
> 
> This addresses bug HIVE-3179.
>     https://issues.apache.org/jira/browse/HIVE-3179
> 
> 
> Diffs
> -----
> 
>   hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
>   hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 
> 
> Diff: https://reviews.apache.org/r/5542/diff/
> 
> 
> Testing
> -------
> 
> Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests
> 
> 
> Thanks,
> 
> Lars Francke
> 
>


Re: Review Request: HIVE-3179: HBase Handler doesn't handle NULLs properly

Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5542/
-----------------------------------------------------------

(Updated June 25, 2012, 5:30 a.m.)


Review request for hive.


Summary (updated)
-----------------

HIVE-3179: HBase Handler doesn't handle NULLs properly


Description
-------

This patch moves the initialization of the fieldsInited variable to the end of the loop because otherwise NULL values will return stale data on the second iteration. All other cases should be unaffected by this change. 


This addresses bug HIVE-3179.
    https://issues.apache.org/jira/browse/HIVE-3179


Diffs
-----

  hbase-handler/src/java/org/apache/hadoop/hive/hbase/LazyHBaseRow.java d35bb52 
  hbase-handler/src/test/org/apache/hadoop/hive/hbase/TestLazyHBaseObject.java f91be4c 

Diff: https://reviews.apache.org/r/5542/diff/


Testing
-------

Debugged problem, added code to existing test to force wrong behavior which is fixed by this patch, ran the hbase-handler unit tests


Thanks,

Lars Francke