[ 
https://issues.apache.org/jira/browse/PARQUET-1229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17112063#comment-17112063
 ] 

ASF GitHub Bot commented on PARQUET-1229:
-----------------------------------------

gszadovszky commented on a change in pull request #776:
URL: https://github.com/apache/parquet-mr/pull/776#discussion_r427925111



##########
File path: parquet-hadoop/src/main/java/org/apache/parquet/crypto/AesCipher.java
##########
@@ -68,19 +67,32 @@
 
   public static byte[] createModuleAAD(byte[] fileAAD, ModuleType moduleType, 
       short rowGroupOrdinal, short columnOrdinal, short pageOrdinal) {

Review comment:
       I understand that large numbers of pages/row groups or columns would 
lead to significant performance drawbacks but it should not limit what the spec 
allows otherwise.
   
   Since it is discussed and approved already, I am fine with using `short` 
values for these. What I would suggest adding though is to have the conversion 
from `int` to `short` centralized and and have specific error messages so it is 
clear that the limit reached is a hard limit for the encryption feature. Also, 
if we will publish any description/example for the encryption feature these 
limitations shall be listed there.
   
   One more thing: the check of `intValue > Short.MAX_VALUE` is not complete. 
In case of `intValue` is negative the cast may result in a valid positive 
`short` value. I would suggest using `(short) intValue != intValue` instead.




----------------------------------------------------------------
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:
[email protected]


> parquet-mr code changes for encryption support
> ----------------------------------------------
>
>                 Key: PARQUET-1229
>                 URL: https://issues.apache.org/jira/browse/PARQUET-1229
>             Project: Parquet
>          Issue Type: Sub-task
>          Components: parquet-mr
>            Reporter: Gidon Gershinsky
>            Assignee: Gidon Gershinsky
>            Priority: Major
>              Labels: pull-request-available
>
> Addition of encryption/decryption support to the existing Parquet classes and 
> APIs



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to