drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #666: DRILL-4956: Temporary tables support
Date Tue, 06 Dec 2016 06:12:56 GMT
Github user paul-rogers commented on a diff in the pull request:

    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
    @@ -82,6 +84,9 @@ public void init(Map<String, String> writerOptions) throws IOException
         Path fileName = new Path(location, prefix + "_" + index + "." + extension);
         try {
           stream = fs.create(fileName);
    +      // set storage strategy for folder and file
    +      storageStrategy.apply(fs, fileName.getParent());
    +      storageStrategy.apply(fs, fileName);
    --- End diff --
    Hmmm... The strategy has permission of "775" (permanent) and "700" (temporary). Those
include executable. May be fine for the directory. Not so good for the data file. Maybe need
an applyToFile and applyToDir? Or, let the storage strategy split the name?
    Also, it is not clear if we are creating the directory here, or just changing an existing
one. Seems we should honor existing perms on existing dirs; but set then only on new dirs.
So, maybe have a mkdir() method that works like mkdirs(): create dir if needed (and imply
permissions) or do nothing if the directory exists.
    Does writer here create a directory? Will we know to remove it at end of session? We set
delete on exit, but exit may be weeks away (for a long-lived Drillbit). Do we know to cascade
deletes down into directories if the writer creates a partitioned file?

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.

View raw message