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

bug:construct table could panic when there is duplicated column in the output columns #15074

Closed
st1page opened this issue Feb 9, 2024 · 3 comments · Fixed by #15338
Closed
Assignees
Labels
type/bug Something isn't working
Milestone

Comments

@st1page
Copy link
Contributor

st1page commented Feb 9, 2024

pub fn find_columns_by_ids(
table_columns: &[ColumnDesc],
column_ids: &[ColumnId],
) -> (Vec<ColumnDesc>, Vec<usize>) {
let mut table_columns = table_columns
.iter()
.enumerate()
.map(|(index, c)| (c.column_id, (c.clone(), index)))
.collect::<HashMap<_, _>>();
column_ids
.iter()
.map(|id| table_columns.remove(id).unwrap())
.unzip()
}

When there are multiple duplicated ids in the column_ids, the unwrap could panic. In most cases, the optimizer could ensure there are no duplicated columns but in some corner case, the optimizer does not optimize it, and the CN panic. I think the execution should handle this case, whether the optimizer does that optimization.

@github-actions github-actions bot added this to the release-1.7 milestone Feb 9, 2024
@liurenjie1024 liurenjie1024 changed the title construct table could panic when there is duplicated column in the output columns bug construct table could panic when there is duplicated column in the output columns Feb 9, 2024
@liurenjie1024 liurenjie1024 changed the title bug construct table could panic when there is duplicated column in the output columns bug:construct table could panic when there is duplicated column in the output columns Feb 9, 2024
@liurenjie1024 liurenjie1024 added the type/bug Something isn't working label Feb 9, 2024
@fuyufjh fuyufjh self-assigned this Feb 28, 2024
@fuyufjh
Copy link
Member

fuyufjh commented Feb 28, 2024

When there are multiple duplicated ids in the column_ids, the unwrap could panic

Just read the code of HashMap and I think the panic is because none of these columns have the given column_id, not caused by duplication. Let me try to reproduce it and inspect these variables.

Sorry I misunderstood. There are duplications in column_ids rather than table_columns

@fuyufjh

This comment was marked as outdated.

@fuyufjh

This comment was marked as outdated.

@fuyufjh fuyufjh closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2024
@fuyufjh fuyufjh reopened this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants