This is an automated email from the ASF dual-hosted git repository.
jlli pushed a commit to branch fix-move-files-when-it-is-a-new-table
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to
refs/heads/fix-move-files-when-it-is-a-new-table by this push:
new 33b7320 Address comments on API doc
33b7320 is described below
commit 33b732020bdd60be310b54ba0f1a0a9ae1d9a144
Author: jackjlli <[email protected]>
AuthorDate: Mon Feb 25 10:37:05 2019 -0800
Address comments on API doc
---
.../org/apache/pinot/filesystem/AzurePinotFS.java | 4 ---
.../org/apache/pinot/filesystem/LocalPinotFS.java | 2 --
.../java/org/apache/pinot/filesystem/PinotFS.java | 41 ++++++++++------------
3 files changed, 18 insertions(+), 29 deletions(-)
diff --git
a/pinot-azure-filesystem/src/main/java/org/apache/pinot/filesystem/AzurePinotFS.java
b/pinot-azure-filesystem/src/main/java/org/apache/pinot/filesystem/AzurePinotFS.java
index e15ace5..3531f74 100644
---
a/pinot-azure-filesystem/src/main/java/org/apache/pinot/filesystem/AzurePinotFS.java
+++
b/pinot-azure-filesystem/src/main/java/org/apache/pinot/filesystem/AzurePinotFS.java
@@ -105,10 +105,6 @@ public class AzurePinotFS extends PinotFS {
*/
public boolean doMove(URI srcUri, URI dstUri)
throws IOException {
- // ensures the parent path of dst exists.
- URI parentUri = Paths.get(dstUri).toUri();
- _adlStoreClient.createDirectory(parentUri.getPath());
- // renames the source file/directory to destination.
return _adlStoreClient.rename(srcUri.getPath(), dstUri.getPath());
}
diff --git
a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/LocalPinotFS.java
b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/LocalPinotFS.java
index fcbf532..b42c794 100644
---
a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/LocalPinotFS.java
+++
b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/LocalPinotFS.java
@@ -79,8 +79,6 @@ public class LocalPinotFS extends PinotFS {
throws IOException {
File srcFile = new File(decodeURI(srcUri.getRawPath()));
File dstFile = new File(decodeURI(dstUri.getRawPath()));
- // ensures the parent path of dst exists.
- FileUtils.forceMkdir(dstFile.getParentFile());
if (srcFile.isDirectory()) {
FileUtils.moveDirectoryToDirectory(srcFile, dstFile, true);
} else {
diff --git
a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
index 36a374d..a497b66 100644
--- a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
+++ b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
@@ -22,6 +22,7 @@ import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.net.URI;
+import java.nio.file.Paths;
import org.apache.commons.configuration.Configuration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -62,13 +63,11 @@ public abstract class PinotFS implements Closeable {
/**
* Moves the file or directory from the src to dst. Does not keep the
original file. If the dst has parent directories
* that haven't been created, this method will create all the necessary
parent directories.
- * If both src and dst are files, dst will be overwritten.
- * If src is a file and dst is an empty directory, src file will get moved
under dst directory.
- * If both src and dst are directories, src directory will get moved under
dst directory.
- * If src is a directory and dst is a file, operation will fail.
+ * Note: In Pinot we recommend the full paths of both src and dst be
specified.
* For example, if a file /a/b/c is moved to a file /x/y/z, in the case of
overwrite, the directory /a/b still exists,
* but will not contain the file 'c'. Instead, /x/y/z will contain the
contents of 'c'.
- * If a file /a is moved to a directory /x/y, all the original files under
/x/y will be kept.
+ * If a directory /a/b/ is renamed to another directory /x/y/, the directory
/x/y/ will contains the content of /a/b/.
+ * If a directory /a/b/ is moved under the directory /x/y/, the dst needs to
be specify as /x/y/b/.
* @param srcUri URI of the original file
* @param dstUri URI of the final file location
* @param overwrite true if we want to overwrite the dstURI, false otherwise
@@ -81,20 +80,17 @@ public abstract class PinotFS implements Closeable {
LOGGER.warn("Source {} does not exist", srcUri);
return false;
}
- // if dst is an existing file
- if (exists(dstUri) && !isDirectory(dstUri)) {
- if (isDirectory(srcUri)) {
- String errorMsg = String.format("Source %s is a directory while
destination %s is a file", srcUri, dstUri);
- LOGGER.warn(errorMsg);
- throw new IOException(errorMsg);
+ if (exists(dstUri)) {
+ if (overwrite) {
+ delete(dstUri, true);
} else {
- if (overwrite) {
- delete(dstUri, true);
- } else {
- // dst file exists, returning
- return false;
- }
+ // dst file exists, returning
+ return false;
}
+ } else {
+ // ensures the parent path of dst exists.
+ URI parentUri = Paths.get(dstUri).toUri();
+ mkdir(parentUri);
}
return doMove(srcUri, dstUri);
}
@@ -108,12 +104,11 @@ public abstract class PinotFS implements Closeable {
/**
* Copies the file or directory from the src to dst. The original file is
retained. If the dst has parent directories
* that haven't been created, this method will create all the necessary
parent directories.
- * If both src and dst are files, dst will be overwritten.
- * If src is a file and dst is a directory, src file will get copied under
dst directory.
- * If both src and dst are directories, src directory will get copied under
dst directory.
- * If src is a directory and dst is a file, operation will fail.
- * For example, if a file /x/y/z is copied to /a/b/c, /x/y/z will be
retained and /x/y/z will also be present as /a/b/c;
- * if a file /a is copied to a directory /x/y, all the original files under
/x/y will be kept.
+ * Note: In Pinot we recommend the full paths of both src and dst be
specified.
+ * For example, if a file /a/b/c is copied to a file /x/y/z, in the case of
overwrite, the directory /a/b still exists,
+ * but will not contain the file 'c'. Instead, /x/y/z will contain the
contents of 'c'.
+ * If a directory /a/b/ is copied to another directory /x/y/, the directory
/x/y/ will contains the content of /a/b/.
+ * If a directory /a/b/ is copied under the directory /x/y/, the dst needs
to be specify as /x/y/b/.
* @param srcUri URI of the original file
* @param dstUri URI of the final file location
* @return true if copy is successful
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
