Skip to content

[SQL] Remove MapIndex operators after tables with primary keys#5887

Merged
mihaibudiu merged 1 commit intomainfrom
issue5842
Mar 27, 2026
Merged

[SQL] Remove MapIndex operators after tables with primary keys#5887
mihaibudiu merged 1 commit intomainfrom
issue5842

Conversation

@mihaibudiu
Copy link
Copy Markdown
Contributor

The initial intent of this PR was to allow sharing of indexes between the table integral and any other joins that may use the table. This does not seem to work actually, since DBSP cannot figure out from the graph supplied that the integrals are shareable.

But this optimization is still useful, since it will allow multiple joins of an input table that join on the primary key to share the same index.

I have refactored the previous code to share indexes by moving all classes to a new package and moving many inner classes around. That's why the PR looks bigger.

Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

LGTM. Clean decomposition of the old monolith into a focused package. The structural circuit assertions in IncrementalRegression2Tests are exactly the right way to test this kind of optimization.

@ryzhyk
Copy link
Copy Markdown
Contributor

ryzhyk commented Mar 27, 2026

@mihaibudiu , can you give an example of what this PR does?

@ryzhyk
Copy link
Copy Markdown
Contributor

ryzhyk commented Mar 27, 2026

I can see that there is a MapIndex attached to every input_map today. What does it do and why is it no longer needed?

@mihaibudiu
Copy link
Copy Markdown
Contributor Author

There are many possible reasons for that MapIndex: to remove unused fields, or to prepare the index for a subsequent operator. Now we can reuse directly the input index for a subsequent join.

@mihaibudiu
Copy link
Copy Markdown
Contributor Author

Before:
image
After:
image

@mihaibudiu
Copy link
Copy Markdown
Contributor Author

Before:
image
After:
image

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
@mihaibudiu
Copy link
Copy Markdown
Contributor Author

I will check whether the bug with indexes is not present here as well and put in the queue if it works.

@mihaibudiu mihaibudiu added this pull request to the merge queue Mar 27, 2026
Merged via the queue into main with commit acc0376 Mar 27, 2026
1 check passed
@mihaibudiu mihaibudiu deleted the issue5842 branch March 27, 2026 23:09
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