Merge pull request #1150 from smvv/fix-retry-transaction

Fix off-by-one in retry transaction

Author: pdet

duckdb

@@ -5,8 +5,8 @@
 #include "duckdb/parser/parser.hpp"
 #include "duckdb/common/file_system.hpp"
 #include "storage/ducklake_metadata_manager.hpp"
-#include "duckdb/planner/filter/optional_filter.hpp"
-#include "duckdb/planner/filter/dynamic_filter.hpp"
+#include "duckdb/planner/filter/expression_filter.hpp"
+#include "duckdb/planner/filter/table_filter_functions.hpp"
 #include "duckdb/function/scalar/variant_utils.hpp"
 
 #include <cmath>
@@ -321,19 +321,8 @@ string DuckLakeUtil::JoinPath(FileSystem &fs, const string &a, const string &b)
 	}
 }
 
-DynamicFilter *DuckLakeUtil::GetOptionalDynamicFilter(const TableFilter &filter) {
-	if (filter.filter_type != TableFilterType::OPTIONAL_FILTER) {
-		return nullptr;
-	}
-	auto &optional = filter.Cast<OptionalFilter>();
-	if (!optional.child_filter || optional.child_filter->filter_type != TableFilterType::DYNAMIC_FILTER) {
-		return nullptr;
-	}
-	auto &dynamic = optional.child_filter->Cast<DynamicFilter>();
-	if (!dynamic.filter_data) {
-		return nullptr;
-	}
-	return &dynamic;
+shared_ptr<DynamicFilterData> DuckLakeUtil::GetOptionalDynamicFilterData(const TableFilter &filter) {
+	return ExpressionFilter::GetRootOptionalDynamicFilterData(filter);
 }
 
 bool DuckLakeUtil::IsInlinedSystemColumn(const string &name) {

extension-ci-tools

@@ -738,8 +738,8 @@ static unique_ptr<LogicalOperator> FlushInlinedDataBind(ClientContext &context,
 	unique_ptr<Expression> sum_col_ref = make_uniq<BoundColumnRefExpression>(aggregate->types[2], agg_bindings[2]);
 	// Note: SUM(BIGINT) returns HUGEINT. We must use the its output type for the 0 constant
 	unique_ptr<Expression> zero_const = make_uniq<BoundConstantExpression>(Value::Numeric(aggregate->types[2], 0));
-	unique_ptr<Expression> filter_expr = make_uniq<BoundComparisonExpression>(
-	    ExpressionType::COMPARE_GREATERTHAN, std::move(sum_col_ref), std::move(zero_const));
+	unique_ptr<Expression> filter_expr =
+	    BoundComparisonExpression::Create(ExpressionType::COMPARE_GREATERTHAN, std::move(sum_col_ref), std::move(zero_const));
 
 	auto filter = make_uniq<LogicalFilter>(std::move(filter_expr));
 	filter->children.push_back(std::move(aggregate));

src/common/ducklake_util.cpp

@@ -16,7 +16,7 @@ namespace duckdb {
 class DuckLakeMetadataManager;
 class FileSystem;
 class TableFilter;
-class DynamicFilter;
+struct DynamicFilterData;
 
 struct ParsedCatalogEntry {
 	string schema;
@@ -36,7 +36,7 @@ class DuckLakeUtil {
 	static ParsedCatalogEntry ParseCatalogEntry(const string &input);
 	static string JoinPath(FileSystem &fs, const string &a, const string &b);
 
-	static DynamicFilter *GetOptionalDynamicFilter(const TableFilter &filter);
+	static shared_ptr<DynamicFilterData> GetOptionalDynamicFilterData(const TableFilter &filter);
 
 	//! Create the data path directory if it does not yet exist
 	static void EnsureDirectoryExists(FileSystem &fs, const string &data_path);

src/functions/ducklake_flush_inlined_data.cpp

@@ -22,6 +22,7 @@
 #include "common/ducklake_encryption.hpp"
 #include "common/ducklake_options.hpp"
 #include "common/index.hpp"
+#include "duckdb/planner/filter/expression_filter.hpp"
 #include "duckdb/planner/table_filter.hpp"
 
 namespace duckdb {
@@ -33,7 +34,6 @@ class DuckLakeTransaction;
 class BoundAtClause;
 class QueryResult;
 class FileSystem;
-class ConstantFilter;
 
 struct SnapshotAndStats;
 struct FlushedInlinedTableInfo;
@@ -62,16 +62,27 @@ struct FilterSQLResult {
 struct ColumnFilterInfo {
 	idx_t column_field_index;
 	LogicalType column_type;
-	unique_ptr<TableFilter> table_filter;
+	unique_ptr<ExpressionFilter> table_filter;
 
-	ColumnFilterInfo(idx_t col_idx, LogicalType type, unique_ptr<TableFilter> filter)
+	ColumnFilterInfo(idx_t col_idx, LogicalType type, unique_ptr<ExpressionFilter> filter)
 	    : column_field_index(col_idx), column_type(std::move(type)), table_filter(std::move(filter)) {
 	}
 
 	ColumnFilterInfo(const ColumnFilterInfo &other)
 	    : column_field_index(other.column_field_index), column_type(other.column_type),
 	      table_filter(other.table_filter->Copy()) {
 	}
+
+	ColumnFilterInfo(ColumnFilterInfo &&other) = default;
+	ColumnFilterInfo &operator=(ColumnFilterInfo &&other) = default;
+	ColumnFilterInfo &operator=(const ColumnFilterInfo &other) {
+		if (this != &other) {
+			column_field_index = other.column_field_index;
+			column_type = other.column_type;
+			table_filter = other.table_filter ? other.table_filter->Copy() : nullptr;
+		}
+		return *this;
+	}
 };
 
 struct FilterPushdownInfo {
@@ -321,16 +332,19 @@ class DuckLakeMetadataManager {
 	virtual FilterSQLResult ConvertFilterPushdownToSQL(const FilterPushdownInfo &filter_info);
 	virtual string GenerateCTESectionFromRequirements(const unordered_map<idx_t, CTERequirement> &requirements,
 	                                                  TableIndex table_id);
-	virtual string GenerateFilterFromTableFilter(const TableFilter &filter, const LogicalType &type,
+	virtual string GenerateFilterFromTableFilter(const ExpressionFilter &filter, const LogicalType &type,
 	                                             unordered_set<string> &referenced_stats);
+	virtual string GenerateFilterFromExpression(const Expression &expr, const LogicalType *type,
+	                                            unordered_set<string> &referenced_stats);
 	virtual bool ValueIsFinite(const Value &val);
 	virtual string CastValueToTarget(const Value &val, const LogicalType &type);
 	virtual string CastStatsToTarget(const string &stats, const LogicalType &type);
-	virtual string GenerateConstantFilter(const ConstantFilter &constant_filter, const LogicalType &type,
+	virtual string GenerateConstantFilter(ExpressionType comparison_type, const Value &constant, const LogicalType &type,
 	                                      unordered_set<string> &referenced_stats);
-	virtual string GenerateConstantFilterDouble(const ConstantFilter &constant_filter, const LogicalType &type,
+	virtual string GenerateConstantFilterDouble(ExpressionType comparison_type, const Value &constant,
+	                                            const LogicalType &type,
 	                                            unordered_set<string> &referenced_stats);
-	virtual string GenerateFilterPushdown(const TableFilter &filter, unordered_set<string> &referenced_stats);
+	virtual string GenerateFilterPushdown(const ExpressionFilter &filter, unordered_set<string> &referenced_stats);
 
 public:
 	//! Read inlined file deletions for regular table scans (no snapshot info per row)

src/include/common/ducklake_util.hpp

@@ -139,8 +139,8 @@ DeleteFileScanResult DuckLakeDeleteFilter::ScanDeletionVectorFile(ClientContext
 unique_ptr<ExpressionFilter> MakeComparisonFilter(ExpressionType comparison_type, Value constant) {
 	auto col_ref = make_uniq<BoundReferenceExpression>(LogicalType::BIGINT, storage_t(0));
 	auto bound_constant = make_uniq<BoundConstantExpression>(std::move(constant));
-	auto comparison = make_uniq<BoundComparisonExpression>(comparison_type, std::move(col_ref),
-			                                                      std::move(bound_constant));
+	auto comparison = BoundComparisonExpression::Create(comparison_type, std::move(col_ref),
+	                                                    std::move(bound_constant));
 	return make_uniq<ExpressionFilter>(std::move(comparison));
 }
 

src/include/storage/ducklake_metadata_manager.hpp

@@ -269,7 +269,7 @@ AsyncResult DuckLakeInlinedDataReader::Scan(ClientContext &context, GlobalTableF
 		if (filters) {
 			for (auto &entry : *filters) {
 				auto &filter = entry.Filter();
-				if (filter.filter_type == TableFilterType::OPTIONAL_FILTER) {
+				if (ExpressionFilter::IsRootOptionalFilter(filter)) {
 					continue;
 				}
 				auto column_id = entry.GetIndex().GetIndex();