You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "gruuya (via GitHub)" <gi...@apache.org> on 2024/12/17 14:26:55 UTC

[I] Reported and actual arrow schema of the table can be different [iceberg-rust]

gruuya opened a new issue, #813:
URL: https://github.com/apache/iceberg-rust/issues/813

   This is related to https://github.com/apache/iceberg-rust/issues/783.
   
   Namely what happens is
   - I use `pyiceberg` to create an Iceberg table from a Parquet file.
   - The Parquet file has type hints for e.g. `DataType::Int16` (`required int32 c1 (INTEGER(16,true)) = 1;`).
   - Thanks to https://github.com/apache/iceberg-rust/issues/783 we now upcast that to the native 32-bit Int type and can read it.
   - This is also the type returned in e.g. `TableProvider::schema`.
   - However the actual type in the read arrow record batches (inferred from the Parquet hint) is now `DataType::Int16`, leading to reported and actual schema mismatch.
   - This now leads to a DataFusion query such as `SELECT c1 FROM t where c1 <= 2` crashing with `Invalid comparison operation: Int16 <= Int32`
   - Ultimately the schema mismatch tricks one of the logical optimizers into thinking that if it casts the right side (i.e. the `2` literal) into `DataType::Int32` (from the reported schema) the comparison will be fine.


-- 
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: issues-unsubscribe@iceberg.apache.org.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "gruuya (via GitHub)" <gi...@apache.org>.
gruuya commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2556524539

   Even more liberally, one could do without changing `get_arrow_datum` at all
   ```diff
            if let Some(idx) = self.bound_reference(reference)? {
   -            let literal = get_arrow_datum(literal)?;
   +            let mut literal = get_arrow_datum(literal)?;
    
                Ok(Box::new(move |batch| {
                    let left = project_column(&batch, idx)?;
   +                if left.data_type() != literal.get().0.data_type() {
   +                    literal = Box::new(Scalar::new(cast(literal.get().0, left.data_type())?));
   +                }
                    lt_eq(&left, literal.as_ref())
                }))
   ```


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "gruuya (via GitHub)" <gi...@apache.org>.
gruuya commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2551525068

   Ok, I've also taken a liberty of opening a PR with an integration pyiceberg -> iceberg-rust -> datafusion integration test https://github.com/apache/iceberg-rust/pull/825.
   
   Besides showing that the basics work, it also reproduces this issue.


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2550722785

   So we have the table schema, and we have the file schema, and we must assume that they are **different**. This can be in the case that you mentioned above, where there is a different arrow type, but more obvious is in the case of schema evolution. Iceberg is lazy, and will not rewrite the historical data until it needs to update the Parquet file.
   
   For the data itself, we should project it into the schema of the table. For the filtering situation, we want to cast the type to the physical type (and also look up the file by ID!). This ties also in with the second part of https://github.com/apache/iceberg-rust/issues/777#issuecomment-2538223320 


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "gruuya (via GitHub)" <gi...@apache.org>.
gruuya commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2556514864

   > For the filtering situation, we want to cast the type to the physical type
   
   Is something like this close to what you had in mind
   ```diff
    /// Convert Iceberg Datum to Arrow Datum.
   -pub(crate) fn get_arrow_datum(datum: &Datum) -> Result<Box<dyn ArrowDatum + Send>> {
   +pub(crate) fn get_arrow_datum(
   +    datum: &Datum,
   +    arrow_type: &DataType,
   +) -> Result<Box<dyn ArrowDatum + Send>> {
        match (datum.data_type(), datum.literal()) {
            (PrimitiveType::Boolean, PrimitiveLiteral::Boolean(value)) => {
                Ok(Box::new(BooleanArray::new_scalar(*value)))
            }
   -        (PrimitiveType::Int, PrimitiveLiteral::Int(value)) => {
   -            Ok(Box::new(Int32Array::new_scalar(*value)))
   -        }
   +        (PrimitiveType::Int, PrimitiveLiteral::Int(value)) => match arrow_type {
   +            DataType::Int8 => Ok(Box::new(Int8Array::new_scalar(*value as i8))),
   +            DataType::Int16 => Ok(Box::new(Int16Array::new_scalar(*value as i16))),
   +            DataType::Int32 => Ok(Box::new(Int32Array::new_scalar(*value))),
   +            _ => Err(Error::new(
   +                ErrorKind::DataInvalid,
   +                format!("Can't convert {datum} to type {arrow_type}"),
   +            )),
   +        },
            (PrimitiveType::Long, PrimitiveLiteral::Long(value)) => {
                Ok(Box::new(Int64Array::new_scalar(*value)))
   
   ```
   
   and then for `PredicateConverter` call it only once the column is projected (and thus the target data type is known)
   ```diff
            if let Some(idx) = self.bound_reference(reference)? {
   -            let literal = get_arrow_datum(literal)?;
   -
                Ok(Box::new(move |batch| {
                    let left = project_column(&batch, idx)?;
   +                let literal = get_arrow_datum(literal, left.data_type())?;
                    lt_eq(&left, literal.as_ref())
                }))
   ```
   
   I found that this also resolves the reported problem. Though arguably less general than just casting the batches at the arrow/parquet level, it is a less invasive 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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2551058929

   I was also able to replicate this issue here: https://github.com/apache/iceberg-rust/blob/74a85e7530e40c56a2a3075be63fd943e632501d/crates/integration_tests/tests/read_evolved_schema.rs#L72-L74


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-3342136288

   This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #813: Reported and actual arrow schema of the table can be different
URL: https://github.com/apache/iceberg-rust/issues/813


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "gruuya (via GitHub)" <gi...@apache.org>.
gruuya commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-2550698180

   I see two ways of resolving this
   
   1. Align the schema returned from `TableProvider::schema` to match the one from the Parquet files. I dislike this approach as it feels hacky. For one, the canonical arrow schema for a given table will still be different than what `TableProvider::schema` returns, and moreover this would necessitate reading the Parquet file metadata when instantiating the table (and it's also vulnerable to schema drift across Parquet files in different table versions).
   2. Coerce the streamed batches from the scan to match the canonical arrow schema 
             - one way to do this is by instructing arrow-rs itself to perform the casting automatically as is suggested in #814
             - another might be to do it via the `RecordBatchTransformer`


-- 
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: issues-unsubscribe@iceberg.apache.org

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


Re: [I] Reported and actual arrow schema of the table can be different [iceberg-rust]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #813:
URL: https://github.com/apache/iceberg-rust/issues/813#issuecomment-3288995151

   This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.


-- 
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: issues-unsubscribe@iceberg.apache.org

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