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

IndexJoin/JoinInner: store ColId #1166

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Apr 26, 2024

Description of Changes

  1. Store ColId in IndexJoin and JoinInner as opposed to FieldName. This avoids querying Header in build_query.
  2. Store Headers for inner joins and projections. QueryExpr::head now provides the end-result header for the entire query, which is the same header as you'd get from build_query. This means that we can eventually stop making a Header in build_query and thereby move more runtime to query compilation and spend less in query building / evaluation.
  3. Dedup logic in the two build_query functions.

API and ABI breaking changes

None

Expected complexity level and risk

2

@Centril Centril force-pushed the centril/index-join-inner-colid branch from a5cb106 to cac1c01 Compare April 26, 2024 13:53
@Centril Centril force-pushed the centril/index-join-inner-colid branch from cac1c01 to 8d9a0f9 Compare April 26, 2024 16:31
@Centril Centril changed the title IndexJoin/JoinInner: store ColId; incr-join perf -21.419% IndexJoin/JoinInner: store ColId; incr-join perf -20.511% Apr 26, 2024
@Centril Centril force-pushed the centril/index-join-inner-colid branch from 8d9a0f9 to 14e2652 Compare April 26, 2024 16:46
@Centril Centril force-pushed the centril/index-join-inner-colid branch from 14e2652 to 28b0725 Compare April 26, 2024 17:22
@bfops bfops added the release-any To be landed in any release window label Apr 29, 2024
@Centril Centril force-pushed the centril/index-join-inner-colid branch from 28b0725 to 70da3b8 Compare April 30, 2024 20:58
Base automatically changed from centril/simplify-fieldname to master April 30, 2024 21:18
@Centril Centril force-pushed the centril/index-join-inner-colid branch 4 times, most recently from 2280723 to 9e24533 Compare May 7, 2024 17:32
@Centril Centril changed the title IndexJoin/JoinInner: store ColId; incr-join perf -20.511% IndexJoin/JoinInner: store ColId May 7, 2024
@Centril Centril force-pushed the centril/index-join-inner-colid branch 4 times, most recently from 5bc3f77 to 8ca55bf Compare May 13, 2024 09:17
@gefjon
Copy link
Contributor

gefjon commented May 16, 2024

Oh, I see. I missed the fact that the row in question is not within the range.

@gefjon
Copy link
Contributor

gefjon commented May 16, 2024

If we had BTreeIndexIter::<K, V>::skip_to_next_greater_than(&mut self, min_bound: &K) (the leapfrog join operator) we could use multi-column indexes to answer range queries...

@Centril Centril force-pushed the centril/index-join-inner-colid branch from 8ca55bf to a53bb88 Compare May 24, 2024 15:37
@Centril Centril force-pushed the centril/index-join-inner-colid branch from a53bb88 to 72b74e4 Compare May 30, 2024 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants