Skip to content

[SYSTEMDS-3949] Column API parquet decode for Delta frame reads#2535

Open
Jakob-al28 wants to merge 2 commits into
apache:mainfrom
Jakob-al28:delta-column-api
Open

[SYSTEMDS-3949] Column API parquet decode for Delta frame reads#2535
Jakob-al28 wants to merge 2 commits into
apache:mainfrom
Jakob-al28:delta-column-api

Conversation

@Jakob-al28

Copy link
Copy Markdown

Follow-up to #2515, as discussed in #2528.

Delta Kernel's default engine decodes parquet through parquet-mr's row-record API (ParquetReader + ReadSupport/RecordMaterializer), which is built to reassemble nested records; each cell goes through a PrimitiveConverter callback. Frames are flat in SystemDS, so this adds a thin Engine decorator that overrides ParquetHandler.readParquetFiles: flat schemas decode through the column API (ColumnReadStoreImpl/ColumnReader). Everything else (predicates, nested/decimal/timestamp columns, unknown metadata columns) falls through to the wrapped default engine, and deletion vectors, partition values and column mapping are still applied by the kernel above the handler.

On TPC-H lineitem, 30M rows (median of 7 runs, cloud VM, n2-highmem-8, 40GB heap): 101.0s to 53.3s (1.89x). On the DeltaFrameRead benchmark from #2515: 1.68x serial, 1.50x parallel (k=8).

Format / Reader Median (ms)
Parquet: Legacy (Group) 102,099
Parquet: Column API 53,381
Delta: default engine 101,028
Delta: column-API engine 53,324
Binary Block 21,576

All runs at the same settings: ZSTD, no dictionary for Parquet and Delta; Binary Block uncompressed.

matched_encoding_all deltaframeread

Two cases needed explicit handling: tables with deletion vectors enabled ask every read for a synthetic _metadata.row_index column that isn't actually in the file, and older files in a table that's had a column added later don't contain that column at all. Both would have failed otherwise; now the first is computed and the second comes back as null, covered by tests (DeltaFrameSparkContractTest). There's also a config kill switch (sysds.io.delta.reader.columnapi, default on).

Notes: the ParquetHandler contract has grown across kernel versions (the row_index requirement came with deletion vector support), so delta-kernel version bumps should re-run the contract tests. The fast path materializes one row group at a time, a bigger per-batch memory footprint than the default engine's fixed-size batches. The decode relies on parquet-mr's column API (org.apache.parquet.column.impl); its signatures are unchanged from parquet 1.13 through 1.17.

The write path stays on the default engine. Two checks suggest there is little to gain there: the parquet writer rewrite in #2528, which removed the per-row Group materialization, only moved write time from 92s to 87s, and with matched codecs the Delta write is within about 8% of that plain parquet writer (2.7s Delta vs 2.5s parquet on 1M lineitem rows, both set to snappy for this check). Neither points to a large win available on the write side.

@Baunsgaard

Copy link
Copy Markdown
Contributor

Overall this looks good, but i would change some things.

  1. can we instead of adding a new way of reading the delta format, simply overwrite the old. We do not want to maintain may ways, if your way is better lets default to that, and only support that.
  2. I am wondering if we can avoid using the Engine abstraction with the columnAPI. This would allow us to avoid allocating the result FrameBlock's arrays times, and instead directly put the values into preallocated columns.

Lets me know what you think?

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.62%. Comparing base (afe793a) to head (7c7de72).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../apache/sysds/runtime/io/ColumnApiDeltaEngine.java 86.23% 20 Missing and 10 partials ⚠️
.../org/apache/sysds/runtime/io/DeltaKernelUtils.java 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2535      +/-   ##
============================================
+ Coverage     71.59%   71.62%   +0.02%     
- Complexity    49684    49876     +192     
============================================
  Files          1596     1603       +7     
  Lines        192298   193230     +932     
  Branches      37675    37825     +150     
============================================
+ Hits         137680   138402     +722     
- Misses        43909    44058     +149     
- Partials      10709    10770      +61     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants