You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2021/09/02 16:47:06 UTC

[GitHub] [orc] pavibhai opened a new pull request #893: ORC-980 Filter processing ignores the case-sensitivity flag

pavibhai opened a new pull request #893:
URL: https://github.com/apache/orc/pull/893


   
   
   ### What changes were proposed in this pull request?
   The filter processing incorrectly ignores the case-sensitivity flag for schema supplied by the reader. This fixes this bug and uses the flag in determining field and vector information.
   
   
   ### Why are the changes needed?
   In the absence of this the read incorrectly fails by performing a case-sensitive match even when case-insensitive match was requested by the reader.
   
   
   ### How was this patch tested?
   Unit Tests were added to verify the failure and subsequently the fix.
   


-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701811596



##########
File path: java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
##########
@@ -283,6 +283,7 @@ protected RecordReaderImpl(ReaderImpl fileReader,
     Consumer<OrcFilterContext> filterCallBack = null;
     BatchFilter filter = FilterFactory.createBatchFilter(options,
                                                          evolution.getReaderBaseSchema(),
+                                                         evolution.isSchemaEvolutionCaseAware,

Review comment:
       For consistency, could you use the getter function which is added by this PR?
   ```
   public boolean isSchemaEvolutionCaseAware()
   ```




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701992522



##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringIOSkip.java
##########
@@ -570,6 +570,29 @@ public void schemaEvolutionLong2StringColumn() throws IOException {
     assertEquals(1, rowCount);
   }
 
+  @Test
+  public void readCaseInsensitive() throws IOException {

Review comment:
       Thank you so much!




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701811596



##########
File path: java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
##########
@@ -283,6 +283,7 @@ protected RecordReaderImpl(ReaderImpl fileReader,
     Consumer<OrcFilterContext> filterCallBack = null;
     BatchFilter filter = FilterFactory.createBatchFilter(options,
                                                          evolution.getReaderBaseSchema(),
+                                                         evolution.isSchemaEvolutionCaseAware,

Review comment:
       For consistency, could you use the getter function?




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun merged pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun merged pull request #893:
URL: https://github.com/apache/orc/pull/893


   


-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] pavibhai commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
pavibhai commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701973669



##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringIOSkip.java
##########
@@ -570,6 +570,29 @@ public void schemaEvolutionLong2StringColumn() throws IOException {
     assertEquals(1, rowCount);
   }
 
+  @Test
+  public void readCaseInsensitive() throws IOException {

Review comment:
       The default value is true for case-sensitivity so all the other tests are case-sensitive tests. I added an explicit failure test with case-sensitivity when the name is not found.




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] pavibhai commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
pavibhai commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701973669



##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringIOSkip.java
##########
@@ -570,6 +570,29 @@ public void schemaEvolutionLong2StringColumn() throws IOException {
     assertEquals(1, rowCount);
   }
 
+  @Test
+  public void readCaseInsensitive() throws IOException {

Review comment:
       The default value is true for case-sensitivity so all the tests are the other tests are case-sensitive tests. I added an explicit failure test with case-sensitivity when the name is not found.




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun commented on pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #893:
URL: https://github.com/apache/orc/pull/893#issuecomment-911904047


   cc @omalley 


-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] pavibhai commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
pavibhai commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701964316



##########
File path: java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
##########
@@ -283,6 +283,7 @@ protected RecordReaderImpl(ReaderImpl fileReader,
     Consumer<OrcFilterContext> filterCallBack = null;
     BatchFilter filter = FilterFactory.createBatchFilter(options,
                                                          evolution.getReaderBaseSchema(),
+                                                         evolution.isSchemaEvolutionCaseAware,

Review comment:
       Good point, changed.




-- 
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-unsubscribe@orc.apache.org

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



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #893: ORC-980: Filter processing respects the case-sensitivity flag

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #893:
URL: https://github.com/apache/orc/pull/893#discussion_r701812550



##########
File path: java/core/src/test/org/apache/orc/TestRowFilteringIOSkip.java
##########
@@ -570,6 +570,29 @@ public void schemaEvolutionLong2StringColumn() throws IOException {
     assertEquals(1, rowCount);
   }
 
+  @Test
+  public void readCaseInsensitive() throws IOException {

Review comment:
       Do we have a case-sensitive test coverage?




-- 
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-unsubscribe@orc.apache.org

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