Skip to content

Review existing usages of the inverse method of ColIndexMapping #8600

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.

Sign up for GitHub

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

Jump to bottom
Closed
chenzl25 opened this issue Mar 16, 2023 · 2 comments · Fixed by #9252
Closed

Review existing usages of the inverse method of ColIndexMapping #8600

chenzl25 opened this issue Mar 16, 2023 · 2 comments · Fixed by #9252
Assignees
@chenzl25
Milestone
release-0.19

Comments

@chenzl25
Copy link
Contributor

chenzl25 commented Mar 16, 2023 •
edited
Loading

The inverse() method of ColIndexMapping seems to have the potential to cause bugs If the map is not invertible. It is better to remove this method if possible.
@github-actions github-actions bot added this to the release-0.1.18 milestone Mar 16, 2023
@BugenZhao
Copy link
Member

I guess we can first make inverse return an Option and replace all current occurrences with .unwrap() to let misuses expose earlier. 🤔

@xxchan
Copy link
Member

xxchan commented Mar 16, 2023

Example of bug: #8598

@fuyufjh fuyufjh modified the milestones: release-0.18, release-0.19 Mar 22, 2023
@chenzl25 chenzl25 self-assigned this Apr 4, 2023
@chenzl25 chenzl25 mentioned this issue Apr 18, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chenzl25 chenzl25

Labels
None yet
Projects
None yet
Milestone
release-0.19
Development

Successfully merging a pull request may close this issue.

refactor(optimizer): refactor the column index mapping's inverse method risingwavelabs/risingwave
4 participants
@chenzl25 @fuyufjh @BugenZhao @xxchan