[GitHub] [orc] pgaref commented on a change in pull request #564: ORC-274: Remove columnNames field from Reader.Options

[GitBox <gi...@apache.org>]

pgaref commented on a change in pull request #564:
URL: https://github.com/apache/orc/pull/564#discussion_r578509303



##########
File path: java/core/src/java/org/apache/orc/Reader.java
##########
@@ -390,8 +399,15 @@ public SearchArgument getSearchArgument() {
       return preFilterColumns;
     }
 
+    /**
+     * Get the search argument leaves' column-name mapping.
+     * @return an array of column names, one for every predicate leave, or NULL when search argument not set
+     */
     public String[] getColumnNames() {
-      return columnNames;
+      if (this.sarg != null) {
+        return sarg.getLeaves().stream().map(l -> l.getColumnName()).toArray(String[]::new);

Review comment:
       Hey @dongjoon-hyun these are valid concerns, in general columnNames var is not used anymore (even on 1.5 branch).
   The actual mapping to internal colIdx is happening as part of mapSargColumnsToOrcInternalColIdx which in turn calls RecordReaderImpl.findColumns where:
   
   * Depending on the schema isSchemaEvolutionCaseAware var -- the SArgs columnNames can be case-insensitive or not
   * If there is not reader schema-file type mapping the colName will be -1 (no Sargs applied for that column)
   * If the column is not found in the Reader schema at all, it will throw an exception
   
   In that sense, its the caller's responsibility to use the right Reader schema, with correct column names, define if needed the case-sensitivity (as part of schema.isSchemaEvolutionCaseAware) and handle any missing schema type issues.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org