Github user jinfengni commented on a diff in the pull request:

    https://github.com/apache/drill/pull/461#discussion_r58616431
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java ---
    @@ -194,4 +195,26 @@ public void dropTable(String tableName) {
             .message("Dropping tables is not supported in schema [%s]", 
getSchemaPath())
             .build(logger);
       }
    -}
    +
    +  /**
    +   * Visit the tables in this schema and write to recordGenerator
    +   * @param recordGenerator recordGenerator for the output
    +   * @param schemaPath      the path to the given schema
    +   */
    --- End diff --
    
    Why do you add this API to AbstractSchema? We should not put the logic of 
RecordGenerator in Schema. 
    
    A cleaner way is to add getTablesByNames() in AbstractSchema, and leave the 
logic of iterating and record generating to RecordGenerator.  If the underlying 
storage supports bulk load ( like Hive does), then use the bulk load. 
Otherwise, just call getTable() individually for getTablesByNames(). 
    



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