[BUG] Fix parquet reads with limit across row groups

[desmondcheongzx]

When reading local parquet files containing multiple row groups with a limit applied, sometimes the resulting table does not respect the given limit, causing errors such as DaftError::ValueError While building a Table with Table::new_with_size, we found that the Series lengths did not match. Series named: col had length: 2048 vs the specified Table length: 1034 to be thrown.

The issue was a small bug where each row group range being read would take the global limit passed into the parquet read, instead of the pre-computed row group limit, which is aware of how many rows had been read by previous row groups. This caused the parquet reader to read more rows from a row group than specified.

To fix this, we pass the pre-computed row group limit properly to the reader.

For example, consider a parquet file with the following layout:

Column: col
--------------------------------------------------------------------------------
  page   type  enc  count   avg size   size       rows     nulls   min / max
  0-D    dict  S _  1       5.00 B     5 B       
  0-1    data  S R  1024    0.01 B     11 B                0       "b" / "b"
  1-D    dict  S _  1       5.00 B     5 B       
  1-1    data  S R  1024    0.01 B     11 B                0       "b" / "b"
  2-D    dict  S _  1       5.00 B     5 B       
  2-1    data  S R  1024    0.01 B     11 B                0       "b" / "b"
  3-D    dict  S _  1       5.00 B     5 B       
  3-1    data  S R  1024    0.01 B     11 B                0       "b" / "b"

When applying a .limit(1050) over this parquet file, with the bug, we would read 1024 rows each from row groups 0 and 1 (data pages 0-1 and 1-1). Row groups 2 and 3 are skipped because the pre-computed row ranges sees that we have the required 1050 rows in the first two row groups. However, the pre-computed row ranges are aware that we only need 26 entries from row group 1, so we simply pass this information correctly into the reader.

[kevinzwang]

LGTM! Thanks again for looking into this

[kevinzwang]

To my understanding, reading this file should have worked before. It just used to error if we applied a limit, which is not done by any tests over the URLs in DAFT_CAN_READ_FILES.

[desmondcheongzx]

Oh you're totally right. Forgot that this file doesn't apply a limit.

Moved the S3 tests

[codspeed-hq]

CodSpeed Performance Report

Merging #2751 will improve performances by ×3

_{Comparing desmondcheongzx:fix-local-parquet-limits (c463352) with main (3a7a0b4)}

Summary

⚡ 1 improvements
✅ 15 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	desmondcheongzx:fix-local-parquet-limits	Change
⚡	test_show[100 Small Files]	154.2 ms	51.1 ms	×3

[kevinzwang]

@desmondcheongzx could you fix up the failing integration test and merge this in?

[desmondcheongzx]

@kevinzwang sounds good. I fixed the integration test and will merge once CI passes