AlenkaF commented on code in PR #38360:
URL: https://github.com/apache/arrow/pull/38360#discussion_r1392136172
##########
python/pyarrow/tests/parquet/test_basic.py:
##########
@@ -882,3 +884,141 @@ def test_thrift_size_limits(tempdir):
assert got == table
got = pq.read_table(path)
assert got == table
+
+
+def test_page_checksum_verification_write_table(tempdir):
Review Comment:
Could we add one more example similar to this with `write_to_dataset`
(`test_page_checksum_verification_write_to_dataset(tempdir)`)?
The cases to be handled are:
- `pyarrow.parquet.write_to_dataset` to write and `pq.read_table` to read.
- `pyarrow.parquet.write_to_dataset` with `use_legacy_dataset=True` to write
and `pq.read_table` to read.
The test will need to use `@pytest.mark.dataset` and the second might also
need `@pytest.mark.filterwarnings("ignore:Passing
'use_legacy_dataset=True':FutureWarning")`.
##########
python/pyarrow/tests/parquet/test_basic.py:
##########
@@ -882,3 +884,141 @@ def test_thrift_size_limits(tempdir):
assert got == table
got = pq.read_table(path)
assert got == table
+
+
+def test_page_checksum_verification_write_table(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_table()"""
+
+ # Write some sample data into a parquet file with page checksum enabled
+ original_path = tempdir / 'correct.parquet'
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+ pq.write_table(table_orig, original_path, page_checksum_enabled=True)
+
+ # Read file and verify that the data is correct
+ table_check = pq.read_table(original_path, page_checksum_verification=True)
+ assert table_orig == table_check
+
+ # Read the original file as binary and swap the 31-th and 36-th bytes. This
+ # should be equivalent to storing the following data:
+ # pa.table({'a': [1, 3, 2, 4]})
+ bin_data = bytearray(original_path.read_bytes())
+
+ # Swap two bytes to emulate corruption. Also, check that the two bytes are
+ # different, otherwise no corruption occurs
+ assert bin_data[31] != bin_data[36]
+ bin_data[31], bin_data[36] = bin_data[36], bin_data[31]
+
+ # Write the corrupted data to another parquet file
+ corrupted_path = tempdir / 'corrupted.parquet'
+ corrupted_path.write_bytes(bin_data)
+
+ # Case 1: Reading the corrupted file with read_table() and without page
+ # checksum verification succeeds but yields corrupted data
+ table_corrupt = pq.read_table(corrupted_path,
+ page_checksum_verification=False)
+ # The read should complete without error, but the table has different
+ # content than the original file!
+ assert table_corrupt != table_orig
+ assert table_corrupt == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 2: Reading the corrupted file with read_table() and with page
+ # checksum verification enabled raises an exception
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = pq.read_table(corrupted_path, page_checksum_verification=True)
+
+ # Case 3: Reading the corrupted file with ParquetFile.read() and without
+ # page checksum verification succeeds but yields corrupted data
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=False)
+ table_corrupt2 = corrupted_pq_file.read()
+ assert table_corrupt2 != table_orig
+ assert table_corrupt2 == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 4: Reading the corrupted file with ParquetFile.read() and with page
+ # checksum verification enabled raises an exception
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=True)
+ # Accessing the data should result in an error
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = corrupted_pq_file.read()
+
+ # Case 5: Check that enabling page checksum verification in combination
+ # with legacy dataset raises an exception
+ with pytest.raises(ValueError, match="page_checksum_verification"):
+ _ = pq.read_table(corrupted_path,
+ page_checksum_verification=True,
+ use_legacy_dataset=True)
+
+
+def test_page_checksum_verification_write_dataset(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_dataset()"""
+
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+
+ pq_write_format = pa.dataset.ParquetFileFormat()
+ # not sure if ParquetFileWriteOptions will be enough here
+ write_options = pq_write_format.make_write_options(
+ page_checksum_enabled=True)
Review Comment:
```suggestion
# Write a sample dataset with page checksum enabled
pq_write_format = pa.dataset.ParquetFileFormat()
write_options = pq_write_format.make_write_options(
page_checksum_enabled=True)
```
##########
python/pyarrow/tests/parquet/test_basic.py:
##########
@@ -882,3 +884,141 @@ def test_thrift_size_limits(tempdir):
assert got == table
got = pq.read_table(path)
assert got == table
+
+
+def test_page_checksum_verification_write_table(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_table()"""
+
+ # Write some sample data into a parquet file with page checksum enabled
+ original_path = tempdir / 'correct.parquet'
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+ pq.write_table(table_orig, original_path, page_checksum_enabled=True)
+
+ # Read file and verify that the data is correct
+ table_check = pq.read_table(original_path, page_checksum_verification=True)
+ assert table_orig == table_check
+
+ # Read the original file as binary and swap the 31-th and 36-th bytes. This
+ # should be equivalent to storing the following data:
+ # pa.table({'a': [1, 3, 2, 4]})
+ bin_data = bytearray(original_path.read_bytes())
+
+ # Swap two bytes to emulate corruption. Also, check that the two bytes are
+ # different, otherwise no corruption occurs
+ assert bin_data[31] != bin_data[36]
+ bin_data[31], bin_data[36] = bin_data[36], bin_data[31]
+
+ # Write the corrupted data to another parquet file
+ corrupted_path = tempdir / 'corrupted.parquet'
+ corrupted_path.write_bytes(bin_data)
+
+ # Case 1: Reading the corrupted file with read_table() and without page
+ # checksum verification succeeds but yields corrupted data
+ table_corrupt = pq.read_table(corrupted_path,
+ page_checksum_verification=False)
+ # The read should complete without error, but the table has different
+ # content than the original file!
+ assert table_corrupt != table_orig
+ assert table_corrupt == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 2: Reading the corrupted file with read_table() and with page
+ # checksum verification enabled raises an exception
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = pq.read_table(corrupted_path, page_checksum_verification=True)
+
+ # Case 3: Reading the corrupted file with ParquetFile.read() and without
+ # page checksum verification succeeds but yields corrupted data
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=False)
+ table_corrupt2 = corrupted_pq_file.read()
+ assert table_corrupt2 != table_orig
+ assert table_corrupt2 == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 4: Reading the corrupted file with ParquetFile.read() and with page
+ # checksum verification enabled raises an exception
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=True)
+ # Accessing the data should result in an error
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = corrupted_pq_file.read()
+
+ # Case 5: Check that enabling page checksum verification in combination
+ # with legacy dataset raises an exception
+ with pytest.raises(ValueError, match="page_checksum_verification"):
+ _ = pq.read_table(corrupted_path,
+ page_checksum_verification=True,
+ use_legacy_dataset=True)
+
+
+def test_page_checksum_verification_write_dataset(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_dataset()"""
Review Comment:
```suggestion
"""Check that checksum verification works for datasets created with
ds.write_dataset"""
```
##########
python/pyarrow/tests/parquet/test_basic.py:
##########
@@ -882,3 +884,141 @@ def test_thrift_size_limits(tempdir):
assert got == table
got = pq.read_table(path)
assert got == table
+
+
+def test_page_checksum_verification_write_table(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_table()"""
+
+ # Write some sample data into a parquet file with page checksum enabled
+ original_path = tempdir / 'correct.parquet'
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+ pq.write_table(table_orig, original_path, page_checksum_enabled=True)
+
+ # Read file and verify that the data is correct
+ table_check = pq.read_table(original_path, page_checksum_verification=True)
+ assert table_orig == table_check
+
+ # Read the original file as binary and swap the 31-th and 36-th bytes. This
+ # should be equivalent to storing the following data:
+ # pa.table({'a': [1, 3, 2, 4]})
+ bin_data = bytearray(original_path.read_bytes())
+
+ # Swap two bytes to emulate corruption. Also, check that the two bytes are
+ # different, otherwise no corruption occurs
+ assert bin_data[31] != bin_data[36]
+ bin_data[31], bin_data[36] = bin_data[36], bin_data[31]
+
+ # Write the corrupted data to another parquet file
+ corrupted_path = tempdir / 'corrupted.parquet'
+ corrupted_path.write_bytes(bin_data)
+
+ # Case 1: Reading the corrupted file with read_table() and without page
+ # checksum verification succeeds but yields corrupted data
+ table_corrupt = pq.read_table(corrupted_path,
+ page_checksum_verification=False)
+ # The read should complete without error, but the table has different
+ # content than the original file!
+ assert table_corrupt != table_orig
+ assert table_corrupt == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 2: Reading the corrupted file with read_table() and with page
+ # checksum verification enabled raises an exception
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = pq.read_table(corrupted_path, page_checksum_verification=True)
+
+ # Case 3: Reading the corrupted file with ParquetFile.read() and without
+ # page checksum verification succeeds but yields corrupted data
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=False)
+ table_corrupt2 = corrupted_pq_file.read()
+ assert table_corrupt2 != table_orig
+ assert table_corrupt2 == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 4: Reading the corrupted file with ParquetFile.read() and with page
+ # checksum verification enabled raises an exception
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=True)
+ # Accessing the data should result in an error
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = corrupted_pq_file.read()
+
+ # Case 5: Check that enabling page checksum verification in combination
+ # with legacy dataset raises an exception
+ with pytest.raises(ValueError, match="page_checksum_verification"):
+ _ = pq.read_table(corrupted_path,
+ page_checksum_verification=True,
+ use_legacy_dataset=True)
+
+
+def test_page_checksum_verification_write_dataset(tempdir):
Review Comment:
Could we move this test to `python/pyarrow/tests/test_dataset.py`?
##########
python/pyarrow/tests/parquet/test_basic.py:
##########
@@ -882,3 +884,141 @@ def test_thrift_size_limits(tempdir):
assert got == table
got = pq.read_table(path)
assert got == table
+
+
+def test_page_checksum_verification_write_table(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_table()"""
+
+ # Write some sample data into a parquet file with page checksum enabled
+ original_path = tempdir / 'correct.parquet'
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+ pq.write_table(table_orig, original_path, page_checksum_enabled=True)
+
+ # Read file and verify that the data is correct
+ table_check = pq.read_table(original_path, page_checksum_verification=True)
+ assert table_orig == table_check
+
+ # Read the original file as binary and swap the 31-th and 36-th bytes. This
+ # should be equivalent to storing the following data:
+ # pa.table({'a': [1, 3, 2, 4]})
+ bin_data = bytearray(original_path.read_bytes())
+
+ # Swap two bytes to emulate corruption. Also, check that the two bytes are
+ # different, otherwise no corruption occurs
+ assert bin_data[31] != bin_data[36]
+ bin_data[31], bin_data[36] = bin_data[36], bin_data[31]
+
+ # Write the corrupted data to another parquet file
+ corrupted_path = tempdir / 'corrupted.parquet'
+ corrupted_path.write_bytes(bin_data)
+
+ # Case 1: Reading the corrupted file with read_table() and without page
+ # checksum verification succeeds but yields corrupted data
+ table_corrupt = pq.read_table(corrupted_path,
+ page_checksum_verification=False)
+ # The read should complete without error, but the table has different
+ # content than the original file!
+ assert table_corrupt != table_orig
+ assert table_corrupt == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 2: Reading the corrupted file with read_table() and with page
+ # checksum verification enabled raises an exception
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = pq.read_table(corrupted_path, page_checksum_verification=True)
+
+ # Case 3: Reading the corrupted file with ParquetFile.read() and without
+ # page checksum verification succeeds but yields corrupted data
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=False)
+ table_corrupt2 = corrupted_pq_file.read()
+ assert table_corrupt2 != table_orig
+ assert table_corrupt2 == pa.table({'a': [1, 3, 2, 4]})
+
+ # Case 4: Reading the corrupted file with ParquetFile.read() and with page
+ # checksum verification enabled raises an exception
+ corrupted_pq_file = pq.ParquetFile(corrupted_path,
+ page_checksum_verification=True)
+ # Accessing the data should result in an error
+ with pytest.raises(OSError, match="CRC checksum verification"):
+ _ = corrupted_pq_file.read()
+
+ # Case 5: Check that enabling page checksum verification in combination
+ # with legacy dataset raises an exception
+ with pytest.raises(ValueError, match="page_checksum_verification"):
+ _ = pq.read_table(corrupted_path,
+ page_checksum_verification=True,
+ use_legacy_dataset=True)
+
+
+def test_page_checksum_verification_write_dataset(tempdir):
+ """Check that checksum verification works for datasets created with
+ pq.write_dataset()"""
+
+ table_orig = pa.table({'a': [1, 2, 3, 4]})
+
+ pq_write_format = pa.dataset.ParquetFileFormat()
+ # not sure if ParquetFileWriteOptions will be enough here
+ write_options = pq_write_format.make_write_options(
+ page_checksum_enabled=True)
+
+ original_dir_path = tempdir / 'correct_dir'
+ ds.write_dataset(
+ data=table_orig,
+ base_dir=original_dir_path,
+ format=pq_write_format,
+ file_options=write_options,
+ )
+
+ # Read file and verify that the data is correct
Review Comment:
```suggestion
# Open dataset and verify that the data is correct
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
