Skip to content

Conversation

jeewonhh
Copy link
Contributor

@jeewonhh jeewonhh commented Sep 8, 2024

No description provided.

@duckdb-draftbot duckdb-draftbot marked this pull request as draft September 8, 2024 23:45
@jeewonhh jeewonhh marked this pull request as ready for review September 8, 2024 23:45
@jeewonhh jeewonhh marked this pull request as draft September 9, 2024 13:10
@Mytherin Mytherin marked this pull request as ready for review September 9, 2024 21:11
@Mytherin Mytherin merged commit 95038c5 into duckdb:main Sep 10, 2024
81 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@Tishj
Copy link
Contributor

Tishj commented Sep 10, 2024

It would also be great if you could overload/update the ParamsToString of these LogicalOperators to show these estimated cardinalities in the EXPLAIN output

For example, LogicalAggregate does this:

InsertionOrderPreservingMap<string> LogicalAggregate::ParamsToString() const {
	InsertionOrderPreservingMap<string> result;
	string groups_info;
	for (idx_t i = 0; i < groups.size(); i++) {
		if (i > 0) {
			groups_info += "\n";
		}
		groups_info += groups[i]->GetName();
	}
	result["Groups"] = groups_info;

	string expressions_info;
	for (idx_t i = 0; i < expressions.size(); i++) {
		if (i > 0) {
			expressions_info += "\n";
		}
		expressions_info += expressions[i]->GetName();
	}
	result["Expressions"] = expressions_info;
	SetParamsEstimatedCardinality(result);
	return result;
}

@Mytherin
Copy link
Collaborator

I think that's done in the default implementation in LogicalOperator::ParamsToString already

Mytherin added a commit that referenced this pull request Sep 16, 2024
Setting cross-product cardinality in the constructor strangely
introduces a new filter operator in the query plan (with cardinality 0).
Subsequent operators will adopt 0 as their cardinality, which is
incorrect.

Instead, only set the cardinality "safely" inside the query graph
manager

This (partially) reverts
[PR](#13818).
github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Sep 25, 2024
Merge pull request duckdb/duckdb#13818 from jeewonhh/fix-card-crossproduct
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Sep 25, 2024
Merge pull request duckdb/duckdb#13818 from jeewonhh/fix-card-crossproduct

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
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.

3 participants