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

Add Table.cross_join and Table.zip to In-Memory Table #4063

Merged
merged 16 commits into from
Jan 23, 2023

Conversation

radeusgd
Copy link
Member

Pull Request Description

Implements https://www.pivotaltracker.com/story/show/184239059

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@radeusgd radeusgd self-assigned this Jan 18, 2023
@radeusgd radeusgd requested a review from jdunkerley as a code owner January 18, 2023 08:14
@radeusgd radeusgd force-pushed the wip/radeusgd/table-cross-join-184239059 branch from c980026 to 02571b9 Compare January 18, 2023 16:45
@radeusgd radeusgd marked this pull request as draft January 18, 2023 16:46
@radeusgd radeusgd changed the title Add Table.cross_join to In-Memory Table Add Table.cross_join and Table.zip to In-Memory Table Jan 18, 2023
@radeusgd radeusgd force-pushed the wip/radeusgd/table-cross-join-184239059 branch from 02571b9 to 5ebec5e Compare January 19, 2023 15:36
@radeusgd radeusgd marked this pull request as ready for review January 19, 2023 15:36
@radeusgd radeusgd force-pushed the wip/radeusgd/table-cross-join-184239059 branch 4 times, most recently from e9ac947 to 2751525 Compare January 20, 2023 14:29
for (int i = 0; i < this.rowCount(); i++) {
if (!matchedLeftRows.contains(i)) {
leftRows.add(i);
rightRows.add(Index.NOT_FOUND);
leftUnmatchedBuilder.addRow(i, Index.NOT_FOUND);
Copy link
Member

Choose a reason for hiding this comment

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

could we not just add to resultsToKeep?

Copy link
Member Author

Choose a reason for hiding this comment

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

resultsToKeep is an aggregator for JoinResults that later will be merged.

The idea is that the JoinResult.Builder is building an array of primitives (to avoid unnecessary boxing of integers).

I guess I could rewrite this to use a single JoinResult.Builder for the three sub-operations, if you think it is worth it.

for (int i = 0; i < right.rowCount(); i++) {
if (!matchedRightRows.contains(i)) {
leftRows.add(Index.NOT_FOUND);
rightRows.add(i);
rightUnmatchedBuilder.addRow(Index.NOT_FOUND, i);
Copy link
Member

Choose a reason for hiding this comment

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

as above

@radeusgd radeusgd force-pushed the wip/radeusgd/table-cross-join-184239059 branch from 2751525 to a31b8c6 Compare January 23, 2023 12:33
@radeusgd radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Jan 23, 2023
@mergify mergify bot merged commit d2e57ed into develop Jan 23, 2023
@mergify mergify bot deleted the wip/radeusgd/table-cross-join-184239059 branch January 23, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants