Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(1231): Basic query cardinality estimation #1273

Merged
merged 7 commits into from
May 23, 2024

Conversation

joshua-spacetime
Copy link
Collaborator

@joshua-spacetime joshua-spacetime commented May 21, 2024

Description of Changes

This patch implements basic cardinality estimation for QueryExpr. It utilizes table cardinalities and number of distinct values for index related operators.

API and ABI breaking changes

None

Expected complexity level and risk

1

Testing

Tests are added.

@Centril Centril force-pushed the joshua/feat/1231/cardinality-estimation branch from 1cf2ea2 to 401a7a5 Compare May 22, 2024 09:25
@Centril
Copy link
Contributor

Centril commented May 22, 2024

@joshua-spacetime Since you are rather busy, I took the liberty of applying my suggestions myself. They simplified things, especially the tests, quite a bit. If you are as well, I am happy to merge the PR now, and the range stuff we can defer to a different PR (although feel free to comment on that,...).

use crate::db::relational_db::Tx;
use spacetimedb_primitives::{ColList, TableId};
use spacetimedb_vm::expr::{Query, QueryExpr, SourceExpr};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this is written is to me not estimated number of rows but MAXIMUM estimated number of rows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true, although the plan is that this will become more accurate over time as we add more stats.

@Centril Centril force-pushed the joshua/feat/1231/cardinality-estimation branch from 36b29e9 to 7b3509a Compare May 23, 2024 08:31
@Centril Centril enabled auto-merge May 23, 2024 08:32
@Centril Centril added this pull request to the merge queue May 23, 2024
Merged via the queue into master with commit 88a8ada May 23, 2024
6 checks passed
@joshua-spacetime joshua-spacetime deleted the joshua/feat/1231/cardinality-estimation branch May 23, 2024 15:52
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