You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2023/01/25 23:48:08 UTC
[GitHub] [arrow-rs] viirya opened a new issue, #3605: Relax column type check in RecordBatch initialization
viirya opened a new issue, #3605:
URL: https://github.com/apache/arrow-rs/issues/3605
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
<!--
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
(This section helps Arrow developers understand the context and *why* for this feature, in addition to the *what*)
-->
While working on internal project incorporating Arrow/DataFusion, we encounter some issues on reading non-dictionary and dictionary arrays for same column.
During initializatoin `RecordBatch` checks if each field type in given schema matches column type in arrays. But two issues are annoying:
1. Before reading in arrays, we only know datatype of column (not arrow datatype), so we don't know if it is dictionary or not. I.e., we only know it is `T` but don't know it is `Dictionary<_, T>`.
2. While reading different files, it is possible that same column is only dictionary encoded in some files instead of all files. It causes schema mismatch later.
I'm wondering if we can relax the column type check to allow compatible one like `T` to `Dictionary<_, T>`.
**Describe the solution you'd like**
<!--
A clear and concise description of what you want to happen.
-->
**Describe alternatives you've considered**
<!--
A clear and concise description of any alternative solutions or features you've considered.
-->
We could not do this.
**Additional context**
<!--
Add any other context or screenshots about the feature request here.
-->
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on issue #3605: Relax column type check in RecordBatch initialization
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #3605:
URL: https://github.com/apache/arrow-rs/issues/3605#issuecomment-1404786191
I think it would be confusing for the RecordBatch to have a schema that doesn't match its contents, especially as this might have non-trivial implications for things like IPC. That being said, I could definitely see value in allowing DataFusion to be more relaxed about dictionaries, the only obvious issue being that many kernels won't work on a mix of dictionary and non-dictionary columns
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jhorstmann commented on issue #3605: Relax column type check in RecordBatch initialization
Posted by "jhorstmann (via GitHub)" <gi...@apache.org>.
jhorstmann commented on issue #3605:
URL: https://github.com/apache/arrow-rs/issues/3605#issuecomment-1404714438
Interesting usecase.
Looking at our codebase I think there are many places that assume `batch.columns()[n].data_type() == batch.schema().fields()[n].data_type()`. We have a separate data type enum for the logical plan level though, so that could indicate a logical type of string, and then during physical execution it could be represented as StringArray, DictionaryArray<String> or even LargeStringArray.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on issue #3605: Relax column type check in RecordBatch initialization
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on issue #3605:
URL: https://github.com/apache/arrow-rs/issues/3605#issuecomment-1407134778
Currently at our integration side, once there is inconsistent column reading in (e.g. `Dictionary<_T>` for a column type `T`), we cast it to `T` and vice versa to make DataFusion happy. I think this is similar to the idea to add type coercions to DataFusion. I feel it is more reasonable to have this in upstream projection instead of handling it in downstream.
But this brings additional computation cost too. Not to mention that it drops the benefit of dictionary-encoding.
> I think it would be confusing for the RecordBatch to have a schema that doesn't match its contents, especially as this might have non-trivial implications for things like IPC.
It doesn't sound to have mismatched schema and arrays in a RecordBatch. My idea is to modify (coerce?) given schema to match given arrays for dictionary-encoded ones. Maybe it could be configured by an option?
> That being said, I could definitely see value in allowing DataFusion to be more relaxed about dictionaries, the only obvious issue being that many kernels won't work on a mix of dictionary and non-dictionary columns
Yea, this is one important concern. But I think this is already an issue even the schema of arrays read in is not 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] crepererum commented on issue #3605: Relax column type check in RecordBatch initialization
Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on issue #3605:
URL: https://github.com/apache/arrow-rs/issues/3605#issuecomment-1404836534
> A simple, but naive idea might be to add type coercions to DataFusion's schema adaption logic so that queries over heterogeneous schemas are possible. This would likely entail hydrating dictionaries, but I'm not sure this is necessarily avoidable
Note that depending on the batches and their sizes, it may be better to dictionary-encode the non-dictionary batches (e.g. when you have a large dictionary-encoded batch and a tiny non-dictionary-encoded one). That's also a good reason to not hide this somewhere in arrow but make this a deliberate step of the DataFusion type coercion.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org