[jira] [Comment Edited] (HIVE-16881) Make extractSqlBoolean More Consistent

["BELUGA BEHR (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16046938#comment-16046938 ] 

BELUGA BEHR edited comment on HIVE-16881 at 6/12/17 6:56 PM:
-------------------------------------------------------------

[~sershe] Thanks for the review.  My point was that the return value is pretty inconsistent.

'YE' Return Null

{code}
Character c = null;
// 'YE' is length of 2 so 'c' will not be set and return NULL
if (value instanceof String && ((String)value).length() == 1) {
  c = ((String)value).charAt(0);
}
if (c == null) return null;
{code}

{code}
Character c = null;
// 'Z' is length of 1 so 'c' will be set to 'Z'
if (value instanceof String && ((String)value).length() == 1) {
  c = ((String)value).charAt(0);
}

if (c == null) return null;
if (c == 'Y') return true;
if (c == 'N') return false;

// Exception is thrown
throw new MetaException("Cannot extract boolean from column value " + value);
{code}

Both value are invalid but result in different behavior.  Primarily 'YE' is ignored without alerting the user.

The dependency is on another Apache project that is already included as a dependency in the project. Code reuse +1


was (Author: belugabehr):
[~sershe] Thanks for the review.  My point was that the return value is pretty inconsistent.

'YE' Return Null

{code}
Character c = null;
// 'YE' is length of 2 so 'c' will not be set and return NULL
if (value instanceof String && ((String)value).length() == 1) {
  c = ((String)value).charAt(0);
}
if (c == null) return null;
{code}

{code}
Character c = null;
// 'Z' is length of 1 so 'c' will be set to 'Z'
if (value instanceof String && ((String)value).length() == 1) {
  c = ((String)value).charAt(0);
}

if (c == null) return null;
if (c == 'Y') return true;
if (c == 'N') return false;

// Exception is thrown
throw new MetaException("Cannot extract boolean from column value " + value);
{code}

The dependency is on another Apache project that is already included as a dependency in the project. Code reuse +1

> Make extractSqlBoolean More Consistent
> --------------------------------------
>
>                 Key: HIVE-16881
>                 URL: https://issues.apache.org/jira/browse/HIVE-16881
>             Project: Hive
>          Issue Type: Improvement
>          Components: Metastore
>    Affects Versions: 2.1.1, 3.0.0
>            Reporter: BELUGA BEHR
>            Priority: Trivial
>         Attachments: HIVE-16881.1.patch
>
>
> {code:title=org.apache.hadoop.hive.metastore.MetaStoreDirectSql|borderStyle=solid}
> private static Boolean extractSqlBoolean(Object value) throws MetaException {
>     // MySQL has booleans, but e.g. Derby uses 'Y'/'N' mapping. People using derby probably
>     // don't care about performance anyway, but let's cover the common case.
>     if (value == null) return null;
>     if (value instanceof Boolean) return (Boolean)value;
>     Character c = null;
>     if (value instanceof String && ((String)value).length() == 1) {
>       c = ((String)value).charAt(0);
>     }
>     if (c == null) return null;
>     if (c == 'Y') return true;
>     if (c == 'N') return false;
>     throw new MetaException("Cannot extract boolean from column value " + value);
>   }
> {code}
> It's not very clear here what the code intends.  For example, if the _value_ is "YE" then the code returns _null_.  If the _value_ is "Z" then an exception is thrown.
> # Change this to throw an exception when any invalid value is encountered
> # Add comments
> # Use Apache Commons Library for parsing



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)