ump45nose opened a new pull request, #1406:
URL: https://github.com/apache/arrow-go/pull/1406

   ### Rationale for this change
   
   Closes #69.
   
   When a file has columns encrypted with their own keys and the 
`FileDecryptionProperties` only carry keys for some of them, touching one of 
the other columns panics with `hidden column exception, path=...` from 
`fileDecryptor.getColumnDecryptor`. This happens on any path that reaches that 
column, e.g. `RowGroupMetaData.ColumnChunk(i)`, 
`RowGroupReader.GetColumnPageReader(i)`, or `pqarrow.FileReader.ReadRowGroups` 
with that column selected. Reading only the columns you have keys for already 
works, but there is no way to recover from the panic when a hidden column is 
touched.
   
   ### What changes are included in this PR?
   
   - `FileDecryptor.GetColumnMetaDecryptor` / `GetColumnDataDecryptor` 
(internal interface) now return `(Decryptor, error)`, and a missing column key 
produces an error naming the column instead of a panic.
   - `NewColumnChunkMetaData`, `RowGroupReader.GetColumnPageReader` and 
`encryption.GetColumnMetaDecryptor` propagate that error. The last one already 
returned an error, so the page index and bloom filter readers pick it up 
without changes.
   
   This does not add a way to read the still-encrypted bytes of a hidden 
column, which was also discussed in the issue; it only turns the panic into an 
error so the rest of the file stays readable.
   
   ### Are these changes tested?
   
   Yes. Added `TestReadEncryptedFileWithMissingColumnKey` in `parquet/pqarrow`: 
it writes a file with two columns encrypted with different keys, opens it with 
only the first key, checks that the first column reads correctly, and that 
column chunk metadata, the page reader and `ReadRowGroups` for the second 
column all return an error. The test panics on `main`. `go test ./parquet/...` 
passes with `PARQUET_TEST_DATA` / `ARROW_TEST_DATA` set.
   
   ### Are there any user-facing changes?
   
   Reading an encrypted column without its key now returns an error instead of 
panicking. The changed interface is in `parquet/internal/encryption`, so there 
is no public API change.
   
   This change was written with the help of an AI assistant. The panic was 
reproduced, the diff reviewed and the tests above run before opening this PR.
   


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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to