Implement truncate transform

[jvansanten]

I've been thoroughly enjoying the iceberg extension, but now find myself needing to partition my dataset on integer-valued columns. The extension handles this gracefully if inefficiently at the moment, simply matching all partitions based on unimplemented transforms, and kicking the filtering down to the parquet scan.

This PR adds support for the truncate transform, allowing iceberg_scan to prune partitions created with truncate at the manifest level. Truncate is defined (and implemented here) for int, long, decimal, bytes, and string types. All other types raise an error.

New sqllogic tests verify that scans return the correct results, and also skip files as intended.

It should be noted that iceberg_scan can't actually read tables truncated-partitioned on decimal fields, but will be able to after #744.

[Tishj]

Thanks for the PR!
I would like to see some tests implemented for the various types that this adds support for
Perhaps have a look at #737 which was merged recently to see which tests to add and how

[jvansanten]

Thanks for the feedback! How would you recommend testing manifest-level pruning? Looking at the tests in #737, it looks like they only verify the result of the query, and so would have passed even before bucket partition filtering was added. Or have I misunderstood the tests?

[Tishj]

I believe they check the amount of logged parquet files that are scanned, which is affected by file pruning

query I
select count(distinct request.url) from duckdb_logs_parsed('HTTP') where request.url like '%.parquet';
----
6

But you can also look at in_filter.test:

query I
select count(*) FROM duckdb_logs() where type = 'Iceberg' and message.contains('data_file')
----
4

Which originates from here:

			// Check whether current data file is filtered out.
			if (!table_filters.filters.empty() && !FileMatchesFilter(manifest_entry, IcebergDataFileType::DATA)) {
				DUCKDB_LOG(context, IcebergLogType, "Iceberg Filter Pushdown, skipped 'data_file': '%s'",
				           data_file.file_path);
				//! Skip this file
				continue;
			}

There is also a manifest_file equivalent:

query I
select count(*) FROM duckdb_logs() where type = 'Iceberg' and message.contains('manifest_file')
----
4

Comes from here:

			if (!ManifestMatchesFilter(manifest_file)) {
				DUCKDB_LOG(context, IcebergLogType, "Iceberg Filter Pushdown, skipped 'manifest_file': '%s'",
				           manifest_file.manifest_path);
				//! Skip this manifest
				continue;
			}

[jvansanten]

Tests were a good idea, and now implemented for integers, binary, and varchar. I also added a test generator for decimal data in eaaa2c5, but no test as yet, as reading the produced table with iceberg_scan results in:

libc++abi: terminating due to uncaught exception of type duckdb::Exception: {"exception_type":"Conversion","exception_message":"Error while reading file \".../duckdb-iceberg/data/generated/iceberg/spark-local/default/truncate_partitioned_decimal/metadata/74f694af-7f68-4d63-8909-4a25453b876d-m0.avro\": : Unimplemented type for cast (BLOB -> DECIMAL(10,2))\n\n"}

That appears to be triggered by an attempt to deserialize the partition key in the manifest, but I have no idea where that actually happens or how to fix it.

Edit: this appears to happen because the partition type is written verbatim into the manifest schema:

duckdb-iceberg/src/iceberg_manifest.cpp

Line 70 in 5bbc6db

children.emplace_back("partition", partition_type);

and something else (duckdb-avro?) chokes on it. I can work around it for this specific case by replacing the partition type with the known serialized type (STRUCT(r1000 BLOB)) in this specific case, but obviously that's not a solution.

[Tishj]

Your issue might be fixed as part of #744
Because that identified and fixed a translation issue between avro -> duckdb types, which required interpreting the logicalType in the avro side

[jvansanten]

Your issue might be fixed as part of #744

Indeed it appears to be; thanks for the tip! Now added the test, which fails with the above error, but passes when merged with #744.

[Tmonster]

Thanks! looks really good to me. Can we also add a test where we make sure DuckDB can still correctly read a truncated table with no filters?

[jvansanten]

Thanks! looks really good to me. Can we also add a test where we make sure DuckDB can still correctly read a truncated table with no filters?

I think this is implicit in the first 2 assertions of all the new tests, e.g. https://github.com/duckdb/duckdb-iceberg/pull/766/changes#diff-6766e54322ca7bee8e0c361ec8dc79aeb08a115b99731d0e64426891c5e4337eR13-R16, no?

Fun fact: these tests also pass against the head of #820, as for the truncate transform there's only one edge case where the difference between partition and range pruning is observable, namely BETWEEN with an upper bound exactly at the partition boundary. I'm loath to add that case, though, because it will break if #820 is merged. Actually testing whether the file pruned by partition or range would likely require #780.

[Tmonster]

Thanks!
Sorry, must have missed that. Looks like the scans are tested.
I had one more comment about the placement of the files, but forgot to include it in my review.

[Tmonster]

You can move the test files to test/sql/local/irc_any_catalog/reads/partitioned_reads. We have a test config that will automatically attach a catalog with the generated spark tables

[jvansanten]

Can do. Currently these tests just use iceberg_scan; should they use an attached catalog instead?

[Tmonster]

yea that would be great thanks 🙏

[Tmonster]

Thanks!