Skip to content

Commit

Permalink
fix: Fix withIntDistributionForField copy (#11644)
Browse files Browse the repository at this point in the history
Summary:
For decimal(8, 5) data build, Arrow reports below error due to out of range.
The reason is in the copy of 'withIntDistributionForField', the target index
and source index are reversed.
```
`C++ exception with description "Invalid: Invalid cast from Decimal128 to 4 byte integer" thrown in the test body.
```

Pull Request resolved: #11644

Reviewed By: Yuhta

Differential Revision: D66560309

Pulled By: bikramSingh91

fbshipit-source-id: bd594f67a58b92915ea3bf24bee3a311db11eb04
  • Loading branch information
rui-mo authored and facebook-github-bot committed Jan 30, 2025
1 parent 3fd46c7 commit 6a05878
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
2 changes: 1 addition & 1 deletion velox/dwio/common/tests/utils/DataSetBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class DataSetBuilder {
if (counter % 100 < repeats) {
numbers->set(row, T(counter % repeats));
} else if (counter % 100 > 90 && row > 0) {
numbers->copy(numbers, row - 1, row, 1);
numbers->copy(numbers, row, row - 1, 1);
} else {
int64_t value;
if (rareFrequency && counter % rareFrequency == 0) {
Expand Down
4 changes: 4 additions & 0 deletions velox/dwio/parquet/tests/reader/E2EFilterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,11 @@ TEST_F(E2EFilterTest, floatAndDouble) {
}

TEST_F(E2EFilterTest, shortDecimalDictionary) {
// decimal(8, 5) maps to 4 bytes FLBA in Parquet.
// decimal(10, 5) maps to 5 bytes FLBA in Parquet.
// decimal(17, 5) maps to 8 bytes FLBA in Parquet.
for (const auto& type : {
"shortdecimal_val:decimal(8, 5)",
"shortdecimal_val:decimal(10, 5)",
"shortdecimal_val:decimal(17, 5)",
}) {
Expand All @@ -386,9 +388,11 @@ TEST_F(E2EFilterTest, shortDecimalDirect) {
options_.enableDictionary = false;
options_.dataPageSize = 4 * 1024;

// decimal(8, 5) maps to 4 bytes FLBA in Parquet.
// decimal(10, 5) maps to 5 bytes FLBA in Parquet.
// decimal(17, 5) maps to 8 bytes FLBA in Parquet.
for (const auto& type : {
"shortdecimal_val:decimal(8, 5)",
"shortdecimal_val:decimal(10, 5)",
"shortdecimal_val:decimal(17, 5)",
}) {
Expand Down

0 comments on commit 6a05878

Please sign in to comment.