stiga-huang commented on a change in pull request #1055:
URL: https://github.com/apache/orc/pull/1055#discussion_r820087592



##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -38,6 +38,8 @@
 public class SchemaEvolution {
   // indexed by reader column id
   private final TypeDescription[] readerFileTypes;
+  // key: file column id, value: reader column id
+  private final Map<Integer, Integer> typeIdsMap = new HashMap<>();

Review comment:
       I think we can use an array just like `readerFileTypes`.
   
   BTW, for readability, I'd suggest renaming `readerFileTypes` to 
`readerIdToFileTypes` and renaming `typeIdsMap` to `fileIdToReaderIds`.

##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -296,13 +303,13 @@ private boolean typesAreImplicitConversion(final 
TypeDescription fileType,
 
   /**
    * Check if column is safe for ppd evaluation
-   * @param colId reader column id
+   * @param colId file column id

Review comment:
       For readability, can we also rename `colId` to `fileColId`?

##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -296,13 +303,13 @@ private boolean typesAreImplicitConversion(final 
TypeDescription fileType,
 
   /**
    * Check if column is safe for ppd evaluation
-   * @param colId reader column id
+   * @param colId file column id
    * @return true if the specified column is safe for ppd evaluation else false
    */
   public boolean isPPDSafeConversion(final int colId) {
     if (hasConversion()) {
-      return !(colId < 0 || colId >= ppdSafeConversion.length) &&
-          ppdSafeConversion[colId];
+      Integer readerTypeId = typeIdsMap.get(colId);
+      return readerTypeId != null && ppdSafeConversion[readerTypeId];

Review comment:
       Since we only use `ppdSafeConversion[]` in this method, I think changing 
`ppdSafeConversion[]` to be indexed by file column ids is a simpler solution, 
i.e. modifying the assignment in `populatePpdSafeConversion` and 
`populatePpdSafeConversionForChildren` to use file ids.




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

To unsubscribe, e-mail: dev-unsubscr...@orc.apache.org

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


Reply via email to