You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/01/05 10:14:45 UTC
[GitHub] [hive] pvargacl opened a new pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
pvargacl opened a new pull request #1826:
URL: https://github.com/apache/hive/pull/1826
### What changes were proposed in this pull request?
- Remove unneccessary AcidUtils.getAcidState call from OrcInputformat when the table is not transactional
- Remove redundant filesystem utility functions from AcidUtils to HdfsUtils
### Why are the changes needed?
Make the code more readable
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Current unit tests.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kuczoram merged pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
kuczoram merged pull request #1826:
URL: https://github.com/apache/hive/pull/1826
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kuczoram commented on a change in pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
kuczoram commented on a change in pull request #1826:
URL: https://github.com/apache/hive/pull/1826#discussion_r553944336
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1866,92 +1870,6 @@ private static boolean isDirUsable(Path child, long visibilityTxnId, List<Path>
return true;
}
- public static class HdfsFileStatusWithoutId implements HdfsFileStatusWithId {
- private final FileStatus fs;
-
- public HdfsFileStatusWithoutId(FileStatus fs) {
- this.fs = fs;
- }
-
- @Override
- public FileStatus getFileStatus() {
- return fs;
- }
-
- @Override
- public Long getFileId() {
- return null;
- }
- }
-
- /**
- * Find the original files (non-ACID layout) recursively under the partition directory.
- * @param fs the file system
- * @param dir the directory to add
- * @return the list of original files
- * @throws IOException
- */
- public static List<HdfsFileStatusWithId> findOriginals(FileSystem fs, Path dir, Ref<Boolean> useFileIds,
- boolean ignoreEmptyFiles, boolean recursive) throws IOException {
- List<HdfsFileStatusWithId> originals = new ArrayList<>();
- List<HdfsFileStatusWithId> childrenWithId = tryListLocatedHdfsStatus(useFileIds, fs, dir, hiddenFileFilter);
- if (childrenWithId != null) {
- for (HdfsFileStatusWithId child : childrenWithId) {
- if (child.getFileStatus().isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getFileStatus().getPath(), useFileIds,
- ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getFileStatus().getLen() > 0) {
- originals.add(child);
- }
- }
- }
- } else {
- List<FileStatus> children = HdfsUtils.listLocatedStatus(fs, dir, hiddenFileFilter);
- for (FileStatus child : children) {
- if (child.isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getPath(), useFileIds, ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getLen() > 0) {
Review comment:
As far as I see, the ignoreEmptyFiles parameter is removed and it is not present in the newly added methods. Is this parameter not used any more? (Just want to make sure that removing it won't have any side effect.)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kuczoram commented on a change in pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
kuczoram commented on a change in pull request #1826:
URL: https://github.com/apache/hive/pull/1826#discussion_r554906173
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1866,92 +1870,6 @@ private static boolean isDirUsable(Path child, long visibilityTxnId, List<Path>
return true;
}
- public static class HdfsFileStatusWithoutId implements HdfsFileStatusWithId {
- private final FileStatus fs;
-
- public HdfsFileStatusWithoutId(FileStatus fs) {
- this.fs = fs;
- }
-
- @Override
- public FileStatus getFileStatus() {
- return fs;
- }
-
- @Override
- public Long getFileId() {
- return null;
- }
- }
-
- /**
- * Find the original files (non-ACID layout) recursively under the partition directory.
- * @param fs the file system
- * @param dir the directory to add
- * @return the list of original files
- * @throws IOException
- */
- public static List<HdfsFileStatusWithId> findOriginals(FileSystem fs, Path dir, Ref<Boolean> useFileIds,
- boolean ignoreEmptyFiles, boolean recursive) throws IOException {
- List<HdfsFileStatusWithId> originals = new ArrayList<>();
- List<HdfsFileStatusWithId> childrenWithId = tryListLocatedHdfsStatus(useFileIds, fs, dir, hiddenFileFilter);
- if (childrenWithId != null) {
- for (HdfsFileStatusWithId child : childrenWithId) {
- if (child.getFileStatus().isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getFileStatus().getPath(), useFileIds,
- ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getFileStatus().getLen() > 0) {
- originals.add(child);
- }
- }
- }
- } else {
- List<FileStatus> children = HdfsUtils.listLocatedStatus(fs, dir, hiddenFileFilter);
- for (FileStatus child : children) {
- if (child.isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getPath(), useFileIds, ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getLen() > 0) {
Review comment:
Oh, ok, I see. Thanks for the details.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1826:
URL: https://github.com/apache/hive/pull/1826#discussion_r553982610
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1866,92 +1870,6 @@ private static boolean isDirUsable(Path child, long visibilityTxnId, List<Path>
return true;
}
- public static class HdfsFileStatusWithoutId implements HdfsFileStatusWithId {
- private final FileStatus fs;
-
- public HdfsFileStatusWithoutId(FileStatus fs) {
- this.fs = fs;
- }
-
- @Override
- public FileStatus getFileStatus() {
- return fs;
- }
-
- @Override
- public Long getFileId() {
- return null;
- }
- }
-
- /**
- * Find the original files (non-ACID layout) recursively under the partition directory.
- * @param fs the file system
- * @param dir the directory to add
- * @return the list of original files
- * @throws IOException
- */
- public static List<HdfsFileStatusWithId> findOriginals(FileSystem fs, Path dir, Ref<Boolean> useFileIds,
- boolean ignoreEmptyFiles, boolean recursive) throws IOException {
- List<HdfsFileStatusWithId> originals = new ArrayList<>();
- List<HdfsFileStatusWithId> childrenWithId = tryListLocatedHdfsStatus(useFileIds, fs, dir, hiddenFileFilter);
- if (childrenWithId != null) {
- for (HdfsFileStatusWithId child : childrenWithId) {
- if (child.getFileStatus().isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getFileStatus().getPath(), useFileIds,
- ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getFileStatus().getLen() > 0) {
- originals.add(child);
- }
- }
- }
- } else {
- List<FileStatus> children = HdfsUtils.listLocatedStatus(fs, dir, hiddenFileFilter);
- for (FileStatus child : children) {
- if (child.isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getPath(), useFileIds, ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getLen() > 0) {
Review comment:
The findoriginals was called with ignoreEmpty = true always, so i removed it. If I understand correctly this parameter could be removed from every other acidutils call. It was there to handle MR, which created empty files for every bucket. https://issues.apache.org/jira/browse/HIVE-13040?focusedCommentId=15159223&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-15159223
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1826:
URL: https://github.com/apache/hive/pull/1826#discussion_r553982610
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1866,92 +1870,6 @@ private static boolean isDirUsable(Path child, long visibilityTxnId, List<Path>
return true;
}
- public static class HdfsFileStatusWithoutId implements HdfsFileStatusWithId {
- private final FileStatus fs;
-
- public HdfsFileStatusWithoutId(FileStatus fs) {
- this.fs = fs;
- }
-
- @Override
- public FileStatus getFileStatus() {
- return fs;
- }
-
- @Override
- public Long getFileId() {
- return null;
- }
- }
-
- /**
- * Find the original files (non-ACID layout) recursively under the partition directory.
- * @param fs the file system
- * @param dir the directory to add
- * @return the list of original files
- * @throws IOException
- */
- public static List<HdfsFileStatusWithId> findOriginals(FileSystem fs, Path dir, Ref<Boolean> useFileIds,
- boolean ignoreEmptyFiles, boolean recursive) throws IOException {
- List<HdfsFileStatusWithId> originals = new ArrayList<>();
- List<HdfsFileStatusWithId> childrenWithId = tryListLocatedHdfsStatus(useFileIds, fs, dir, hiddenFileFilter);
- if (childrenWithId != null) {
- for (HdfsFileStatusWithId child : childrenWithId) {
- if (child.getFileStatus().isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getFileStatus().getPath(), useFileIds,
- ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getFileStatus().getLen() > 0) {
- originals.add(child);
- }
- }
- }
- } else {
- List<FileStatus> children = HdfsUtils.listLocatedStatus(fs, dir, hiddenFileFilter);
- for (FileStatus child : children) {
- if (child.isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getPath(), useFileIds, ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getLen() > 0) {
Review comment:
The findoriginals was called with ignoreEmpty = true always, so i removed it. If I understand correctly this parameter could be removed from every other acidutils call. It was there to handle MR, which created empty files for every bucket. https://issues.apache.org/jira/browse/HIVE-13040?focusedCommentId=15159223&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-15159223
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kuczoram commented on a change in pull request #1826: HIVE-24581: Remove AcidUtils call from OrcInputformat for non transactional tables
Posted by GitBox <gi...@apache.org>.
kuczoram commented on a change in pull request #1826:
URL: https://github.com/apache/hive/pull/1826#discussion_r553944336
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -1866,92 +1870,6 @@ private static boolean isDirUsable(Path child, long visibilityTxnId, List<Path>
return true;
}
- public static class HdfsFileStatusWithoutId implements HdfsFileStatusWithId {
- private final FileStatus fs;
-
- public HdfsFileStatusWithoutId(FileStatus fs) {
- this.fs = fs;
- }
-
- @Override
- public FileStatus getFileStatus() {
- return fs;
- }
-
- @Override
- public Long getFileId() {
- return null;
- }
- }
-
- /**
- * Find the original files (non-ACID layout) recursively under the partition directory.
- * @param fs the file system
- * @param dir the directory to add
- * @return the list of original files
- * @throws IOException
- */
- public static List<HdfsFileStatusWithId> findOriginals(FileSystem fs, Path dir, Ref<Boolean> useFileIds,
- boolean ignoreEmptyFiles, boolean recursive) throws IOException {
- List<HdfsFileStatusWithId> originals = new ArrayList<>();
- List<HdfsFileStatusWithId> childrenWithId = tryListLocatedHdfsStatus(useFileIds, fs, dir, hiddenFileFilter);
- if (childrenWithId != null) {
- for (HdfsFileStatusWithId child : childrenWithId) {
- if (child.getFileStatus().isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getFileStatus().getPath(), useFileIds,
- ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getFileStatus().getLen() > 0) {
- originals.add(child);
- }
- }
- }
- } else {
- List<FileStatus> children = HdfsUtils.listLocatedStatus(fs, dir, hiddenFileFilter);
- for (FileStatus child : children) {
- if (child.isDirectory()) {
- if (recursive) {
- originals.addAll(findOriginals(fs, child.getPath(), useFileIds, ignoreEmptyFiles, true));
- }
- } else {
- if (!ignoreEmptyFiles || child.getLen() > 0) {
Review comment:
As far as I see, the ignoreEmptyFiles parameter is removed and it is not present in the newly added methods. Is this parameter not used any more? (Just want to make sure that removing it won't have any side effect.)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org