Update development

NEWS.md

@@ -1,5 +1,7 @@
 # collapse 2.1.7
 
+* Fixed a bug in `fmatch()` (and thus `%in%`/`%!in%`/`%iin%`/`%!iin%` and joins) where a logical `NA` in `x` could spuriously match a non-`NA` value in `table` (e.g. `2L`) when `table` was not itself logical. Thanks @LJ-Jenkins for reporting (#870).
+
 * Fixed a bug in `fslice()` (grouped, `n = 1`, `with.ties = FALSE`) that caused R to crash with a fatal error when a group had only missing values in `order.by`. Thanks @chihyunkim for reporting (#867).
 
 * The *collapse* article is now published in the Journal of Statistical Software: https://doi.org/10.18637/jss.v116.i01. This article is now the primary citation for academic use of *collapse*. It is also a great reference to quickly and thoroughly understand the package. `citation("collapse")` was also updated in this regard. The APA-style citation is:

src/match.c

@@ -152,7 +152,8 @@ SEXP match_single(SEXP x, SEXP table, SEXP nomatch) {
     } else goto bigint;
     anyNA = !(inherits(x, "na.included") && inherits(table, "na.included"));
   } else if (tx == LGLSXP) {
-    M = 3;
+    if(TYPEOF(table) == LGLSXP) M = 3;
+    else { tx = INTSXP; goto bigint; } // table is not logical: cannot assume values are restricted to {0, 1, NA}, need generic hashing to avoid spurious NA collisions (#870)
   } else error("Type %s is not supported.", type2char(tx));
 
   int *restrict h = (int*)R_Calloc(M, int); // Table to save the hash values, table has size M

tests/testthat/test-fmatch.R

@@ -178,3 +178,28 @@ test_that("fmatch works with logical data", {
   expect_equal(fmatch(FALSE, x), 2L)
 })
 
+test_that("fmatch does not match logical NA in x to non-logical values in table (#870)", {
+  table_int <- c(0L, 1L, 2L)
+  expect_identical(fmatch(c(FALSE, TRUE, NA), table_int), match(c(FALSE, TRUE, NA), table_int))
+  expect_identical(fmatch(NA, table_int), NA_integer_)
+  expect_identical(NA %iin% table_int, integer(0))
+
+  # Larger table where the colliding value is at a different position
+  table_int2 <- c(5L, 2L, 0L, 1L, 9L)
+  expect_identical(fmatch(c(FALSE, TRUE, NA), table_int2), match(c(FALSE, TRUE, NA), table_int2))
+
+  # table actually containing NA_integer_: NA in x should match it
+  table_int3 <- c(0L, 1L, NA_integer_)
+  expect_identical(fmatch(c(FALSE, TRUE, NA), table_int3), match(c(FALSE, TRUE, NA), table_int3))
+
+  # Reverse direction (already worked prior to the fix)
+  expect_identical(fmatch(table_int, c(FALSE, TRUE, NA)), match(table_int, c(FALSE, TRUE, NA)))
+
+  # table is double
+  table_dbl <- c(0, 1, 2)
+  expect_identical(fmatch(c(FALSE, TRUE, NA), table_dbl), match(c(FALSE, TRUE, NA), table_dbl))
+
+  # genuine logical-vs-logical matching still works, including NA matching NA
+  expect_identical(fmatch(c(FALSE, TRUE, NA), c(TRUE, NA, FALSE)), match(c(FALSE, TRUE, NA), c(TRUE, NA, FALSE)))
+})
+