Backport positional deletes error

.github/workflows/MainDistributionPipeline.yml

@@ -14,23 +14,23 @@ concurrency:
 jobs:
   duckdb-stable-build:
     name: Build extension binaries
-    uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.4.3
+    uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.4.4
     with:
       extension_name: iceberg
-      duckdb_version: v1.4.3
-      ci_tools_version: v1.4.3
+      duckdb_version: v1.4.4
+      ci_tools_version: v1.4.4
       exclude_archs: 'windows_amd64_mingw'
       extra_toolchains: 'python3'
 
   duckdb-stable-deploy:
     name: Deploy extension binaries
     needs: duckdb-stable-build
-    uses: duckdb/extension-ci-tools/.github/workflows/_extension_deploy.yml@v1.4.3
+    uses: duckdb/extension-ci-tools/.github/workflows/_extension_deploy.yml@v1.4.4
     secrets: inherit
     with:
       extension_name: iceberg
-      duckdb_version: v1.4.3
-      ci_tools_version: v1.4.3
+      duckdb_version: v1.4.4
+      ci_tools_version: v1.4.4
       exclude_archs: 'windows_amd64_mingw'
       deploy_latest: ${{ startsWith(github.ref, 'refs/heads/v') || github.ref == 'refs/heads/main' }}
       deploy_versioned: ${{ startsWith(github.ref, 'refs/heads/v') || github.ref == 'refs/heads/main' }}

scripts/data_generators/tests/hour_partitioned_timestamp/__init__.py

@@ -0,0 +1,8 @@
+from scripts.data_generators.tests.base import IcebergTest
+import pathlib
+
+@IcebergTest.register()
+class Test(IcebergTest):
+    def __init__(self):
+        path = pathlib.PurePath(__file__)
+        super().__init__(path.parent.name)

scripts/data_generators/tests/hour_partitioned_timestamp/q00.sql

@@ -0,0 +1,11 @@
+CREATE or REPLACE TABLE default.hour_partitioned_timestamp (
+	id int,
+	dt TIMESTAMP,
+	customer varchar(20),
+    amount double
+)
+PARTITIONED BY (hours(dt))
+TBLPROPERTIES (
+    'format-version'='2',
+    'write.update.mode'='merge-on-read'
+);

scripts/data_generators/tests/hour_partitioned_timestamp/q01.sql

@@ -0,0 +1,16 @@
+insert into default.hour_partitioned_timestamp values
+(1000,TIMESTAMP '2025-01-01 00:00:00','C01-00',16.87),
+(1001,TIMESTAMP '2025-01-01 00:00:00','C01-01',80.19),
+(1002,TIMESTAMP '2025-01-01 00:00:00','C01-02',49.46),
+(1003,TIMESTAMP '2025-01-01 00:00:00','C01-03',75.11),
+(1004,TIMESTAMP '2025-01-02 00:00:00','C01-04',98.02),
+(2000,TIMESTAMP '2025-01-02 00:00:00','C02-00',58.46),
+(2001,TIMESTAMP '2025-01-02 00:00:00','C02-01',55.1),
+(2002,TIMESTAMP '2025-01-02 00:00:00','C02-02',16.48),
+(2003,TIMESTAMP '2025-01-02 00:00:00','C02-03',34.16),
+(2004,TIMESTAMP '2025-01-03 00:00:00','C02-04',54.99),
+(3000,TIMESTAMP '2025-01-03 00:00:00','C03-00',71.13),
+(3001,TIMESTAMP '2025-01-03 00:00:00','C03-01',82.34),
+(3002,TIMESTAMP '2025-01-04 00:00:00','C03-02',44.28),
+(3003,TIMESTAMP '2025-01-04 00:00:00','C03-03',15.93),
+(3004,TIMESTAMP '2025-01-04 00:00:00','C03-04',35.93);

scripts/data_generators/tests/month_partitioned_date/__init__.py

@@ -0,0 +1,8 @@
+from scripts.data_generators.tests.base import IcebergTest
+import pathlib
+
+@IcebergTest.register()
+class Test(IcebergTest):
+    def __init__(self):
+        path = pathlib.PurePath(__file__)
+        super().__init__(path.parent.name)

