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.

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

Fixing crash when attempting to join on character(0) #4272

Merged
merged 11 commits into from
Jun 8, 2020
Merged

Fixing crash when attempting to join on character(0) #4272

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d0fd094
Fixed crash when attempting to join on character(0)
tlapak Mar 1, 2020
2e185f6
Minor grammar fix in error message
tlapak Mar 1, 2020
a3e0e62
Atted test for crash when attempting to join on character(0)
tlapak Mar 1, 2020
e85e836
Added news entry
tlapak Mar 1, 2020
3e5ae0b
Reflect grammar change to error message in tests
tlapak Mar 1, 2020
bdfb694
Added link to news file linking the PR
tlapak Mar 2, 2020
cba5c55
Add check in bmerge.c
tlapak May 25, 2020
008e472
Add tests for check in bmerge.c
tlapak May 25, 2020
153f712
Update to pass 2126.*
tlapak Jun 6, 2020
04cedfd
Expand tests 2126
tlapak Jun 6, 2020
91ec306
Merge branch 'master' into fix_join_crash
tlapak Jun 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16639,6 +16639,9 @@ options(old_width)
DT = data.table(A="a", key="A")
test(2126.1, DT[J(NULL)], DT[0])
test(2126.2, DT[data.table()], DT[0])
# additional segfault when i is NULL and roll = 'nearest'
test(2126.3, DT[J(NULL), roll = 'nearest'], DT[0])
test(2126.4, DT[data.table(), roll = 'nearest'], DT[0])

# fcase, #3823
test_vec1 = -5L:5L < 0L
Expand Down Expand Up @@ -16853,3 +16856,5 @@ B = data.table(B='b')
test(2139.1, A[B, on=character(0)], error = "'on' argument should be a named atomic vector")
test(2139.2, merge(A, B, by=character(0) ), error = "non-empty vector of column names for `by` is required.")
test(2139.3, merge(A, B, by.x=character(0), by.y=character(0)), error = "non-empty vector of column names is required")
# Also shouldn't crash when using internal functions
test(2139.4, bmerge(A, B, integer(), integer(), 0, c(FALSE, TRUE), NA, 'all', integer(), FALSE), error = 'icols and xcols must be non-empty')
4 changes: 3 additions & 1 deletion src/bmerge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE
i = iArg; x = xArg; // set globals so bmerge_r can see them.
if (!isInteger(icolsArg)) error(_("Internal error: icols is not integer vector")); // # nocov
if (!isInteger(xcolsArg)) error(_("Internal error: xcols is not integer vector")); // # nocov
if ((LENGTH(icolsArg) == 0 || LENGTH(xcolsArg) == 0) && LENGTH(i) > 0) // We let through LENGTH(i) == 0 for tests 2126.*
error(_("Internal error: icols and xcols must be non-empty integer vectors."));
if (LENGTH(icolsArg) > LENGTH(xcolsArg)) error(_("Internal error: length(icols) [%d] > length(xcols) [%d]"), LENGTH(icolsArg), LENGTH(xcolsArg)); // # nocov
icols = INTEGER(icolsArg);
xcols = INTEGER(xcolsArg);
Expand All @@ -68,7 +70,7 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE
roll = 0.0; rollToNearest = FALSE;
if (isString(rollarg)) {
if (strcmp(CHAR(STRING_ELT(rollarg,0)),"nearest") != 0) error(_("roll is character but not 'nearest'"));
if (TYPEOF(VECTOR_ELT(i, icols[ncol-1]-1))==STRSXP) error(_("roll='nearest' can't be applied to a character column, yet."));
if (ncol > 0 && TYPEOF(VECTOR_ELT(i, icols[ncol-1]-1))==STRSXP) error(_("roll='nearest' can't be applied to a character column, yet."));
jangorecki marked this conversation as resolved.
Show resolved Hide resolved
roll=1.0; rollToNearest=TRUE; // the 1.0 here is just any non-0.0, so roll!=0.0 can be used later
} else {
if (!isReal(rollarg)) error(_("Internal error: roll is not character or double")); // # nocov
Expand Down