Skip to content

Commit 67e7840

Browse files
authored
Fix non-equi join error with equi + 2 inequality conditions (#7642)
* Fix non-equi join error with equi + 2 inequality conditions (#7641) When rightcols contains duplicates (same x column in multiple non-equi conditions), chmatchdup remaps them into the expanded result namespace. This overwrote rightcols, causing downstream code (e.g. .shallow(x, rightcols) in .join_result_key) to reference columns beyond ncol(x). Introduce ansrightcols for the remapped indices and keep rightcols as original column indices into x. Only icolsAns needs the remapped values; all other uses (xcols, names_x[rightcols], .join_result_key) need the original x-relative indices. Closes #7641 * Add contributor credit in DESCRIPTION and NEWS Per reviewer request: thank reporter in NEWS.md and add Tarun Thammisetty as contributor to DESCRIPTION.
1 parent 1bd88cb commit 67e7840

4 files changed

Lines changed: 15 additions & 4 deletions

File tree

DESCRIPTION

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,5 +107,6 @@ Authors@R: c(
107107
person("Vinit", "Thakur", role="ctb"),
108108
person("Mukul", "Kumar", role="ctb"),
109109
person("Ildikó", "Czeller", role="ctb"),
110-
person("Manmita", "Das", role="ctb")
110+
person("Manmita", "Das", role="ctb"),
111+
person("Tarun", "Thammisetty", role="ctb")
111112
)

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838

3939
4. `rowwiseDT()` now provides a helpful error message when a complex object that is not a list (e.g., a function) is provided as a cell value, instructing the user to wrap it in `list()`, [#7219](https://github.com/Rdatatable/data.table/issues/7219). Thanks @kylebutts for the report and @venom1204 for the fix.
4040

41+
5. Non-equi joins combining an equality condition with two inequality conditions on the same column (e.g., `on = .(id == id, val >= lo, val <= hi)`) no longer error, [#7641](https://github.com/Rdatatable/data.table/issues/7641). The internal `chmatchdup` remapping of duplicate `rightcols` was overwriting the original column indices, causing downstream code to reference non-existent columns. Thanks @tarun-t for the report and fix, and @aitap for the diagnosis.
42+
4143
### Notes
4244

4345
1. {data.table} now depends on R 3.5.0 (2018).

R/data.table.R

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,12 +1075,15 @@ replace_dot_alias = function(e) {
10751075
if (length(tt)) jisvars[tt] = paste0("i.",jisvars[tt])
10761076
if (length(duprightcols <- rightcols[duplicated(rightcols)])) {
10771077
nx = c(names_x, names_x[duprightcols])
1078-
rightcols = chmatchdup(names_x[rightcols], nx)
1078+
ansrightcols = chmatchdup(names_x[rightcols], nx) # indices into result namespace nx, #7641
10791079
nx = make.unique(nx)
1080-
} else nx = names_x
1080+
} else {
1081+
nx = names_x
1082+
ansrightcols = rightcols
1083+
}
10811084
ansvars = make.unique(c(nx, jisvars))
10821085
icols = c(leftcols, seq_along(i)[-leftcols])
1083-
icolsAns = c(rightcols, seq.int(length(nx)+1L, length.out=ncol(i)-length(unique(leftcols))))
1086+
icolsAns = c(ansrightcols, seq.int(length(nx)+1L, length.out=ncol(i)-length(unique(leftcols))))
10841087
xcols = xcolsAns = seq_along(x)[-rightcols]
10851088
}
10861089
ansvals = chmatch(ansvars, nx)

inst/tests/tests.Rraw

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6954,6 +6954,11 @@ DT1 <- data.table(a=1L, key='a')
69546954
DT2 <- data.table(a=2.0, key='a')
69556955
test(1483.83, DT1[DT2, roll='nearest'], data.table(a=2L, key='a'))
69566956

6957+
# non-equi join with equi + 2 inequality conditions should not error, #7641
6958+
dt_x = data.table(id = c("A","A","B","B"), val = c(1,5,2,4), key = c("id","val"))
6959+
dt_i = data.table(id = c("A","B"), lo = c(2,1), hi = c(6,3))
6960+
test(1483.91, nrow(dt_x[dt_i, on = .(id == id, val >= lo, val <= hi)]), 2L)
6961+
69576962
# NULL items should be removed when making data.table from list, #842
69586963
# Original fix for #842 added a branch in as.data.table.list() using point()
69596964
# Then PR#3471 moved logic from data.table() into as.data.table.list() and now removes NULL items up front, so longer need for the branch

0 commit comments

Comments
 (0)