[SYSTEMDS-3949] Column API parquet decode for Delta frame reads by Jakob-al28 · Pull Request #2535 · apache/systemds

Jakob-al28 · 2026-07-06T21:31:04Z

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 48.5s (2.08x). On the DeltaFrameRead benchmark from #2515: 1.73x serial, 1.51x parallel (k=8).

Format / Reader	Median (ms)
Parquet: Legacy (Group)	102,099
Parquet: Column API	53,381
Delta: Row-record	101,028
Delta: Column API	48,513
Binary Block	21,576

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

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

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 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 · 2026-07-07T13:46:43Z

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

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.
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 · 2026-07-07T14:04:00Z

Codecov Report

❌ Patch coverage is 86.40000% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.59%. Comparing base (e557836) to head (c409dc0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../org/apache/sysds/runtime/io/DeltaKernelUtils.java	85.71%	6 Missing and 9 partials ⚠️
.../org/apache/sysds/runtime/io/FrameReaderDelta.java	89.47%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2535      +/-   ##
============================================
- Coverage     71.61%   71.59%   -0.02%     
- Complexity    49867    49882      +15     
============================================
  Files          1602     1602              
  Lines        193054   193154     +100     
  Branches      37792    37817      +25     
============================================
+ Hits         138247   138293      +46     
- Misses        44031    44083      +52     
- Partials      10776    10778       +2

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

Jakob-al28 · 2026-07-07T23:35:02Z

I agree, the changes have been applied. The engine decorator and the config flag are gone, the column decode is now the path for a table's data files whenever it applies, it writes directly into the preallocated frame columns. The kernel is still used for the log/snapshot and for the cases where the data files alone aren't enough: deletion vectors and partitioned tables fall back to the kernel engine read. This simplification improved the speedup from 1.89x to 2.08x. Updated numbers are in the PR description.

Baunsgaard

A few things to consider changing, but overall a fine PR. I do not think my performance suggestions will make much of a difference since the main overhead is in parquet parsing, but it does not hurt to do.

Can you doublecheck that this API is working with various shape and size inputs, and it is faster in those cases.

e.g. 100 rows, -> 1mil rows and 1 column -> 1000 columns ?

Baunsgaard · 2026-07-10T16:12:05Z

+			MessageType parquetSchema = reader.getFooter().getFileMetaData().getSchema();
+			String createdBy = reader.getFooter().getFileMetaData().getCreatedBy();


Avoid calling reader.getFooter().getFileMetadata() twice, just put another variable.

Baunsgaard · 2026-07-10T16:13:39Z

+				// guard before decoding: writing past the limit would overflow into the
+				// next file's slice (or off the array) in the pre-allocated output


This comment is redundant, the code bellow is sufficient

Baunsgaard · 2026-07-10T16:16:59Z

+				double[] a = (double[]) dest;
+				for(int r = 0; r < nrow; r++) {
+					if(creader.getCurrentDefinitionLevel() == maxDef)
+						a[off + r] = creader.getDouble();


we can move the off + r out to the forloop, this can be done for all the cases.

int end = nrow + off; for(int r = off; r < end; r++) { }

…ge tests Validate each data file's parquet physical layout before decoding and fall back to the kernel engine for files the typed decode cannot handle, e.g. INT32 data files under a schema widened to bigint, which the kernel converts on read. Add DeltaFrameShapeCoverageTest, round-tripping shapes from 1x1 to 1M rows and 1 to 1000 columns on all three read paths (serial direct, parallel direct, forced kernel-buffered). Add tests for type-widened tables and schema evolution leftovers (the widened read fails with an UnsupportedOperationException on the previous code).

Jakob-al28 · 2026-07-11T14:07:28Z

The inline comments are applied. For the shapes and sizes I added DeltaFrameShapeCoverageTest, which round-trips 27 shapes from 1x1 up to 1M rows and 1 to 1000 columns, including the 4096 writer batch boundaries, all-null string columns, NaN/infinity/min/max values and unicode strings.

For the speed I benchmarked this branch against the kernel read path on main, same on-disk tables, same JVM flags, alternating runs, best of three each. On a 10Mx3 table with the default file layout (208MB, string/long/double with nulls) the direct decode is 1.7x faster serial and 1.5x parallel. Smaller and wider tables gain more, between 1.8x and 5.2x across the grid, no shape was slower.

While validating this I found and fixed one gap: type widening can leave INT32 data files under a column that is now bigint. The kernel converts those on read, the typed decode cannot. decodeDataFileInto now checks each file's physical types up front and falls back to the kernel read for that file only; readTypeWidenedIntFilesAsLongColumn builds such a table and fails on the previous code.

Baunsgaard

Great, Really like how this PR is turning out.

But, we need to reduce the testing to a reasonable amount for the round trips.

Another flavor of tests we could instead refine is the spark round trip testing, to ensure our reader is understanding the external engines. However this would be a followup PR.

Baunsgaard · 2026-07-12T16:30:16Z

+	private static void assertFramesEqual(String label, FrameBlock expected, FrameBlock actual) {
+		assertEquals(label + ": rows", expected.getNumRows(), actual.getNumRows());
+		assertEquals(label + ": cols", expected.getNumColumns(), actual.getNumColumns());
+		int ncol = expected.getNumColumns();
+		for(int c = 0; c < ncol; c++) {
+			assertEquals(label + ": schema col " + c, expected.getSchema()[c], actual.getSchema()[c]);
+			assertEquals(label + ": name col " + c, expected.getColumnNames()[c], actual.getColumnNames()[c]);
+		}
+		int nrow = expected.getNumRows();
+		for(int r = 0; r < nrow; r++)
+			for(int c = 0; c < ncol; c++)
+				assertEquals(label + ": cell (" + r + "," + c + ")", expected.get(r, c), actual.get(r, c));
+	}


I think in TestUtils we have a frame comparison. Can we use this?

systemds/src/test/java/org/apache/sysds/test/TestUtils.java

Line 882 in e557836

public static void compareFrames(FrameBlock expected, FrameBlock actual, boolean checkMeta) {

Baunsgaard · 2026-07-12T16:32:04Z

+	 * so the null definition-level handling and UTF-8 decode are covered at every size, including across file and batch
+	 * boundaries of the larger shapes.
+	 */
+	private static FrameBlock genFrame(int nrow, int ncol, long seed) {


Similarly, we have frame generators.

one of many examples:

systemds/src/test/java/org/apache/sysds/test/TestUtils.java

Line 2357 in e557836

public static FrameBlock generateRandomFrameBlock(int rows, ValueType[] schema, Random random){

Baunsgaard · 2026-07-12T16:37:12Z

+	private static void assertRoundTrip(int nrow, int ncol, long seed, boolean multiFile, boolean checkBuffered)
+		throws Exception {
+		roundTripAllReaders(nrow + "x" + ncol, genFrame(nrow, ncol, seed), multiFile, checkBuffered);
+	}


While I appreciate the amount of testing, It is a bit excessive ATM.

You have many calls now to assert the round trip is correct.
please reduce it to a reasonable amount (remember we will run this test many many times)

Baunsgaard · 2026-07-12T16:38:03Z

+	@Test
+	public void rowScalingRoundTrip() throws Exception {
+		// 100 -> 1M rows (the 100-row points are in tinyShapesRoundTrip); prime/odd
+		// dimensions and multi-file layouts included
+		assertRoundTrip(997, 7, 401, false, true);
+		assertRoundTrip(10_000, 3, 402, false, true);
+		assertRoundTrip(100_000, 6, 403, true, true);
+		assertRoundTrip(250_000, 2, 404, true, true);
+	}
+
+	@Test
+	public void millionRowsRoundTrip() throws Exception {
+		// the 1M-row end of the row scaling, as multi-file tables so the parallel
+		// reader really splits across files; the kernel-path parity at this scale
+		// is covered via the forced-buffered read of the 1Mx3 table
+		assertRoundTrip(1_000_000, 1, 501, true, false);
+		assertRoundTrip(1_000_000, 3, 502, true, true);
+	}


These ones are especially excessively large. Do they really test anything differently than the smaller tests?

Jakob-al28 · 2026-07-12T18:21:37Z

Done, thanks, reduced the tests to a smaller set (8 round trips total). Also swapped the comparison and data generation to use TestUtils.compareFrames and TestUtils.generateRandomFrameBlock. Agreed on the Spark round-trip refinement being a good followup.

Baunsgaard · 2026-07-12T20:39:01Z

@Jakob-al28 Thanks for the PR, and addressing the comments, It is now merged!

Jakob-al28 · 2026-07-12T21:33:41Z

Thanks for the review!

github-project-automation Bot added this to SystemDS PR Queue Jul 6, 2026

github-project-automation Bot moved this to In Progress in SystemDS PR Queue Jul 6, 2026

Jakob-al28 mentioned this pull request Jul 6, 2026

[SYSTEMDS-3929] Speed up Parquet frame reader/writer #2528

Open

Jakob-al28 and others added 3 commits July 7, 2026 20:15

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

f9925e8

Apply Eclipse formatter to CI-flagged comment lines

a4cfb70

[SYSTEMDS-3949] Decode Delta data files directly into frame columns

06c3a4b

Jakob-al28 force-pushed the delta-column-api branch from 7c7de72 to 06c3a4b Compare July 7, 2026 23:23

Baunsgaard reviewed Jul 10, 2026

View reviewed changes

[SYSTEMDS-3949] Keep shape coverage test correctness-only

4183e54

Baunsgaard reviewed Jul 12, 2026

View reviewed changes

[SYSTEMDS-3949] Reduce shape coverage test set

c409dc0

Baunsgaard merged commit f81e669 into apache:main Jul 12, 2026
51 checks passed

github-project-automation Bot moved this from In Progress to Done in SystemDS PR Queue Jul 12, 2026

		MessageType parquetSchema = reader.getFooter().getFileMetaData().getSchema();
		String createdBy = reader.getFooter().getFileMetaData().getCreatedBy();

		// guard before decoding: writing past the limit would overflow into the
		// next file's slice (or off the array) in the pre-allocated output

Uh oh!

Conversation

Jakob-al28 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Baunsgaard commented Jul 7, 2026

Uh oh!

codecov Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jakob-al28 commented Jul 7, 2026

Uh oh!

Baunsgaard left a comment

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jakob-al28 commented Jul 11, 2026

Uh oh!

Baunsgaard left a comment

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Baunsgaard Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Jakob-al28 commented Jul 12, 2026

Uh oh!

Uh oh!

Baunsgaard commented Jul 12, 2026

Uh oh!

Jakob-al28 commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jakob-al28 commented Jul 6, 2026 •

edited

Loading

codecov Bot commented Jul 7, 2026 •

edited

Loading