Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/140#discussion_r38386470
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
 ---
    @@ -321,8 +327,101 @@ public DrillTable create(String key) {
           return null;
         }
     
    +    private FormatMatcher findMatcher(FileStatus file) {
    +      FormatMatcher matcher = null;
    +      try {
    +        for (FormatMatcher m : dropFileMatchers) {
    +          if (m.isFileReadable(fs, file)) {
    +            return m;
    +          }
    +        }
    +      } catch (IOException e) {
    +        logger.debug("Failed to find format matcher for file: %s", file, 
e);
    +      }
    +      return matcher;
    +    }
    +
         @Override
         public void destroy(DrillTable value) {
         }
    +
    +    /**
    +     * Check if the table contains homogenenous files that can be read by 
Drill. Eg: parquet, json csv etc.
    +     * However if it contains more than one of these formats or a totally 
different file format that Drill cannot
    +     * understand then we will raise an exception.
    +     * @param key
    +     * @return
    +     * @throws IOException
    +     */
    +    private boolean isHomogeneous(String key) throws IOException {
    +      FileSelection fileSelection = FileSelection.create(fs, 
config.getLocation(), key);
    +
    +      if (fileSelection == null) {
    +        throw UserException
    +            .validationError()
    +            .message(String.format("Table [%s] not found", key))
    +            .build(logger);
    +      }
    +
    +      FormatMatcher matcher = null;
    +      Queue<FileStatus> listOfFiles = new LinkedList<>();
    +      listOfFiles.addAll(fileSelection.getFileStatusList(fs));
    +
    +      while (!listOfFiles.isEmpty()) {
    +        FileStatus currentFile = listOfFiles.poll();
    +        if (currentFile.isDirectory()) {
    +          listOfFiles.addAll(fs.list(true, currentFile.getPath()));
    +        } else {
    +          if (matcher != null) {
    +            if (!matcher.isFileReadable(fs, currentFile)) {
    +              return false;
    +            }
    +          } else {
    +            matcher = findMatcher(currentFile);
    +            // Did not match any of the file patterns, exit
    +            if (matcher == null) {
    +              return false;
    +            }
    +          }
    +        }
    +      }
    +      return true;
    +    }
    +
    +    /**
    +     * We check if the table contains homogeneous file formats that Drill 
can read. Once the checks are performed
    +     * we rename the file to start with an "_". After the rename we issue 
a recursive delete of the directory.
    +     * @param table - Path of table to be dropped
    +     */
    +    @Override
    +    public void dropTable(String table) {
    +
    +      String[] pathSplit = table.split(Path.SEPARATOR);
    +      String dirName = DrillFileSystem.HIDDEN_FILE_PREFIX + 
pathSplit[pathSplit.length - 1];
    +      int lastSlashIndex = table.lastIndexOf(Path.SEPARATOR);
    +
    +      if (lastSlashIndex != -1) {
    +        dirName = table.substring(0, lastSlashIndex + 1) + dirName;
    +      }
    +
    +      DrillFileSystem fs = getFS();
    +      String defaultLocation = getDefaultLocation();
    +      try {
    +        if (!isHomogeneous(table)) {
    +          throw UserException
    +              .validationError()
    +              .message("Table contains different file formats. \n" +
    +                  "Drop Table is only supported for directories that 
contain homogeneous file formats consumable by Drill")
    +              .build(logger);
    +        }
    +        fs.rename(new Path(defaultLocation, table), new 
Path(defaultLocation, dirName));
    +        fs.delete(new Path(defaultLocation, dirName), true);
    +      } catch (IOException e) {
    --- End diff --
    
    Shouldn't the permission error only be constrained to certain types of 
IOExceptions, not all?  It seems like failing on the delete would indicate 
something else.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to