Skip to content

Conversation

Damon07
Copy link
Contributor

@Damon07 Damon07 commented Feb 7, 2025

This PR fixes #16104.

@Tishj
Copy link
Contributor

Tishj commented Feb 7, 2025

These seem to be iterated over here:

TableFilterSet FilterCombiner::GenerateTableScanFilters(const vector<ColumnIndex> &column_ids) {
	TableFilterSet table_filters;
	//! First, we figure the filters that have constant expressions that we can push down to the table scan
	for (auto &constant_value : constant_values) {
		if (!constant_value.second.empty()) {

and here:

void FilterCombiner::GenerateFilters(const std::function<void(unique_ptr<Expression> filter)> &callback) {
	// first loop over the remaining filters
	for (auto &filter : remaining_filters) {
		callback(std::move(filter));
	}
	remaining_filters.clear();
	// now loop over the equivalence sets
	for (auto &entry : equivalence_map) {

iterating over an unordered map is indeed not going to result in a deterministic order, the change sounds fine to me, but maybe someone with better ownership of this code can weigh in

@Mytherin Mytherin changed the base branch from main to v1.2-histrionicus February 17, 2025 16:28
@Mytherin Mytherin changed the base branch from v1.2-histrionicus to main February 17, 2025 16:29
@Mytherin
Copy link
Collaborator

Mytherin commented Feb 17, 2025

Thanks for the PR! The changeset looks good to me - could you just rebase it to the v1.2-histrionicus branch so we can add this to v1.2.1?

@Damon07 Damon07 changed the base branch from main to v1.2-histrionicus February 18, 2025 02:42
@Damon07
Copy link
Contributor Author

Damon07 commented Feb 18, 2025

Done.

@Mytherin Mytherin merged commit c19a7fe into duckdb:v1.2-histrionicus Feb 18, 2025
@Mytherin
Copy link
Collaborator

Thanks!

krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Mar 7, 2025
Use ordered map to preserve expressions order (duckdb/duckdb#16111)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter with an error-throwing function getting incorrectly reordered
3 participants