You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "felipecrv (via GitHub)" <gi...@apache.org> on 2023/05/04 01:04:42 UTC

[GitHub] [arrow] felipecrv opened a new pull request, #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

felipecrv opened a new pull request, #35422:
URL: https://github.com/apache/arrow/pull/35422

   ### Rationale for this change
   
   `vector::at` performs bounds checking and can throw an exception [1]. Its use is discouraged and in this specific case, the access is provably safe because array is previously resized to `buffers.size()`.
   
   [1] https://en.cppreference.com/w/cpp/container/vector/at
   
   ### What changes are included in this PR?
   
   Use of `operator[]` instead of `at()`.
   
   ### Are these changes tested?
   
   By the existing concatenation tests.


-- 
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] felipecrv commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192714247


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   Done. Pasted the links there.



-- 
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] felipecrv commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192619204


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   That's what I suspect caused the issue, yes. https://github.com/apache/arrow/issues/35571



-- 
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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1542306891

   @pitrou I only found unreleated issues, but one of them I know how and have a fix. Submitted here https://github.com/apache/arrow/pull/35533
   
   When that is merged I will rebase and force-push 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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1550061424

   > Can you rebase on git main so as to get a slightly less failing CI? :-)
   
   I'm waiting for @mapleFU's PR to be merged.


-- 
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] pitrou commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1549767142

   Can you rebase on git main so as to get a slightly less failing CI? :-)


-- 
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] mapleFU commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192703960


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   You can try to put your test code and result under the issue and I'll try to figure out why it failed tomorrow



-- 
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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1533932749

   @bkietz 


-- 
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] mapleFU commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192634010


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   Convincing! If that's true, would the code below would be better?
   
   ```
   if (std::isnan(x)) EXPECT_EQ(std::isnan(y)));
   ```
   
   or  https://github.com/google/googletest/blob/main/docs/reference/matchers.md#floating-point-matchers-fpmatchers



-- 
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] mapleFU commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1540558340

   ```c++
   
   static void VectorIndex(benchmark::State& state) {
     // Code inside this loop is measured repeatedly
     std::vector<int64_t> range_vec(10000, 0);
     for (auto _ : state) {
       // Make sure the variable is not optimized away by compiler
       for (int i = 0; i < 10000; ++i) {
         int64_t v = range_vec[i];
         benchmark::DoNotOptimize(v);
       }
     }
   }
   
   // Register the function as a benchmark
   BENCHMARK(VectorIndex);
   
   static void VectorAt(benchmark::State& state) {
     // Code inside this loop is measured repeatedly
     std::vector<int64_t> range_vec(10000, 0);
     for (auto _ : state) {
       // Make sure the variable is not optimized away by compiler
       for (int i = 0; i < 10000; ++i) {
         int64_t v = range_vec.at(i);
         benchmark::DoNotOptimize(v);
       }
     }
   }
   
   BENCHMARK(VectorAt);
   ```
   
   With release(-O2) on my MacOS:
   
   ```
   ------------------------------------------------------
   Benchmark            Time             CPU   Iterations
   ------------------------------------------------------
   VectorIndex      24632 ns        17785 ns        38620
   VectorAt         37438 ns        31141 ns        21829
   ```


-- 
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] pitrou commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1557363406

   CI failures are unrelated, will merge.


-- 
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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1538718988

   Rebased and pushed again to see if all CI can pass now.


-- 
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] felipecrv commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192630123


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   I will have a dedicated PR if that error disappears from the CI builds here. I don't have a Windows/AMD64/MINGW setup, so I'm relying fully on CI.



