Skip to content

Fix segfaults when assigning all-NA-vector to factor column#4829

Merged
mattdowle merged 6 commits intomasterfrom
shrektan/fix4824
May 7, 2021
Merged

Fix segfaults when assigning all-NA-vector to factor column#4829
mattdowle merged 6 commits intomasterfrom
shrektan/fix4824

Conversation

@shrektan
Copy link
Copy Markdown
Member

@shrektan shrektan commented Dec 3, 2020

Closes #4824

library(data.table)
dt = data.table(FACTOR = factor(rep("a", 100L))) 
set(dt, i = 1:99, j = "FACTOR", value = rep(NA, 99L)) 
dt
str(dt)

Before the PR, print(dt) leads to an error that Error in as.character.factor(x) : malformed factor. By inspecting the value of dt$FACTOR, you should find that some of the values are garbaged.

This PR fixes the issue.

@shrektan shrektan requested a review from mattdowle December 3, 2020 18:01
@shrektan shrektan changed the title Fix assigning an all-NA-vector to a factor column Fix segfaults due to assigning all-NA-vector to factor column Dec 3, 2020
@shrektan shrektan changed the title Fix segfaults due to assigning all-NA-vector to factor column Fix segfaults when assigning all-NA-vector to factor column Dec 3, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 3, 2020

Codecov Report

Merging #4829 (3d7f52c) into master (1f0d00e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4829   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          73       73           
  Lines       14465    14465           
=======================================
  Hits        14382    14382           
  Misses         83       83           
Impacted Files Coverage Δ
src/assign.c 99.70% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f0d00e...3d7f52c. Read the comment docs.

@mattdowle mattdowle added this to the 1.14.1 milestone May 7, 2021
Copy link
Copy Markdown
Member

@mattdowle mattdowle left a comment

Choose a reason for hiding this comment

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

Great fix! Thanks for chasing it down.

@mattdowle mattdowle merged commit be22eff into master May 7, 2021
@mattdowle mattdowle deleted the shrektan/fix4824 branch May 7, 2021 08:08
@jangorecki jangorecki modified the milestones: 1.14.9, 1.15.0 Oct 29, 2023
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.

set function crashes the R session

3 participants