You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2023/01/03 08:47:05 UTC
[GitHub] [arrow-rs] liukun4515 opened a new pull request, #3431: Support decimal int32/64 for writer
liukun4515 opened a new pull request, #3431:
URL: https://github.com/apache/arrow-rs/pull/3431
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes https://github.com/apache/arrow-rs/issues/205
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
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] tustvold commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1062189222
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -135,8 +143,40 @@ impl<W: Write> ArrowWriter<W> {
));
}
- for (buffer, column) in self.buffer.iter_mut().zip(batch.columns()) {
- buffer.push_back(column.clone())
+ for ((buffer, column), parquet_field) in self
+ .buffer
+ .iter_mut()
+ .zip(batch.columns())
+ .zip(&self.parquet_fields)
+ {
+ match column.data_type() {
+ // if the arrow data type is decimal
+ ArrowDataType::Decimal128(_, _) => {
Review Comment:
I think this logic should be moved to write_leaf where we have other coercion logic, this will also be simpler
--
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] tustvold commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1062189222
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -135,8 +143,40 @@ impl<W: Write> ArrowWriter<W> {
));
}
- for (buffer, column) in self.buffer.iter_mut().zip(batch.columns()) {
- buffer.push_back(column.clone())
+ for ((buffer, column), parquet_field) in self
+ .buffer
+ .iter_mut()
+ .zip(batch.columns())
+ .zip(&self.parquet_fields)
+ {
+ match column.data_type() {
+ // if the arrow data type is decimal
+ ArrowDataType::Decimal128(_, _) => {
Review Comment:
I think this logic should be moved to write_leaf where we have other coercion logic
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370615611
> > I think the data in the arrow ecosystem is exchanged by IPC format
>
> Sometimes, but an important property is that data written by one implementation to CSV, Parquet, or whatever can be read by another
>
why is it related to other file format?
The changes just enhance the writing for parquet file format, and it will not impact the CSV and other file format.
> To phrase my concern differently, decimals are a relatively esoteric type, with most arrow implementations having limited support. I worry with this PR we will now write decimal data in such a way arrow implementations that used to understand it, now won't.
>
> Can you confirm pyarrow at least can correctly read the data written by this PR?
From https://github.com/apache/parquet-cpp/blob/master/src/parquet/arrow/reader.cc#L1227, the c++ support reading the decimal data from INT32/INT64, but c++ does not support writing decimal using the INT32/INT64 parquet physical type https://github.com/apache/parquet-cpp/blob/master/src/parquet/arrow/writer.cc#L811, this is consistent with the comments for the arrow writing parquet.
```
DECIMAL | INT32 / INT64 / BYTE_ARRAY / FIXED_LENGTH_BYTE_ARRAY | Decimal128 / Decimal256 | (2)
(2) On the write side, a FIXED_LENGTH_BYTE_ARRAY is always emitted.
```
The writing path of go is same with the c++.
go: https://github.com/apache/arrow/blob/master/go/parquet/pqarrow/schema.go#L303
But I can't find the writing path for the pyarrow. @tustvold
But all languages support reading the decimal from INT32/INT64/FIXED_BYTE_ARRAY/BYTE_ARRAY from parquet file.
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370620114
If we need to following other language of arrow, we should close this pr.
Thanks @tustvold
But I want to know why we not implement writing decimal with INT32 or INT64 for parquet file, maybe it is a matter of history, and I will file a email about this in the dev mail list.
--
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] ursabot commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1378160075
Benchmark runs are scheduled for baseline = a8276c09b1a4eb31886e288919bf31ea655550b9 and contender = ccb80e82bbfd19e8b353e107ba41cfe0cbaa029a. ccb80e82bbfd19e8b353e107ba41cfe0cbaa029a is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/ad25c157fa1643cd9f383236e86cb5e9...83352c59e65d41aeacd4254597f7ec66/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/25643ff12b4c42a7b2a7a261ad44bdd3...85df1140720d4b18b3a6ef91ddd2ce70/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/8875521c7fb945dd9eb362739cbe2d0f...f38b66edd859449d8f57198a4aa22f57/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/22046061edcd42a793ab550f99f9bd03...e96c59bbb7fe4926a916c2ed6ae7260a/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370433954
I also find the schema mapping in java version of the `parquet-mr` project: https://github.com/apache/parquet-mr/blob/433de8df33fcf31927f7b51456be9f53e64d48b9/parquet-arrow/src/main/java/org/apache/parquet/arrow/schema/SchemaConverter.java#L227, and it supports the mapping from arrow decimal type to INT32/INT64 parquet physical type.
But find the implementation of c++ version arrow: https://arrow.apache.org/docs/cpp/parquet.html#logical-types, and find some notes about the arrow decimal:
```
DECIMAL | INT32 / INT64 / BYTE_ARRAY / FIXED_LENGTH_BYTE_ARRAY | Decimal128 / Decimal256 | (2)
(2) On the write side, a FIXED_LENGTH_BYTE_ARRAY is always emitted.
```
--
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] nevi-me commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
nevi-me commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1371889652
> Looks like @nevi-me filed the original ticket -- I wonder if he has any additional context?
Hey @alamb @tustvold
My recollection is that the idea was that if the precision is low enough, Parquet can write the file to i32 or i64 physicla types to result in smaller files (https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#decimal).
There's 2 scenarios:
* When reading from Arrow > Parquet, should the behaviour to use the smallest possible physical type based on precision?
* When reading from Parquet > Arrow, I suppose Decimal128 makes sense.
--
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] tustvold commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1065847480
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -435,6 +444,15 @@ fn write_leaf(
let array: &[i64] = data.buffers()[0].typed_data();
write_primitive(typed, &array[offset..offset + data.len()], levels)?
}
+ ArrowDataType::Decimal128(_, _) => {
+ // use the int32 to represent the decimal with low precision
+ let array = column
Review Comment:
You could consider using as_primitive_array 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
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1377311794
PTAL @tustvold @alamb
--
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] alippai commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
alippai commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1379323390
Is this backwards compatible for datasets? Eg if 2022.parquet was written the old way, 2023.parquet was using the new physical type, can I query/read both with the mixed physical types?
--
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] tustvold commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1369651399
What is the ecosystem support for this like? Do all arrow implementations understand how to convert to a decimal128 from i32 or i64? Just wondering if we need to put this behind an optional flag?
--
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] tustvold commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370621428
> I will file a email about this in the dev mail list.
I would be very interested in why arrow C++ doesn't write to Int32/Int64
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370428422
> What is the ecosystem support for this like? Do all arrow implementations understand how to convert to a decimal128 from i32 or i64? Just wondering if we need to put this behind an optional flag?
This implementation just contains the writer path for parquet file for decimal data, and
just following the definition of parquet format: https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#decimal
```
DECIMAL can be used to annotate the following types:
int32: for 1 <= precision <= 9
int64: for 1 <= precision <= 18; precision < 10 will produce a warning
fixed_len_byte_array: precision is limited by the array size. Length n can store <= floor(log_10(2^(8*n - 1) - 1)) base-10 digits
binary: precision is not limited, but is required. The minimum number of bytes to store the unscaled value should be used.
```
> Do all arrow implementations understand how to convert to a decimal128 from i32 or i64?
I think the data in the arrow ecosystem is exchanged by IPC format for the different language like `rust -> java`, or `c++ -> rust`.
cc @tustvold
--
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] tustvold commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1370578823
> I think the data in the arrow ecosystem is exchanged by IPC format
Sometimes, but an important property is that data written by one implementation to CSV, Parquet, or whatever can be read by another
To phrase my concern differently, decimals are a relatively esoteric type, with most arrow implementations having limited support. I worry with this PR we will now write decimal data in such a way arrow implementations that used to understand it, now won't.
Can you confirm pyarrow at least can correctly read the data written by 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1371287531
Looks like @nevi-me filed the original ticket -- I wonder if he has any additional context?
--
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] liukun4515 merged pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 merged PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431
--
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] liukun4515 commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1065779555
##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -399,21 +399,32 @@ fn arrow_to_parquet_type(field: &Field) -> Result<Type> {
.with_length(*length)
.build()
}
- DataType::Decimal128(precision, scale)
- | DataType::Decimal256(precision, scale) => {
+ DataType::Decimal128(precision, scale) => {
// Decimal precision determines the Parquet physical type to use.
- // TODO(ARROW-12018): Enable the below after ARROW-10818 Decimal support
- //
- // let (physical_type, length) = if *precision > 1 && *precision <= 9 {
- // (PhysicalType::INT32, -1)
- // } else if *precision <= 18 {
- // (PhysicalType::INT64, -1)
- // } else {
- // (
- // PhysicalType::FIXED_LEN_BYTE_ARRAY,
- // decimal_length_from_precision(*precision) as i32,
- // )
- // };
+ // Following the: https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#decimal
+ let (physical_type, length) = if *precision > 1 && *precision <= 9 {
+ (PhysicalType::INT32, -1)
+ } else if *precision <= 18 {
+ (PhysicalType::INT64, -1)
+ } else {
+ (
+ PhysicalType::FIXED_LEN_BYTE_ARRAY,
+ decimal_length_from_precision(*precision) as i32,
+ )
+ };
+ Type::primitive_type_builder(name, physical_type)
+ .with_repetition(repetition)
+ .with_length(length)
+ .with_logical_type(Some(LogicalType::Decimal {
+ scale: *scale as i32,
+ precision: *precision as i32,
+ }))
+ .with_precision(*precision as i32)
+ .with_scale(*scale as i32)
+ .build()
+ }
+ DataType::Decimal256(precision, scale) => {
Review Comment:
Now we ignore the decimal 256
--
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] tustvold commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1379430060
Yes, this should be transparent to users
--
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] tustvold commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1060487459
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -129,14 +137,51 @@ impl<W: Write> ArrowWriter<W> {
/// and drop any fully written `RecordBatch`
pub fn write(&mut self, batch: &RecordBatch) -> Result<()> {
// validate batch schema against writer's supplied schema
- if self.arrow_schema != batch.schema() {
+ if self.arrow_schema != batch.schema()
+ || self.arrow_schema.fields().len() != self.parquet_schema.columns().len()
+ {
return Err(ParquetError::ArrowError(
"Record batch schema does not match writer schema".to_string(),
));
}
- for (buffer, column) in self.buffer.iter_mut().zip(batch.columns()) {
- buffer.push_back(column.clone())
+ for ((buffer, column), parquet_column_field) in self
+ .buffer
+ .iter_mut()
+ .zip(batch.columns())
+ .zip(self.parquet_schema.columns())
+ {
+ match column.data_type() {
+ // if the arrow data type is decimal
+ ArrowDataType::Decimal128(_, _) => {
+ match parquet_column_field.as_ref().physical_type() {
+ // the physical data type is not fixed length byte array
+ // convert the decimal array to the INT32/INT64 array
+ Type::INT32 => {
+ let new_array = column
+ .as_any()
+ .downcast_ref::<Decimal128Array>()
+ .unwrap()
+ .iter()
+ .map(|v| v.map(|v| v as i32))
+ .collect::<Int32Array>();
+ buffer.push_back(Arc::new(new_array))
+ }
+ Type::INT64 => {
+ let new_array = column
+ .as_any()
+ .downcast_ref::<Decimal128Array>()
+ .unwrap()
+ .iter()
+ .map(|v| v.map(|v| v as i64))
+ .collect::<Int64Array>();
Review Comment:
```suggestion
.unary::<_, Int64Array>(|v| v as i64);
```
--
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] tustvold commented on a diff in pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#discussion_r1060487225
##########
parquet/src/arrow/arrow_writer/mod.rs:
##########
@@ -129,14 +137,51 @@ impl<W: Write> ArrowWriter<W> {
/// and drop any fully written `RecordBatch`
pub fn write(&mut self, batch: &RecordBatch) -> Result<()> {
// validate batch schema against writer's supplied schema
- if self.arrow_schema != batch.schema() {
+ if self.arrow_schema != batch.schema()
+ || self.arrow_schema.fields().len() != self.parquet_schema.columns().len()
+ {
return Err(ParquetError::ArrowError(
"Record batch schema does not match writer schema".to_string(),
));
}
- for (buffer, column) in self.buffer.iter_mut().zip(batch.columns()) {
- buffer.push_back(column.clone())
+ for ((buffer, column), parquet_column_field) in self
+ .buffer
+ .iter_mut()
+ .zip(batch.columns())
+ .zip(self.parquet_schema.columns())
+ {
+ match column.data_type() {
+ // if the arrow data type is decimal
+ ArrowDataType::Decimal128(_, _) => {
+ match parquet_column_field.as_ref().physical_type() {
+ // the physical data type is not fixed length byte array
+ // convert the decimal array to the INT32/INT64 array
+ Type::INT32 => {
+ let new_array = column
+ .as_any()
+ .downcast_ref::<Decimal128Array>()
+ .unwrap()
+ .iter()
+ .map(|v| v.map(|v| v as i32))
+ .collect::<Int32Array>();
Review Comment:
```suggestion
.unary::<_, Int32Array>(|v| v as i32);
```
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1371720716
> So what is this PR waiting on? Some demonstration that parquet files written with decimal and smaller field width can be read by some other parquet implementation?
go/c++ arrow can read decimal from INT32/INT64 parquet physical type.
I have filed a email to talk about this in C++ version and the notes in the document, but until now I have not got the response.
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1369510823
cc @alamb
--
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] liukun4515 commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1374493907
> I think let's move forward with this, and we can reassess if we hear anything back
From the feedback of the email, the arrow of c++ has the plan to support this. https://github.com/apache/arrow/issues/15239
Can we going on this PR？
@tustvold @alamb
--
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] alippai commented on pull request #3431: Support decimal int32/64 for writer
Posted by GitBox <gi...@apache.org>.
alippai commented on PR #3431:
URL: https://github.com/apache/arrow-rs/pull/3431#issuecomment-1379447449
I was asking because of the hybrid schema. It’s good it depends on the logical schema instead of the physical types 🎉
Thanks for the swift reply
--
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