[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16238567#comment-16238567 ] Ashutosh Chauhan commented on HIVE-16881: - +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 >Assignee: BELUGA BEHR > Attachments: HIVE-16881.1.patch, HIVE-16881.2.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)
[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16062210#comment-16062210 ] BELUGA BEHR commented on HIVE-16881: Future-proofs the code if something changes later. The consistent results will be helpful. > 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 >Assignee: BELUGA BEHR >Priority: Trivial > Attachments: HIVE-16881.1.patch, HIVE-16881.2.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)
[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16048199#comment-16048199 ] Hive QA commented on HIVE-16881: Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12872881/HIVE-16881.2.patch {color:red}ERROR:{color} -1 due to no test(s) being added or modified. {color:red}ERROR:{color} -1 due to 8 failed/errored test(s), 10830 tests executed *Failed tests:* {noformat} org.apache.hadoop.hive.cli.TestMiniLlapCliDriver.testCliDriver[orc_ppd_basic] (batchId=140) org.apache.hadoop.hive.cli.TestMiniLlapLocalCliDriver.testCliDriver[columnstats_part_coltype] (batchId=157) org.apache.hadoop.hive.cli.TestMiniTezCliDriver.testCliDriver[explainanalyze_2] (batchId=99) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query16] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query94] (batchId=232) org.apache.hadoop.hive.metastore.TestMetaStoreAuthorization.testMetaStoreAuthorization (batchId=205) {noformat} Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/5637/testReport Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/5637/console Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-5637/ Messages: {noformat} Executing org.apache.hive.ptest.execution.TestCheckPhase Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 8 tests failed {noformat} This message is automatically generated. ATTACHMENT ID: 12872881 - PreCommit-HIVE-Build > 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, HIVE-16881.2.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)
[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16046938#comment-16046938 ] BELUGA BEHR commented on HIVE-16881: [~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)
[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16046924#comment-16046924 ] Sergey Shelukhin commented on HIVE-16881: - It won't return true for 'YE' since it only handles single character strings. This looks like an unnecessary change to me that needlessly adds a dependency for a very simple piece of code. > 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)
[jira] [Commented] (HIVE-16881) Make extractSqlBoolean More Consistent
[ https://issues.apache.org/jira/browse/HIVE-16881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16045713#comment-16045713 ] Hive QA commented on HIVE-16881: Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12872453/HIVE-16881.1.patch {color:red}ERROR:{color} -1 due to no test(s) being added or modified. {color:red}ERROR:{color} -1 due to 8 failed/errored test(s), 10829 tests executed *Failed tests:* {noformat} org.apache.hadoop.hive.cli.TestMiniLlapCliDriver.testCliDriver[orc_ppd_basic] (batchId=140) org.apache.hadoop.hive.cli.TestMiniLlapLocalCliDriver.testCliDriver[vector_if_expr] (batchId=145) org.apache.hadoop.hive.cli.TestMiniTezCliDriver.testCliDriver[explainanalyze_2] (batchId=99) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query14] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query16] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query23] (batchId=232) org.apache.hadoop.hive.cli.TestPerfCliDriver.testCliDriver[query94] (batchId=232) org.apache.hive.jdbc.TestJdbcWithMiniHS2.testHttpRetryOnServerIdleTimeout (batchId=226) {noformat} Test results: https://builds.apache.org/job/PreCommit-HIVE-Build/5621/testReport Console output: https://builds.apache.org/job/PreCommit-HIVE-Build/5621/console Test logs: http://104.198.109.242/logs/PreCommit-HIVE-Build-5621/ Messages: {noformat} Executing org.apache.hive.ptest.execution.TestCheckPhase Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 8 tests failed {noformat} This message is automatically generated. ATTACHMENT ID: 12872453 - PreCommit-HIVE-Build > 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 > # User Apache Commons Library for parsing -- This message was sent by Atlassian JIRA (v6.3.15#6346)