https://issues.apache.org/jira/browse/ARROW-17904

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

Thanks for the PR @mapleFU . I haven't looked in detail yet.

IMHO The testing approach should be to add reference files in https://github.com/apache/parquet-testing/tree/master/data .
Ideally these files should provide the given feature matrix:

• correct and incorrect CRCs
• v1 and v2 data pages
• compressed and uncompressed columns

This would probably be spread over two files, one with correct CRCs, one with incorrect CRCs.
What do you think?

I'd like to follow the testing here: https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestDataPageV1Checksums.java
@pitrou

I'd like to follow the testing here: https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestDataPageV1Checksums.java

This actually seems more complicated to implement (you have to replicate details of the file format in the tests).
Also, without reference files we wouldn't test that different implementations actually agree on the CRC computation.

This actually seems more complicated to implement (you have to replicate details of the file format in the tests). Also, without reference files we wouldn't test that different implementations actually agree on the CRC computation.

Sure, let me generate some data files in parquet-mr following TestDataPageV1Checksums first.

@pitrou @kou Hi, so should I put crc to vendored or util?

Since the code is simple and the algorithm well-know, my vote is to own it and keep it in util.

I like cpp/src/arrow/vendored/ but it's not a strong opinion. So I'm OK with util.

@mapleFU Is this still WIP?

Some comment fixed, test would be added later

I add a crc32 test which crc "123456789". But I found it hard to find some authority crc tests:

1. boost crc only crc "123456789"
2. folly crc uses boost crc to do some cross verification

So, I wonder where should I copy more crc tests? Or just test "123456789" is enough? @pitrou @kou

As I said before:

Also, we need unit tests for the CRC32 functionality using well-known test vectors (or generated from a well-known implementation, such as Python's).

And, yes, one or two test vectors is enough. But make sure to include bytes >= 128 (to check for signedness issues).

https://github.com/python/cpython/blob/3ceb4b8d3ad48101e67da8fb8f581d3377863bfa/Lib/test/test_binascii.py#L237-L242
Seems that it's also short, lol...

@pitrou I decide to add boost crc32 for testing, though we don't introduce it in our code, but I think using it to test is a goot idea.

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\random(1839): error C2338: invalid template argument for uniform_int_distribution: N4659 29.6.1.1 [rand.req.genl]/1e requires one of short, int, long, long long, unsigned short, unsigned int, unsigned long, or unsigned long long
D:/a/arrow/arrow/cpp/src/arrow/util/crc32_test.cc(55): note: see reference to class template instantiation 'std::uniform_int_distribution<uint8_t>' being compiled
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\random(1839): error C2338: note: char, signed char, unsigned char, char8_t, int8_t, and uint8_t are not allowed

Seems windows cannot use random uint8_t...

Oh, seems #34038 cause the error... Let's waiting it to be merged...
@pitrou @wjones127 and comments are fixed, PTAL again