fix: local corpus paths were unreadable on Windows (backslashes eaten) #55
@@ -122,9 +122,25 @@
|
|||||||
#' fallback -- correct for the local temp corpora the direct-execution tests
|
#' fallback -- correct for the local temp corpora the direct-execution tests
|
||||||
#' build.
|
#' build.
|
||||||
#' @noRd
|
#' @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()) {
|
.render_view_sql <- function(sql, url, manifest = list()) {
|
||||||
sql <- gsub("\\{long_files\\}", .long_files_sql(url, manifest), sql, fixed = FALSE)
|
sql <- gsub("{long_files}", .long_files_sql(url, manifest), sql, fixed = TRUE)
|
||||||
gsub("\\{url\\}", url, sql, fixed = FALSE)
|
gsub("{url}", url, sql, fixed = TRUE)
|
||||||
}
|
}
|
||||||
|
|
||||||
#' Register DuckDB views from inst/sql/ SQL files
|
#' Register DuckDB views from inst/sql/ SQL files
|
||||||
|
|||||||
@@ -62,3 +62,36 @@ test_that("registered `long` view reads through the enumerated list", {
|
|||||||
expect_true(all(c(2011, 2012, 2019, 2020) %in% yrs))
|
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))
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user