Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Oct 29, 2025

Which issue does this PR close?

Rationale for this change

While reviewing #18152 I think I found an issue in the example.

What changes are included in this PR?

Fix example

While i had it checked out, I also added a few OCD ascii touchups

Are these changes tested?

Are there any user-facing changes?

No, this is an update to an internal diagram. There is no behavior change

@github-actions github-actions bot added the physical-expr Changes to the physical-expr crates label Oct 29, 2025
/// └───────────┘ └─────────┘ └─────────┘
/// │└─────────┘│ │ 2 │ │ C │
/// │┌─────────┐│ ├─────────┤ ├─────────┤
/// ││ D ││ │ 2 │ │ C │
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fix is that the last entry of result should be C - not D because the last entry in indices is also 2

Copy link
Contributor

@pepijnve pepijnve Oct 29, 2025

Choose a reason for hiding this comment

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

@alamb I hate to be the 'well actually' guy. The diagram was actually correct. An index n value means 'take a value from array n'. So the first 2 takes the first element from array 2 which is C. The second 2 takes the second element which is D.

The input arrays in the example were the vecs ["A"], ["B"], ["C", "D"].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the clarification!

@alamb alamb marked this pull request as ready for review October 29, 2025 19:44
@alamb alamb added the documentation Improvements or additions to documentation label Oct 29, 2025
@alamb alamb closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants