[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[ https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=457160=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-457160 ] ASF GitHub Bot logged work on HIVE-23638: - Author: ASF GitHub Bot Created on: 10/Jul/20 13:13 Start Date: 10/Jul/20 13:13 Worklog Time Spent: 10m Work Description: kgyrtkirk merged pull request #1161: URL: https://github.com/apache/hive/pull/1161 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 457160) Time Spent: 2h 40m (was: 2.5h) > Fix FindBug issues in hive-common > - > > Key: HIVE-23638 > URL: https://issues.apache.org/jira/browse/HIVE-23638 > Project: Hive > Issue Type: Sub-task >Reporter: Panagiotis Garefalakis >Assignee: Panagiotis Garefalakis >Priority: Major > Labels: pull-request-available > Attachments: spotbugsXml.xml > > Time Spent: 2h 40m > Remaining Estimate: 0h > > mvn -Pspotbugs > -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO > -pl :hive-common test-compile > com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456694=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456694
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 09/Jul/20 15:43
Start Date: 09/Jul/20 15:43
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452200559
##
File path: common/src/java/org/apache/hadoop/hive/common/jsonexplain/Vertex.java
##
@@ -308,6 +309,16 @@ public void setType(String type) {
this.edgeType = this.parser.mapEdgeType(type);
}
+ @Override
+ public boolean equals(Object o) {
+return super.equals(o);
Review comment:
You are right comparison could be implemented as part of the class
(added that part).
Regarding the parser I agree it should not be a field, only given as
parameter where needed -- there are more places where we follow this weird
logic and should probably be addressed all together:
https://github.com/apache/hive/blob/ba0217ff17501fb849d8999e808d37579db7b4f1/common/src/java/org/apache/hadoop/hive/common/jsonexplain/Stage.java#L52
and
https://github.com/apache/hive/blob/99f21bb42d7c1b258f5c34b86e71777978e31919/common/src/java/org/apache/hadoop/hive/common/jsonexplain/Op.java#L58
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456694)
Time Spent: 2.5h (was: 2h 20m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 2.5h
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456600=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456600
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 09/Jul/20 13:05
Start Date: 09/Jul/20 13:05
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452200559
##
File path: common/src/java/org/apache/hadoop/hive/common/jsonexplain/Vertex.java
##
@@ -308,6 +309,16 @@ public void setType(String type) {
this.edgeType = this.parser.mapEdgeType(type);
}
+ @Override
+ public boolean equals(Object o) {
+return super.equals(o);
Review comment:
You are right comparison could be implemented as part of the class
(added that part).
Regarding the parser I agree it should not be a field, only given as
parameter where needed -- there are more places where we follow this weird
implementation and should probably be addressed all together:
https://github.com/apache/hive/blob/ba0217ff17501fb849d8999e808d37579db7b4f1/common/src/java/org/apache/hadoop/hive/common/jsonexplain/Stage.java#L52
and
https://github.com/apache/hive/blob/99f21bb42d7c1b258f5c34b86e71777978e31919/common/src/java/org/apache/hadoop/hive/common/jsonexplain/Op.java#L58
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456600)
Time Spent: 2h 20m (was: 2h 10m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456597=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456597
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 09/Jul/20 13:03
Start Date: 09/Jul/20 13:03
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452200559
##
File path: common/src/java/org/apache/hadoop/hive/common/jsonexplain/Vertex.java
##
@@ -308,6 +309,16 @@ public void setType(String type) {
this.edgeType = this.parser.mapEdgeType(type);
}
+ @Override
+ public boolean equals(Object o) {
+return super.equals(o);
Review comment:
You are right comparison could be implemented as part of the class
(added that part).
Regarding the parser I agree it should not be a field, only given as
parameter where needed -- there are more places where we follow this weird
implementation and should probably be addressed all together:
https://github.com/apache/hive/blob/ba0217ff17501fb849d8999e808d37579db7b4f1/common/src/java/org/apache/hadoop/hive/common/jsonexplain/Stage.java#L52
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456597)
Time Spent: 2h 10m (was: 2h)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456588=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456588
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 09/Jul/20 12:51
Start Date: 09/Jul/20 12:51
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452193076
##
File path:
common/src/java/org/apache/hadoop/hive/common/jsonexplain/DagJsonParserUtils.java
##
@@ -26,9 +28,13 @@
public class DagJsonParserUtils {
- public static List OperatorNoStats = Arrays.asList(new String[] {
"File Output Operator",
+ private static final List operatorNoStats = Arrays.asList(new
String[] { "File Output Operator",
Review comment:
Well, it kinda needs to be static as Op class is using it directly
within getNameWithOpIdStats method
##
File path: common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
##
@@ -133,6 +134,7 @@ public static void setPerfLogger(PerfLogger
resetPerfLogger) {
* @param callerName the logging object to be used.
* @param method method or ID that identifies this perf log element.
*/
+ @SuppressFBWarnings(value = "NM_METHOD_NAMING_CONVENTION", justification =
"Intended")
public void PerfLogBegin(String callerName, String method) {
Review comment:
Sure, opened HIVE-23823 for tracking this
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456588)
Time Spent: 2h (was: 1h 50m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456521=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456521
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 09/Jul/20 10:10
Start Date: 09/Jul/20 10:10
Worklog Time Spent: 10m
Work Description: kgyrtkirk commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r452110531
##
File path: common/src/java/org/apache/hadoop/hive/common/jsonexplain/Vertex.java
##
@@ -308,6 +309,16 @@ public void setType(String type) {
this.edgeType = this.parser.mapEdgeType(type);
}
+ @Override
+ public boolean equals(Object o) {
+return super.equals(o);
Review comment:
interesting choices were made when this class was created...
the most important fields seems to be `String name, JSONObject vertexObject,
Stage stage`
passing the `parser` in the constructor is an interesting idea...
although this `equals` resorts to identity comparision - I don't see that so
out of scope for this class...
##
File path:
common/src/java/org/apache/hadoop/hive/common/jsonexplain/DagJsonParserUtils.java
##
@@ -26,9 +28,13 @@
public class DagJsonParserUtils {
- public static List OperatorNoStats = Arrays.asList(new String[] {
"File Output Operator",
+ private static final List operatorNoStats = Arrays.asList(new
String[] { "File Output Operator",
Review comment:
the issue here was:
* missing final
* naming convention
I don't think we need that static method...but no big deal
##
File path: common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
##
@@ -133,6 +134,7 @@ public static void setPerfLogger(PerfLogger
resetPerfLogger) {
* @param callerName the logging object to be used.
* @param method method or ID that identifies this perf log element.
*/
+ @SuppressFBWarnings(value = "NM_METHOD_NAMING_CONVENTION", justification =
"Intended")
public void PerfLogBegin(String callerName, String method) {
Review comment:
it's not just "spotbugs" who don't like these method names :D
it certainly out of scope of this patch - but I think they should be
improved - could you open a jira for it? :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456521)
Time Spent: 1h 50m (was: 1h 40m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456064=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456064
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 08/Jul/20 12:04
Start Date: 08/Jul/20 12:04
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r451489380
##
File path: common/src/java/org/apache/hive/common/util/SuppressFBWarnings.java
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hive.common.util;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.CLASS)
+public @interface SuppressFBWarnings {
+/**
+ * The set of FindBugs warnings that are to be suppressed in
+ * annotated element. The value can be a bug category, kind or pattern.
+ *
+ */
+String[] value() default {};
+
+/**
+ * Optional documentation of the reason why the warning is suppressed
+ */
+String justification() default "";
+}
Review comment:
Sure, done :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456064)
Time Spent: 1h 40m (was: 1.5h)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456062=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456062
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 08/Jul/20 12:03
Start Date: 08/Jul/20 12:03
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r451488767
##
File path: common/src/java/org/apache/hadoop/hive/common/StringInternUtils.java
##
@@ -135,10 +135,10 @@ public static Path internUriStringsInPath(Path path) {
public static Map internValuesInMap(Map map) {
if (map != null) {
- for (K key : map.keySet()) {
-String value = map.get(key);
+ for (Map.Entry entry : map.entrySet()) {
+String value = entry.getValue();
if (value != null) {
- map.put(key, value.intern());
+ map.put(entry.getKey(), value.intern());
Review comment:
Nice idea! I followed similar logic to check if values are already
interned in all helper methods in StringInternUtils class
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456062)
Time Spent: 1h 20m (was: 1h 10m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456063=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456063
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 08/Jul/20 12:03
Start Date: 08/Jul/20 12:03
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r451489258
##
File path: common/src/java/org/apache/hadoop/hive/conf/Validator.java
##
@@ -357,14 +357,15 @@ public String validate(String value) {
final Path path = FileSystems.getDefault().getPath(value);
if (path == null && value != null) {
return String.format("Path '%s' provided could not be located.",
value);
- }
- final boolean isDir = Files.isDirectory(path);
- final boolean isWritable = Files.isWritable(path);
- if (!isDir) {
-return String.format("Path '%s' provided is not a directory.", value);
- }
- if (!isWritable) {
-return String.format("Path '%s' provided is not writable.", value);
+ } else if (path != null) {
+final boolean isDir = Files.isDirectory(path);
+final boolean isWritable = Files.isWritable(path);
+if (!isDir) {
+ return String.format("Path '%s' provided is not a directory.",
value);
+}
+if (!isWritable) {
+ return String.format("Path '%s' provided is not writable.", value);
+}
}
return null;
Review comment:
Refactored the code to return early when the argument is actually null,
the following logic is now simplified to Null and non null path
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456063)
Time Spent: 1.5h (was: 1h 20m)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=456061=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-456061
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 08/Jul/20 12:00
Start Date: 08/Jul/20 12:00
Worklog Time Spent: 10m
Work Description: pgaref commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r451487709
##
File path: common/src/java/org/apache/hadoop/hive/common/FileUtils.java
##
@@ -926,8 +925,7 @@ public static File createLocalDirsTempFile(Configuration
conf, String prefix, St
* delete a temporary file and remove it from delete-on-exit hook.
*/
public static boolean deleteTmpFile(File tempFile) {
-if (tempFile != null) {
- tempFile.delete();
+if (tempFile != null && tempFile.delete()) {
Review comment:
Good catch, fixed
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 456061)
Time Spent: 1h 10m (was: 1h)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=454902=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-454902
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 06/Jul/20 15:26
Start Date: 06/Jul/20 15:26
Worklog Time Spent: 10m
Work Description: kgyrtkirk commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r450299299
##
File path: common/src/java/org/apache/hadoop/hive/conf/Validator.java
##
@@ -357,14 +357,15 @@ public String validate(String value) {
final Path path = FileSystems.getDefault().getPath(value);
if (path == null && value != null) {
return String.format("Path '%s' provided could not be located.",
value);
- }
- final boolean isDir = Files.isDirectory(path);
- final boolean isWritable = Files.isWritable(path);
- if (!isDir) {
-return String.format("Path '%s' provided is not a directory.", value);
- }
- if (!isWritable) {
-return String.format("Path '%s' provided is not writable.", value);
+ } else if (path != null) {
+final boolean isDir = Files.isDirectory(path);
+final boolean isWritable = Files.isWritable(path);
+if (!isDir) {
+ return String.format("Path '%s' provided is not a directory.",
value);
+}
+if (!isWritable) {
+ return String.format("Path '%s' provided is not writable.", value);
+}
}
return null;
Review comment:
I think this should be something else than `null` - did the old
behaviour accept `null` as a valid path? I believe `Files.isDirectory(null)`
returned false
looking from the other end: I don't think `null` should be accepted as a
"writeabledirectory"
##
File path: common/src/java/org/apache/hadoop/hive/common/FileUtils.java
##
@@ -926,8 +925,7 @@ public static File createLocalDirsTempFile(Configuration
conf, String prefix, St
* delete a temporary file and remove it from delete-on-exit hook.
*/
public static boolean deleteTmpFile(File tempFile) {
-if (tempFile != null) {
- tempFile.delete();
+if (tempFile != null && tempFile.delete()) {
Review comment:
this change will leave the file registered in the shutdownmanager in
case the file was already deleted
##
File path: common/src/java/org/apache/hadoop/hive/common/type/HiveVarchar.java
##
@@ -62,6 +62,9 @@ public boolean equals(Object rhs) {
if (rhs == this) {
return true;
}
-return this.getValue().equals(((HiveVarchar)rhs).getValue());
+if (rhs instanceof HiveVarchar) {
+ return this.getValue().equals(((HiveVarchar) rhs).getValue());
Review comment:
nice catch! :D
##
File path: common/src/java/org/apache/hive/common/util/SuppressFBWarnings.java
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hive.common.util;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.CLASS)
+public @interface SuppressFBWarnings {
+/**
+ * The set of FindBugs warnings that are to be suppressed in
+ * annotated element. The value can be a bug category, kind or pattern.
+ *
+ */
+String[] value() default {};
+
+/**
+ * Optional documentation of the reason why the warning is suppressed
+ */
+String justification() default "";
+}
Review comment:
I think at some point we should probably introduce some rule to ensure
that every file ends with a newline...but for now: could you add one here ? :D
##
File path: common/src/java/org/apache/hadoop/hive/common/StringInternUtils.java
##
@@ -135,10 +135,10 @@ public static Path internUriStringsInPath(Path path) {
public static Map internValuesInMap(Map map) {
if (map != null) {
- for (K key : map.keySet()) {
-String value = map.get(key);
+ for (Map.Entry entry : map.entrySet()) {
+String value =
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[ https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=453395=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-453395 ] ASF GitHub Bot logged work on HIVE-23638: - Author: ASF GitHub Bot Created on: 01/Jul/20 12:40 Start Date: 01/Jul/20 12:40 Worklog Time Spent: 10m Work Description: pgaref commented on pull request #1161: URL: https://github.com/apache/hive/pull/1161#issuecomment-652394311 @belugabehr thanks for the review! Addressed your comments in the latest commit -- could you please take another look? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 453395) Time Spent: 50m (was: 40m) > Fix FindBug issues in hive-common > - > > Key: HIVE-23638 > URL: https://issues.apache.org/jira/browse/HIVE-23638 > Project: Hive > Issue Type: Sub-task >Reporter: Panagiotis Garefalakis >Assignee: Panagiotis Garefalakis >Priority: Major > Labels: pull-request-available > Attachments: spotbugsXml.xml > > Time Spent: 50m > Remaining Estimate: 0h > > mvn -Pspotbugs > -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO > -pl :hive-common test-compile > com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[
https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=452324=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-452324
]
ASF GitHub Bot logged work on HIVE-23638:
-
Author: ASF GitHub Bot
Created on: 29/Jun/20 13:52
Start Date: 29/Jun/20 13:52
Worklog Time Spent: 10m
Work Description: belugabehr commented on a change in pull request #1161:
URL: https://github.com/apache/hive/pull/1161#discussion_r446983489
##
File path: common/src/java/org/apache/hadoop/hive/common/FileUtils.java
##
@@ -483,12 +483,6 @@ public static boolean
isActionPermittedForFileHierarchy(FileSystem fs, FileStatu
String userName, FsAction action, boolean recurse) throws Exception {
boolean isDir = fileStatus.isDir();
-FsAction dirActionNeeded = action;
-if (isDir) {
- // for dirs user needs execute privileges as well
- dirActionNeeded.and(FsAction.EXECUTE);
-}
-
Review comment:
So, I understand why this would be removed from a find-bugs perspective
(this is a no-op), but this is actually an all around bug. This should be:
```
// for dirs user needs execute privileges as well
FsAction dirActionNeeded = (isDir) ? action.and(FsAction.EXECUTE) : action;
```
##
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##
@@ -6475,17 +6477,17 @@ private static boolean isAllowed(Configuration conf,
ConfVars setting) {
}
public static String getNonMrEngines() {
-String result = StringUtils.EMPTY;
+StringBuffer result = new StringBuffer();
for (String s : ConfVars.HIVE_EXECUTION_ENGINE.getValidStringValues()) {
if ("mr".equals(s)) {
continue;
}
- if (!result.isEmpty()) {
-result += ", ";
+ if (result.length() != 0) {
+result.append(", ");
}
- result += s;
+ result.append(s);
}
-return result;
+return result.toString();
Review comment:
Please change this to just use String#join and more human friendly.
```
Set engines = new
HashSet<>(ConfVars.HIVE_EXECUTION_ENGINE.getValidStringValues());
boolean removedMR = engines.remove("mr");
LOG.debug("Found and removed MapReduce engine from list of valid execution
engines: {}", removedMR);
return String.join(", ", engines);
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
---
Worklog Id: (was: 452324)
Time Spent: 40m (was: 0.5h)
> Fix FindBug issues in hive-common
> -
>
> Key: HIVE-23638
> URL: https://issues.apache.org/jira/browse/HIVE-23638
> Project: Hive
> Issue Type: Sub-task
>Reporter: Panagiotis Garefalakis
>Assignee: Panagiotis Garefalakis
>Priority: Major
> Labels: pull-request-available
> Attachments: spotbugsXml.xml
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> mvn -Pspotbugs
> -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO
> -pl :hive-common test-compile
> com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[ https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=450874=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-450874 ] ASF GitHub Bot logged work on HIVE-23638: - Author: ASF GitHub Bot Created on: 25/Jun/20 08:56 Start Date: 25/Jun/20 08:56 Worklog Time Spent: 10m Work Description: pgaref opened a new pull request #1161: URL: https://github.com/apache/hive/pull/1161 ## NOTICE Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull request which starts with the corresponding JIRA issue number. (e.g. HIVE-X: Fix a typo in YYY) For more details, please see https://cwiki.apache.org/confluence/display/Hive/HowToContribute This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 450874) Time Spent: 0.5h (was: 20m) > Fix FindBug issues in hive-common > - > > Key: HIVE-23638 > URL: https://issues.apache.org/jira/browse/HIVE-23638 > Project: Hive > Issue Type: Sub-task >Reporter: Panagiotis Garefalakis >Assignee: Panagiotis Garefalakis >Priority: Major > Labels: pull-request-available > Attachments: spotbugsXml.xml > > Time Spent: 0.5h > Remaining Estimate: 0h > > mvn -Pspotbugs > -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO > -pl :hive-common test-compile > com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[ https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=450872=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-450872 ] ASF GitHub Bot logged work on HIVE-23638: - Author: ASF GitHub Bot Created on: 25/Jun/20 08:56 Start Date: 25/Jun/20 08:56 Worklog Time Spent: 10m Work Description: pgaref closed pull request #1161: URL: https://github.com/apache/hive/pull/1161 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 450872) Time Spent: 20m (was: 10m) > Fix FindBug issues in hive-common > - > > Key: HIVE-23638 > URL: https://issues.apache.org/jira/browse/HIVE-23638 > Project: Hive > Issue Type: Sub-task >Reporter: Panagiotis Garefalakis >Assignee: Panagiotis Garefalakis >Priority: Major > Labels: pull-request-available > Attachments: spotbugsXml.xml > > Time Spent: 20m > Remaining Estimate: 0h > > mvn -Pspotbugs > -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO > -pl :hive-common test-compile > com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Work logged] (HIVE-23638) Fix FindBug issues in hive-common
[ https://issues.apache.org/jira/browse/HIVE-23638?focusedWorklogId=449336=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-449336 ] ASF GitHub Bot logged work on HIVE-23638: - Author: ASF GitHub Bot Created on: 22/Jun/20 15:22 Start Date: 22/Jun/20 15:22 Worklog Time Spent: 10m Work Description: pgaref opened a new pull request #1161: URL: https://github.com/apache/hive/pull/1161 ## NOTICE Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull request which starts with the corresponding JIRA issue number. (e.g. HIVE-X: Fix a typo in YYY) For more details, please see https://cwiki.apache.org/confluence/display/Hive/HowToContribute This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 449336) Remaining Estimate: 0h Time Spent: 10m > Fix FindBug issues in hive-common > - > > Key: HIVE-23638 > URL: https://issues.apache.org/jira/browse/HIVE-23638 > Project: Hive > Issue Type: Sub-task >Reporter: Panagiotis Garefalakis >Assignee: Panagiotis Garefalakis >Priority: Major > Attachments: spotbugsXml.xml > > Time Spent: 10m > Remaining Estimate: 0h > > mvn -Pspotbugs > -Dorg.slf4j.simpleLogger.log.org.apache.maven.plugin.surefire.SurefirePlugin=INFO > -pl :hive-common test-compile > com.github.spotbugs:spotbugs-maven-plugin:4.0.0:check -- This message was sent by Atlassian Jira (v8.3.4#803005)