scripts/data_generators/tests/month_partitioned_date/q00.sql

@@ -0,0 +1,11 @@
+CREATE or REPLACE TABLE default.month_partitioned_date (
+	id int,
+	dt date,
+	customer varchar(20),
+    amount double
+)
+PARTITIONED BY (month(dt))
+TBLPROPERTIES (
+    'format-version'='2',
+    'write.update.mode'='merge-on-read'
+);

scripts/data_generators/tests/month_partitioned_date/q01.sql

@@ -0,0 +1,10 @@
+insert into default.month_partitioned_date values
+(1000,DATE '2025-01-01','C01-00',16.87),
+(1001,DATE '2025-01-15','C01-01',80.19),
+(1002,DATE '2025-01-31','C01-02',49.46),
+(2000,DATE '2025-06-01','C06-00',58.46),
+(2001,DATE '2025-06-15','C06-01',55.10),
+(2002,DATE '2025-06-30','C06-02',16.48),
+(3000,DATE '2025-12-01','C12-00',71.13),
+(3001,DATE '2025-12-12','C12-01',82.34),
+(3002,DATE '2025-12-31','C12-02',44.28);

src/iceberg_functions/iceberg_multi_file_list.cpp

@@ -309,7 +309,7 @@ IcebergPredicateStats IcebergPredicateStats::DeserializeBounds(const Value &lowe
 	return res;
 }
 
