[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14185335#comment-14185335
]
Alan Gates commented on HIVE-8583:
--
I'm not opposed to changing the order of the modifiers, I just didn't
understand why it mattered. So no need for a new patch.
We do need the tests to run on this patch though. I don't think the build
failure has anything with your patch. So just canceling the patch,
re-attaching the file, and re-submitting the patch should force the tests to
run.
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14183332#comment-14183332
]
Lars Francke commented on HIVE-8583:
Thanks for taking a look.
The "missorted modifiers" term is coming from my IDE but it is referenced in
the [Java Language
Spec|http://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.3.1]
which lists this order: {{public protected private static final transient
volatile}}. The [Hive Contributors
Guide|https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConventions]
mentions following the Sun standards so I've set up my IDE to notify me of
these things.
I also think it's great to have consistency in these small details because it
makes foreign code easier to grok. Happy to provide a patch without the change
if you like.
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14183171#comment-14183171
]
Alan Gates commented on HIVE-8583:
--
+1, patch looks fine.
The statement "missorted modifiers" implies there is a correct order. If the
compiler doesn't care about "final static private" versus "private static
final" why should we?
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14183002#comment-14183002
]
Alan Gates commented on HIVE-8583:
--
Yes, Lars is correct. That is just a piece of earlier code that I neglected to
take out.
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14182481#comment-14182481
]
Lars Francke commented on HIVE-8583:
As far as I understand the work is done on the non-replaced original
configuration properties:
{code}
void addJobConfToEnvironment(Configuration conf, Map env) {
Iterator> it = conf.iterator();
while (it.hasNext()) {
Map.Entry en = it.next();
String name = en.getKey();
if (!blackListed(name)) {
String value = conf.get(name); // does variable expansion
name = safeEnvVarName(name);
{code}
So the replacing happens later.
BTW. the replaceAll is wrong too. It takes a regex so "." means every
character. So it'd replace everything with underscores.
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14182407#comment-14182407
]
Gunther Hagleitner commented on HIVE-8583:
--
I think that's not only confusing, but it's a bug. The replacement has to
happen otherwise the comparison will fail (and the entries won't be
blacklisted). [~alangates] what do you think?
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14182341#comment-14182341
]
Hive QA commented on HIVE-8583:
---
{color:red}Overall{color}: -1 no tests executed
Here are the results of testing the latest attachment:
https://issues.apache.org/jira/secure/attachment/12676749/HIVE-8583.1.patch
Test results:
http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/1428/testReport
Console output:
http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/1428/console
Test logs:
http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-1428/
Messages:
{noformat}
This message was trimmed, see log for full details
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:68:4:
Decision can match input such as "LPAREN KW_CASE KW_IF" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:68:4:
Decision can match input such as "LPAREN LPAREN KW_IF" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:115:5:
Decision can match input such as "KW_CLUSTER KW_BY LPAREN" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:127:5:
Decision can match input such as "KW_PARTITION KW_BY LPAREN" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:138:5:
Decision can match input such as "KW_DISTRIBUTE KW_BY LPAREN" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:149:5:
Decision can match input such as "KW_SORT KW_BY LPAREN" using multiple
alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:166:7:
Decision can match input such as "STAR" using multiple alternatives: 1, 2
As a result, alternative(s) 2 were disabled for that input
warning(200): IdentifiersParser.g:179:5:
Decision can match input such as "KW_STRUCT" using multiple alternatives: 4, 6
As a result, alternative(s) 6 were disabled for that input
warning(200): IdentifiersParser.g:179:5:
Decision can match input such as "KW_ARRAY" using multiple alternatives: 2, 6
As a result, alternative(s) 6 were disabled for that input
warning(200): IdentifiersParser.g:179:5:
Decision can match input such as "KW_UNIONTYPE" using multiple alternatives: 5,
6
As a result, alternative(s) 6 were disabled for that input
warning(200): IdentifiersParser.g:261:5:
Decision can match input such as "KW_NULL" using multiple alternatives: 1, 8
As a result, alternative(s) 8 were disabled for that input
warning(200): IdentifiersParser.g:261:5:
Decision can match input such as "KW_FALSE" using multiple alternatives: 3, 8
As a result, alternative(s) 8 were disabled for that input
warning(200): IdentifiersParser.g:261:5:
Decision can match input such as "KW_TRUE" using multiple alternatives: 3, 8
As a result, alternative(s) 8 were disabled for that input
warning(200): IdentifiersParser.g:261:5:
Decision can match input such as "KW_DATE StringLiteral" using multiple
alternatives: 2, 3
As a result, alternative(s) 3 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_CLUSTER
KW_BY" using multiple alternatives: 2, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_UNION
KW_ALL" using multiple alternatives: 2, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_SORT KW_BY"
using multiple alternatives: 2, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_INSERT
KW_INTO" using multiple alternatives: 2, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_LATERAL
KW_VIEW" using multiple alternatives: 2, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "KW_BETWEEN KW_MAP LPAREN" using multiple
alternatives: 8, 9
As a result, alternative(s) 9 were disabled for that input
warning(200): IdentifiersParser.g:393:5:
Decision can match input such as "{KW_LIKE, KW_REGEXP, KW_RLIKE} KW_DISTRIBUTE
KW_BY" using multiple alternatives: 2, 9
As a result,
[jira] [Commented] (HIVE-8583) HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
[
https://issues.apache.org/jira/browse/HIVE-8583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14182050#comment-14182050
]
Lars Francke commented on HIVE-8583:
I'm not sure if the default value reading stuff is needed or if it'll somehow
be propagated to the Configuration object in some other way
> HIVE-8341 Cleanup & Test for hive.script.operator.env.blacklist
> ---
>
> Key: HIVE-8583
> URL: https://issues.apache.org/jira/browse/HIVE-8583
> Project: Hive
> Issue Type: Improvement
>Reporter: Lars Francke
>Assignee: Lars Francke
>Priority: Minor
> Attachments: HIVE-8583.1.patch
>
>
> [~alangates] added the following in HIVE-8341:
> {code}
> String bl =
> hconf.get(HiveConf.ConfVars.HIVESCRIPT_ENV_BLACKLIST.toString());
> if (bl != null && bl.length() > 0) {
> String[] bls = bl.split(",");
> for (String b : bls) {
> b.replaceAll(".", "_");
> blackListedConfEntries.add(b);
> }
> }
> {code}
> The {{replaceAll}} call is confusing as its result is not used at all.
> This patch contains the following:
> * Minor style modification (missorted modifiers)
> * Adds reading of default value for HIVESCRIPT_ENV_BLACKLIST
> * Removes replaceAll
> * Lets blackListed take a Configuration job as parameter which allowed me to
> add a test for this
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
