fix!: NA unilateral = difference sur les colonnes d'egalite du chemin local

NEWS.md

@@ -40,6 +40,19 @@
 
 ## Breaking changes
 
+* The local (data.frame) path now applies the same NA semantics as the lazy
+  path and the numeric tolerance kernel to **equality** columns: a one-sided
+  NA (a value facing a missing value, including candidate rows with no
+  reference match after the key join) is always a difference; a two-sided NA
+  follows `na_equal`. Previously, with `na_equal = TRUE` (the default), any NA
+  on either side passed on the local path (`na_pass` semantics), while the
+  same data failed on the lazy path: identical datasets could get opposite
+  verdicts depending on the backend. Comparisons that relied on one-sided NA
+  passing locally will now fail; the two-sided NA behavior is unchanged.
+  On the local failure path, `pointblank::get_data_extracts()` output for a
+  failing equality column now also carries its `<col>__eq` boolean column
+  (as the lazy path always did) (issue #14).
+
 * `compare_datasets_from_yaml()` now raises an explicit error when one or more
   key columns are absent from either dataset, naming the missing column(s) and
   the dataset(s) concerned, on every code path (with or without an explicit

R/compare_datasets_from_yaml.R

@@ -700,6 +700,18 @@ compare_datasets_from_yaml <- function(data_reference,
       tbl = cmp_for_agent, tol_cols = tol_cols, eq_cols = eq_cols,
       ref_suffix = ref_suffix, na_equal = na_equal
     )
+    # Local path: materialise the __eq boolean for the failing equality
+    # columns so the pointblank step validates the exact boolean the verdict
+    # used (one-sided NA fails, two-sided NA follows na_equal). The lazy path
+    # already carries __eq columns; col_vals_equal(na_pass = ...) alone cannot
+    # express these semantics.
+    if (!is_lazy && length(fail$eq) > 0) {
+      for (c in fail$eq) {
+        cmp_for_agent[[paste0(c, "__eq")]] <- eq_col_bool(
+          cmp_for_agent, col = c, ref_suffix = ref_suffix, na_equal = na_equal
+        )
+      }
+    }
     agent <- setup_pointblank_agent(
       cmp_for_agent,
       cols_reference,

R/fast_path.R

@@ -1,11 +1,12 @@
 # Per-column pass predicates, shared by the all-pass short-circuit and the
-# failing-column selection. They reproduce pointblank's verdict exactly:
+# failing-column selection.
 #   - a tolerance column passes iff every <col>__ok value is TRUE (NA counts as
 #     a failure, matching col_vals_equal(..., na_pass = FALSE));
-#   - an equality column passes iff its pre-computed <col>__eq column is all TRUE
-#     (lazy path) or, when no __eq exists (local path), the raw comparison holds
-#     under the same na_pass = na_equal semantics pointblank uses (any NA on
-#     either side passes iff na_equal).
+#   - an equality column passes iff its pre-computed <col>__eq column is all
+#     TRUE (lazy path) or, when no __eq exists (local path), the raw comparison
+#     holds under the shared NA semantics: a one-sided NA is always a
+#     difference, a two-sided NA follows na_equal. This matches both the
+#     numeric tolerance kernel and the lazy SQL CASE WHEN.
 
 # Per-row boolean outcome of a tolerance column: the pre-computed <col>__ok
 # vector. NA (never produced by construction) would count as a failure, matching
@@ -15,18 +16,24 @@ tol_col_bool <- function(tbl, col) {
 }
 
 # Per-row boolean outcome of an equality column, resolved to TRUE/FALSE (no NA):
-# the pre-computed <col>__eq vector (lazy path) or, when absent (local path), the
-# raw comparison under the same na_pass = na_equal semantics pointblank uses
-# (any NA on either side passes iff na_equal).
+# the pre-computed <col>__eq vector (lazy path) or, when absent (local path),
+# the raw comparison with one-sided NA = FALSE and two-sided NA = na_equal.
 eq_col_bool <- function(tbl, col, ref_suffix, na_equal) {
   eq_precomputed <- tbl[[paste0(col, "__eq")]]
   if (!is.null(eq_precomputed)) {
     return(eq_precomputed)
   }
   cand_vals <- tbl[[col]]
   ref_vals  <- tbl[[paste0(col, ref_suffix)]]
+  cand_na   <- is.na(cand_vals)
+  ref_na    <- is.na(ref_vals)
   cmp_res   <- cand_vals == ref_vals
-  ifelse(is.na(cmp_res), na_equal, cmp_res)
+  # Both values present and equal; any NA resolves below
+  out <- !is.na(cmp_res) & cmp_res
+  # Two-sided NA follows na_equal; a one-sided NA stays FALSE (a value facing
+  # a missing value is a difference)
+  out[cand_na & ref_na] <- na_equal
+  out
 }
 
 tol_col_passes <- function(tbl, col) {

tests/testthat/test-equivalence-guard.R

@@ -1,10 +1,11 @@
-# Equivalence guard for the speed optimization of compare_datasets_from_yaml.
+# Equivalence guard for compare_datasets_from_yaml.
 #
 # These tests pin BOTH the verdict (all_passed) AND the exact set of failing
 # cells (as recovered through pointblank::get_data_extracts, the way the
-# enc.mco oracle consumes the result). They must stay green before and after
-# the optimization: the optimized code may change HOW the verdict is produced,
-# never WHAT it produces.
+# enc.mco oracle consumes the result). Optimizations may change HOW the
+# verdict is produced, never WHAT it produces; local and lazy backends must
+# agree. A documented semantic fix (with a NEWS "Breaking changes" entry) MAY
+# re-pin a verdict here, deliberately and never silently.
 
 KEY <- c("id", ".row")
 
@@ -101,31 +102,36 @@ test_that("multiple failing cells across columns are all surfaced", {
   expect_equal(out$cells, c("a@3|1", "n@5|1", "txt@2|1"))
 })
 
-test_that("character NA (any side) passes under na_equal = TRUE", {
-  for (mutate in list(
-    function(r, c) { r$txt[2] <- NA; c$txt[2] <- NA; list(r, c) },
-    function(r, c) { c$txt[2] <- NA; list(r, c) },
-    function(r, c) { r$txt[2] <- NA; list(r, c) }
-  )) {
-    ref <- make_ref(); cand <- ref
-    m <- mutate(ref, cand); ref <- m[[1]]; cand <- m[[2]]
-    out <- run_compare(ref, cand, key = KEY, na_equal_default = TRUE)
-    expect_true(out$all_passed)
-    expect_equal(out$cells, character(0))
-  }
-})
+# NA semantics for equality columns (aligned with the tolerance kernel and the
+# lazy SQL path): a two-sided NA follows na_equal, a one-sided NA is always a
+# difference.
 
-test_that("character NA (any side) fails under na_equal = FALSE", {
-  for (mutate in list(
-    function(r, c) { r$txt[2] <- NA; c$txt[2] <- NA; list(r, c) },
-    function(r, c) { c$txt[2] <- NA; list(r, c) },
-    function(r, c) { r$txt[2] <- NA; list(r, c) }
-  )) {
-    ref <- make_ref(); cand <- ref
-    m <- mutate(ref, cand); ref <- m[[1]]; cand <- m[[2]]
-    out <- run_compare(ref, cand, key = KEY, na_equal_default = FALSE)
-    expect_false(out$all_passed)
-    expect_equal(out$cells, "txt@2|1")
+test_that("character NA on both sides follows na_equal", {
+  ref <- make_ref(); cand <- ref
+  ref$txt[2]  <- NA
+  cand$txt[2] <- NA
+
+  out_true <- run_compare(ref, cand, key = KEY, na_equal_default = TRUE)
+  expect_true(out_true$all_passed)
+  expect_equal(out_true$cells, character(0))
+
+  out_false <- run_compare(ref, cand, key = KEY, na_equal_default = FALSE)
+  expect_false(out_false$all_passed)
+  expect_equal(out_false$cells, "txt@2|1")
+})
+
+test_that("character NA on one side only fails regardless of na_equal", {
+  for (na_equal in c(TRUE, FALSE)) {
+    for (mutate in list(
+      function(r, c) { c$txt[2] <- NA; list(r, c) },
+      function(r, c) { r$txt[2] <- NA; list(r, c) }
+    )) {
+      ref <- make_ref(); cand <- ref
+      m <- mutate(ref, cand); ref <- m[[1]]; cand <- m[[2]]
+      out <- run_compare(ref, cand, key = KEY, na_equal_default = na_equal)
+      expect_false(out$all_passed)
+      expect_equal(out$cells, "txt@2|1")
+    }
   }
 })
 

tests/testthat/test-na-equality-semantics.R

@@ -0,0 +1,82 @@
+# NA semantics for equality (non-tolerance) columns, aligned across the local
+# and lazy paths: a one-sided NA is always a difference; a two-sided NA follows
+# na_equal. Same semantics as the numeric tolerance kernel.
+
+compare_both_paths <- function(ref, cand, key, na_equal) {
+  yaml_path <- tempfile(fileext = ".yaml")
+  on.exit(unlink(yaml_path), add = TRUE)
+  writeLines(sprintf('
+version: 1
+defaults:
+  keys: [%s]
+  na_equal: %s
+row_validation:
+  check_count: no
+', key, if (na_equal) "yes" else "no"), con = yaml_path)
+
+  res_local <- suppressWarnings(suppressMessages(
+    compare_datasets_from_yaml(ref, cand, key = key, path = yaml_path)
+  ))
+
+  con <- DBI::dbConnect(duckdb::duckdb())
+  on.exit(DBI::dbDisconnect(con, shutdown = TRUE), add = TRUE)
+  duckdb::dbWriteTable(con, "ref_na",  ref)
+  duckdb::dbWriteTable(con, "cand_na", cand)
+  res_lazy <- suppressWarnings(suppressMessages(
+    compare_datasets_from_yaml(
+      dplyr::tbl(con, "ref_na"), dplyr::tbl(con, "cand_na"),
+      key = key, path = yaml_path
+    )
+  ))
+  list(local = res_local$all_passed, lazy = res_lazy$all_passed)
+}
+
+test_that("one-sided NA on an equality column fails identically on both paths", {
+  skip_if_not_installed("duckdb")
+  skip_if_not_installed("dbplyr")
+  ref  <- data.frame(id = 1:2, s = c("a", NA), stringsAsFactors = FALSE)
+  cand <- data.frame(id = 1:2, s = c("a", "b"), stringsAsFactors = FALSE)
+
+  for (na_equal in c(TRUE, FALSE)) {
+    out <- compare_both_paths(ref, cand, key = "id", na_equal = na_equal)
+    expect_false(out$local)
+    expect_identical(out$local, out$lazy)
+  }
+
+  # Mirror case: NA on the candidate side
+  for (na_equal in c(TRUE, FALSE)) {
+    out <- compare_both_paths(cand, ref, key = "id", na_equal = na_equal)
+    expect_false(out$local)
+    expect_identical(out$local, out$lazy)
+  }
+})
+
+test_that("two-sided NA on an equality column follows na_equal on both paths", {
+  skip_if_not_installed("duckdb")
+  skip_if_not_installed("dbplyr")
+  ref  <- data.frame(id = 1:2, s = c("a", NA), stringsAsFactors = FALSE)
+  cand <- data.frame(id = 1:2, s = c("a", NA), stringsAsFactors = FALSE)
+
+  out_true <- compare_both_paths(ref, cand, key = "id", na_equal = TRUE)
+  expect_true(out_true$local)
+  expect_identical(out_true$local, out_true$lazy)
+
+  out_false <- compare_both_paths(ref, cand, key = "id", na_equal = FALSE)
+  expect_false(out_false$local)
+  expect_identical(out_false$local, out_false$lazy)
+})
+
+test_that("a candidate row with no reference match fails on both paths", {
+  skip_if_not_installed("duckdb")
+  skip_if_not_installed("dbplyr")
+  ref  <- data.frame(id = 1:2, s = c("a", "b"), stringsAsFactors = FALSE)
+  cand <- data.frame(id = 1:3, s = c("a", "b", "c"), stringsAsFactors = FALSE)
+
+  # The left join fills the reference side with NA: one-sided NA, hence a
+  # failure, even under na_equal = TRUE
+  for (na_equal in c(TRUE, FALSE)) {
+    out <- compare_both_paths(ref, cand, key = "id", na_equal = na_equal)
+    expect_false(out$local)
+    expect_identical(out$local, out$lazy)
+  }
+})