-- 
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] ursabot commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1569115001

   Benchmark runs are scheduled for baseline = 584dc7bcde7afa0c67cedf737bac6b4b7a221088 and contender = 2216a0a483b6e5f9633bbee85c5a623360ca8a94. 2216a0a483b6e5f9633bbee85c5a623360ca8a94 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/c11a305bfaa04256a0cfd81cd2d4cd22...91139e85e6464be18b0a24bac5525570/)
   [Finished :arrow_down:0.86% :arrow_up:0.09%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/97a973a48b5f4c1aa5acb7f587d3dbf1...ff5e21efe68842d6b2268e5ec1eebb79/)
   [Finished :arrow_down:1.63% :arrow_up:4.58%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/3a26bc43b5d74370b5c2b5de14c57684...9cf73ac83f0a44179e6538b2c1c7babd/)
   [Finished :arrow_down:0.52% :arrow_up:1.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/678e29ba1a694843b8656c26bfa133c1...c6981bebc1e345819fbbd6e1a07ee06d/)
   Buildkite builds:
   [Finished] [`2216a0a4` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2914)
   [Finished] [`2216a0a4` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2950)
   [Finished] [`2216a0a4` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2915)
   [Finished] [`2216a0a4` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2940)
   [Finished] [`584dc7bc` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2913)
   [Finished] [`584dc7bc` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2949)
   [Finished] [`584dc7bc` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2914)
   [Finished] [`584dc7bc` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2939)
   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] mapleFU commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192615733


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   Just curious why would you change float comparing to this. Did you found NaN 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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1546011760

   I pushed a commit here to test a fix for parquet CI tests. If it passes, I will create a separate issue/PR for it.


-- 
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] felipecrv commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1541254986

   > This PR is ok but I'm not sure std::vector::at is "discouraged". Can you provide a reference to that claim?
   
   @pitrou C++ lore.
   
   `at()` is forbidden in exception-free codebases which Arrow mostly is: `Status` and `Result` instead of `throw` everywhere.
   
   We return a bad `Status` when we bounds-check and try to avoid bounds-checking when we can prove that access is safe. That's why I believe it's fair to say the use of `at` is discouraged — Arrow's bounds-checks are explicit, non-superfluous and lead to bad `Status` instead of an exception being thrown.


-- 
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] pitrou merged pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #35422:
URL: https://github.com/apache/arrow/pull/35422


-- 
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] pitrou commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1540534764

   This PR is ok but I'm not sure `std::vector::at` is "discouraged". Can you provide a reference to that claim?


-- 
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] pitrou commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1541511402

   > We return a bad `Status` when we bounds-check and try to avoid bounds-checking when we can prove that access is safe. That's why I believe it's fair to say the use of `at` is discouraged — Arrow's bounds-checks are explicit, non-superfluous and lead to bad `Status` instead of an exception being thrown.
   
   Ah, that's a good point.


-- 
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] pitrou commented on pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35422:
URL: https://github.com/apache/arrow/pull/35422#issuecomment-1541513649

   The "[AMD64 Conda C++](https://github.com/apache/arrow/actions/runs/4918687806/jobs/8806728403?pr=35422#logs)" CI failure seems unexpected, can you take a look?


-- 
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] felipecrv commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192630123


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   I will have a dedicated PR if that error disappears from the CI builds here. I don't have a Windows/AMD64/MINGW setup, so I'm relying fully on CI 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] felipecrv commented on a diff in pull request #35422: MINOR: [C++] Use [] instead of exception-throwing at(i) in concatenate.cc

Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35422:
URL: https://github.com/apache/arrow/pull/35422#discussion_r1192701832


##########
cpp/src/parquet/encryption/test_encryption_util.cc:
##########
@@ -322,7 +323,14 @@ void ReadAndVerifyColumn(RowGroupReader* rg_reader, RowGroupMetadata* rg_md,
   }
   ASSERT_EQ(rows_read, rows_should_read);
   ASSERT_EQ(values_read, rows_should_read);
-  ASSERT_EQ(read_col_data.values, expected_column_data.values);

Review Comment:
   Results are here and it's bad news:
   
   ```
   D:/a/arrow/arrow/cpp/src/parquet/encryption/test_encryption_util.cc:328: Failure
   Expected equality of these values:
     memcmp(read_col_data.values.data(), expected_column_data.values.data(), read_col_data.values.size() * sizeof(typename DType::c_type))
       Which is: 1
     0
   ```



-- 
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