-bool IcebergMultiFileList::FileMatchesFilter(const IcebergManifestEntry &file) const {
+bool IcebergMultiFileList::FileMatchesFilter(const IcebergManifestEntry &file, IcebergDataFileType file_type) const {
 	D_ASSERT(!table_filters.filters.empty());
 
 	auto &filters = table_filters.filters;
@@ -325,7 +325,6 @@ bool IcebergMultiFileList::FileMatchesFilter(const IcebergManifestEntry &file) c
 		auto &metadata = GetMetadata();
 		// First check if there are partitions
 		if (!file.partition_values.empty()) {
-			// check if the index is in the parititon value thing.
 			auto partition_spec_it = metadata.partition_specs.find(file.partition_spec_id);
 			if (partition_spec_it == metadata.partition_specs.end()) {
 				throw InvalidConfigurationException(
@@ -386,8 +385,9 @@ bool IcebergMultiFileList::FileMatchesFilter(const IcebergManifestEntry &file) c
 				}
 			}
 		}
-		if (file.lower_bounds.empty() || file.upper_bounds.empty()) {
-			//! There are no bounds statistics for the file, can't filter
+		if (file.lower_bounds.empty() || file.upper_bounds.empty() || file_type == IcebergDataFileType::DELETE) {
+			// There are no bounds statistics for the file, can't filter,
+			// or it is a delete file, which should only be filtered on partitions
 			continue;
 		}
 
@@ -406,18 +406,30 @@ bool IcebergMultiFileList::FileMatchesFilter(const IcebergManifestEntry &file) c
 		auto stats = IcebergPredicateStats::DeserializeBounds(lower_bound, upper_bound, column.name, column.type);
 
 		int64_t value_count = 0;
+		bool has_value_counts = false;
 		auto value_counts_it = file.value_counts.find(column_id);
 		if (value_counts_it != file.value_counts.end()) {
 			value_count = value_counts_it->second;
+			has_value_counts = true;
 		}
 
 		auto null_counts_it = file.null_value_counts.find(column_id);
 		if (null_counts_it != file.null_value_counts.end()) {
 			auto &null_counts = null_counts_it->second;
 			stats.has_null = null_counts != 0;
-			stats.has_not_null = (value_count - null_counts) > 0;
+			if (has_value_counts) {
+				stats.has_not_null = (value_count - null_counts) > 0;
+			} else {
+				// if no value counts are active, assume there are values
+				stats.has_not_null = true;
+			}
 		} else {
-			stats.has_not_null = value_count > 0;
+			if (has_value_counts) {
+				stats.has_not_null = value_count > 0;
+			} else {
+				// if no value counts are active, assume there are values
+				stats.has_not_null = true;
+			}
 		}
 
 		auto nan_counts_it = file.nan_value_counts.find(column_id);
@@ -480,7 +492,7 @@ optional_ptr<const IcebergManifestEntry> IcebergMultiFileList::GetDataFile(idx_t
 			auto &data_file = current_data_files[data_file_idx];
 			data_file_idx++;
 			// Check whether current data file is filtered out.
-			if (!table_filters.filters.empty() && !FileMatchesFilter(data_file)) {
+			if (!table_filters.filters.empty() && !FileMatchesFilter(data_file, IcebergDataFileType::DATA)) {
 				DUCKDB_LOG(context, IcebergLogType, "Iceberg Filter Pushdown, skipped 'data_file': '%s'",
 				           data_file.file_path);
 				//! Skip this file
@@ -719,7 +731,7 @@ void IcebergMultiFileList::ProcessDeletes(const vector<MultiFileColumnDefinition
 
 		for (auto &entry : manifest_file.data_files) {
 			// Check whether current data file is filtered out.
-			if (!table_filters.filters.empty() && !FileMatchesFilter(entry)) {
+			if (!table_filters.filters.empty() && !FileMatchesFilter(entry, IcebergDataFileType::DELETE)) {
 				DUCKDB_LOG(context, IcebergLogType, "Iceberg Filter Pushdown, skipped 'data_file': '%s'",
 				           entry.file_path);
 				//! Skip this file

src/include/iceberg_multi_file_list.hpp

@@ -27,6 +27,8 @@
 
 namespace duckdb {
 
+enum class IcebergDataFileType : uint8_t { DATA, DELETE };
+
 struct IcebergMultiFileList : public MultiFileList {
 public:
 	IcebergMultiFileList(ClientContext &context, shared_ptr<IcebergScanInfo> scan_info, const string &path,
@@ -76,7 +78,7 @@ struct IcebergMultiFileList : public MultiFileList {
 
 protected:
 	bool ManifestMatchesFilter(const IcebergManifestListEntry &manifest);
-	bool FileMatchesFilter(const IcebergManifestEntry &file) const;
+	bool FileMatchesFilter(const IcebergManifestEntry &file, IcebergDataFileType file_type) const;
 	// TODO: How to guarantee we only call this after the filter pushdown?
 	void InitializeFiles(lock_guard<mutex> &guard);
 

src/include/metadata/iceberg_transform.hpp

@@ -125,8 +125,9 @@ struct MonthTransform {
 			return Value::INTEGER(diff.months);
 		}
 		case LogicalTypeId::DATE: {
-			auto val = constant.GetValue<date_t>();
-			return Value::INTEGER(val.days / Interval::DAYS_PER_MONTH);
+			int32_t year, month, day;
+			Date::Convert(constant.GetValue<date_t>(), year, month, day);
+			return Value::INTEGER((year - 1970) * 12 + (month - 1));
 		}
 		default:
 			throw NotImplementedException("'month' transform for type %s", constant.type().ToString());
@@ -194,21 +195,11 @@ struct HourTransform {
 		switch (constant.type().id()) {
 		case LogicalTypeId::TIMESTAMP: {
 			auto val = constant.GetValue<timestamp_t>();
-			auto diff = Interval::GetDifference(val, timestamp_t::epoch());
-			int32_t hours = 0;
-			hours += diff.months * (Interval::HOURS_PER_DAY * Interval::DAYS_PER_MONTH);
-			hours += diff.days * (Interval::HOURS_PER_DAY);
-			hours += diff.micros / Interval::MICROS_PER_HOUR;
-			return Value::INTEGER(hours);
+			return Value::INTEGER(static_cast<int32_t>(val.value / Interval::MICROS_PER_HOUR));
 		}
 		case LogicalTypeId::TIMESTAMP_TZ: {
 			auto val = constant.GetValue<timestamp_tz_t>();
-			auto diff = Interval::GetDifference(val, timestamp_t::epoch());
-			int32_t hours = 0;
-			hours += diff.months * (Interval::HOURS_PER_DAY * Interval::DAYS_PER_MONTH);
-			hours += diff.days * (Interval::HOURS_PER_DAY);
-			hours += diff.micros / Interval::MICROS_PER_HOUR;
-			return Value::INTEGER(hours);
+			return Value::INTEGER(static_cast<int32_t>(val.value / Interval::MICROS_PER_HOUR));
 		}
 		default:
 			throw NotImplementedException("'hour' transform for type %s", constant.type().ToString());

test/sql/local/irc/reads/partitioned_reads/test_read_from_month_partitioned_date.test

@@ -0,0 +1,61 @@
+# name: test/sql/local/irc/reads/partitioned_reads/test_read_from_month_partitioned_date.test
+# group: [partitioned_reads]
+
+require-env ICEBERG_SERVER_AVAILABLE
+
+require avro
+
+require parquet
+
+require iceberg
+
+require httpfs
+
+require icu
+
+require ducklake
+
+# Do not ignore 'HTTP' error messages!
+set ignore_error_messages
+
+statement ok
+set enable_logging=true
+
+statement ok
+SET TimeZone = 'UTC';
+
+statement ok
+CREATE SECRET (
+    TYPE S3,
+    KEY_ID 'admin',
+    SECRET 'password',
+    ENDPOINT '127.0.0.1:9000',
+    URL_STYLE 'path',
+    USE_SSL 0
+);
+
+
+statement ok
+ATTACH '' AS my_datalake (
+    TYPE ICEBERG,
+    CLIENT_ID 'admin',
+    CLIENT_SECRET 'password',
+    ENDPOINT 'http://127.0.0.1:8181'
+);
+
+
+statement ok
+call enable_logging('http');
+
+query I
+select dt from my_datalake.default.month_partitioned_date where dt = DATE '2025-06-01';
+----
+2025-06-01
+
+query I
+select dt from my_datalake.default.hour_partitioned_timestamp where dt = '2025-01-01 00:00:00';
+----
+2025-01-01 00:00:00+00
+2025-01-01 00:00:00+00
+2025-01-01 00:00:00+00
+2025-01-01 00:00:00+00

test/sql/local/irc/reads/test_read_from_table_with_deletes.test

@@ -0,0 +1,76 @@
+# name: test/sql/local/irc/reads/test_read_from_table_with_deletes.test
+# group: [reads]
+
+require-env ICEBERG_SERVER_AVAILABLE
+
+require avro
+
+require parquet
+
+require iceberg
+
+require httpfs
+
+# Do not ignore 'HTTP' error messages!
+set ignore_error_messages
+
+statement ok
+set logging_level='debug'
+
+statement ok
+CREATE SECRET (
+TYPE S3,
+  KEY_ID 'admin',
+  SECRET 'password',
+  ENDPOINT '127.0.0.1:9000',
+  URL_STYLE 'path',
+  USE_SSL 0
+);
+
+statement ok
+ATTACH '' AS my_datalake (
+  TYPE ICEBERG,
+  CLIENT_ID 'admin',
+  CLIENT_SECRET 'password',
+  ENDPOINT 'http://127.0.0.1:8181'
+);
+
+statement ok
+DROP TABLE if EXISTS my_datalake."default".test_filter_deletes;
+
+statement ok
+create table my_datalake."default".test_filter_deletes as
+from values (NULL, 'b') t(a, b);
+
+statement ok
+insert into my_datalake."default".test_filter_deletes values (NULL, 'b');
+
+query II
+select * from my_datalake."default".test_filter_deletes;
+----
+NULL	b
+NULL	b
+
+query II
+select * from my_datalake."default".test_filter_deletes where a is null;
+----
+NULL	b
+NULL	b
+
+query II
+select * from my_datalake."default".test_filter_deletes where a is not null;
+----
+
+statement ok
+DELETE FROM my_datalake."default".test_filter_deletes WHERE b = 'b';
+
+# rows are empty
+query II
+select * from my_datalake."default".test_filter_deletes;
+----
+
+# but it currently returns 2, because we skip the delete files because of the filter
+query I
+select count(*) from my_datalake."default".test_filter_deletes where a is null;
+----
+0