drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacques-n <...@git.apache.org>
Subject [GitHub] drill pull request: DRILL-3535: Add support for Drop Table
Date Tue, 01 Sep 2015 05:21:25 GMT
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/140#discussion_r38386449
  
    --- 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));
    --- End diff --
    
    If this rename is to provide transactional capabilities, shouldn't it also be uniquely
named (rather than simply underscore prefixed)?
    
    Shouldn't we capture a failure of rename and return the user that the table no longer
exists? (in the case of a race)


---
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.
---

Mime
View raw message