You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/07/03 22:34:29 UTC

[GitHub] [iceberg] rdblue opened a new pull request #1164: Parquet: Fix bug building value readers

rdblue opened a new pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164


   The visitor that builds Parquet value readers for Iceberg generics did not handle cases where the expected type was null because a field was is not projected. This was not a problem for most primitive types that did not use the expected type, but cases that did would throw a `NullPointerException`.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdsr commented on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdsr commented on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654380877


   > I assumed it would have been caught by the test, but I guess we need to fix this there as well. 
   I believe it is not getting caught because in ORC (as well as Avro) the reader func are utilizing the read schema which completely match the Iceberg expected schema so there will not be a case where the iceberg type is null and the format type is not, whereas in Parquet the reader func is using the file schema.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue merged pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164


   


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdsr edited a comment on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdsr edited a comment on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654050168


   I was trying to look into why this was only a failure for Parquet. I might be wrong here, but It seems that for ORC and Avro were are building the readerFuncs in `TestReadProjection.writeAndRead` using Iceberg expected schema and format specific read schemas, whereas in Parquet we are using Iceberg expected schemas and the format specific **file** schema. Any thoughts on why that is the case?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue commented on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654322045


   @rdsr, the test applies to all formats and only Parquet was broken.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdsr commented on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdsr commented on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654050584


   Also should this be addressed for other formats?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdblue commented on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654323689


   > Any thoughts on why that is the case?
   
   The equivalent Avro builder doesn't use the expected primitive at all. The variable name is `ignored`. But looking at the ORC one, I think there would be a similar problem. I assumed it would have been caught by the test, but I guess we need to fix this there as well.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdsr edited a comment on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdsr edited a comment on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654380877


   > I assumed it would have been caught by the test, but I guess we need to fix this there as well. 
   
   I believe it is not getting caught because in ORC (as well as Avro) the reader func are utilizing the read schema which completely match the Iceberg expected schema so there will not be a case where the iceberg type is null and the format type is not, whereas in Parquet the reader func is using the file schema.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] rdsr commented on pull request #1164: Parquet: Fix NPE in value reader building for projections

Posted by GitBox <gi...@apache.org>.
rdsr commented on pull request #1164:
URL: https://github.com/apache/iceberg/pull/1164#issuecomment-654050168


   I was trying to look into why this was only a failure for Parquet. I might be wrong here, but It seems that for ORC and Avro were are building the readerFuncs in `TestReadProjection.writeAndRead` using Iceberg expected schema and format specific read schemas, whereas in Parquet we are using Iceberg expected schemas and the format specific *file* schema. Any thoughts on why that is the case?


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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org