diff --git a/R/views.R b/R/views.R index 5d1c4dc..0433ebc 100644 --- a/R/views.R +++ b/R/views.R @@ -122,9 +122,25 @@ #' fallback -- correct for the local temp corpora the direct-execution tests #' build. #' @noRd +#' `fixed = TRUE` is load-bearing, not a style choice. +#' +#' In regex mode, `gsub()` interprets backslashes in the REPLACEMENT string as +#' escape sequences and silently drops them. A Windows corpus path is full of +#' them, so `C:\Users\RUNNER\AppData\...` was substituted in as +#' `C:UsersRUNNERAppData...` and every DuckDB read failed with "No files found +#' that match the pattern". `fixed = TRUE` treats pattern and replacement as +#' literal text, which is what a filesystem path needs. +#' +#' This is why the package could not read a LOCAL corpus on Windows at all -- +#' including the test fixture, hence the entire suite, and any `cog_mirror()` +#' copy. Remote https URLs were unaffected, having no backslashes, which is +#' part of why it stayed hidden: the bug predates the `{long_files}` token and +#' lived in the original `{url}` substitution, unnoticed because nothing ever +#' ran on Windows until the mirror's check matrix existed. +#' @noRd .render_view_sql <- function(sql, url, manifest = list()) { - sql <- gsub("\\{long_files\\}", .long_files_sql(url, manifest), sql, fixed = FALSE) - gsub("\\{url\\}", url, sql, fixed = FALSE) + sql <- gsub("{long_files}", .long_files_sql(url, manifest), sql, fixed = TRUE) + gsub("{url}", url, sql, fixed = TRUE) } #' Register DuckDB views from inst/sql/ SQL files diff --git a/tests/testthat/test-long-files.R b/tests/testthat/test-long-files.R index af84007..dbd1161 100644 --- a/tests/testthat/test-long-files.R +++ b/tests/testthat/test-long-files.R @@ -62,3 +62,36 @@ test_that("registered `long` view reads through the enumerated list", { expect_true(all(c(2011, 2012, 2019, 2020) %in% yrs)) }) }) + +test_that("a Windows-style corpus path survives token substitution", { + # gsub() in regex mode treats backslashes in the REPLACEMENT as escape + # sequences and silently drops them, so a Windows path went in as + # C:\Users\RUNNER\... and came out as C:UsersRUNNER..., after which every + # DuckDB read failed with "No files found that match the pattern". + # + # That made a LOCAL corpus unreadable on Windows -- the bundled fixture + # included, so the whole suite failed there -- while remote https URLs + # worked fine, having no backslashes. It went unnoticed for the life of the + # package because nothing ever ran on Windows. + # + # Reproducible on any platform: this is string handling, not a filesystem + # behaviour, so it does not need a Windows runner to catch. + win <- "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\Rtmp123/" + + out <- uscogdata:::.render_view_sql( + "FROM read_parquet('{url}data/summary_categories.parquet')", win + ) + expect_true(grepl("C:\\Users\\RUNNER~1\\AppData", out, fixed = TRUE)) + expect_false(grepl("C:Users", out, fixed = TRUE)) + + # The same must hold through the {long_files} path, which embeds the url + # once per enumerated partition. + manifest <- list(files = list(long_partitions = list( + list(year = 2011L, path = "data/long/year=2011/part-0.parquet") + ))) + out2 <- uscogdata:::.render_view_sql( + "FROM read_parquet({long_files}, hive_partitioning = true)", win, manifest + ) + expect_true(grepl("C:\\Users\\RUNNER~1\\AppData", out2, fixed = TRUE)) + expect_false(grepl("C:Users", out2, fixed = TRUE)) +})