diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d8fee8b97..b1497e322 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -15,8 +15,8 @@ kind, and dirsql never injects a column the table did not produce: `content` column — the one place dirsql supplies columns for you. Attaching a `--on-file` parser replaces the stat columns with the parser's output. -There is no automatic fact injection and no glob capture: a `{name}` glob -segment is rewritten to `*` and captures nothing. (Both mechanisms were +There is no automatic fact injection and no glob capture: a `{name}` in a glob is +a literal, not a wildcard. (Both mechanisms were removed in the fact-removal epic, [#624](https://github.com/thekevinscott/dirsql/issues/624).) **Content interpretation is intentionally out of scope.** dirsql does not @@ -224,7 +224,7 @@ Walks a directory tree and matches files against table globs. Returns a list of ### `matcher` -- Glob-to-table mapping -Maps glob patterns to table names and handles ignore patterns. A file is matched against every glob in registration order; every matching pattern fires, so a file can belong to multiple tables. A `{name}` placeholder in a glob is rewritten to `*` before compilation, so it matches a single path segment but captures no value (glob captures were removed in #624). +Maps glob patterns to table names and handles ignore patterns. A file is matched against every glob in registration order; every matching pattern fires, so a file can belong to multiple tables. A `{name}` in a glob is a literal; `{a,b}` is an alternation. ### `watcher` -- Filesystem monitoring diff --git a/packages/python/changelog.d/2026-10-06-config-braces.md b/packages/python/changelog.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..35b093d67 --- /dev/null +++ b/packages/python/changelog.d/2026-10-06-config-braces.md @@ -0,0 +1,3 @@ +**Changed** In a declared `[[table]] glob`, `{name}` is a literal, not a +wildcard, as in a path-table and in bash. The load-time error for a `{name}` +that collides with a declared column is gone. Write `*` for the old meaning. diff --git a/packages/python/e2e-attestations/claude-1258-config-braces.json b/packages/python/e2e-attestations/claude-1258-config-braces.json new file mode 100644 index 000000000..74c00fab2 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1258-config-braces.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e/glob_speed_matchers_test.py tests/e2e/glob_speed_one_level_test.py tests/e2e/query_contract_test.py -x -q", + "ran_at": 1791333540, + "exit_code": 0, + "commit": "37586d9320ebca5d4be159cfe3a523ebcf017884", + "branch": "claude/1258-config-braces" +} diff --git a/packages/python/migrations.d/2026-10-06-config-braces.md b/packages/python/migrations.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..5a3ef25a5 --- /dev/null +++ b/packages/python/migrations.d/2026-10-06-config-braces.md @@ -0,0 +1,40 @@ +### A braced name in a config glob is a literal + +#### Summary + +`glob = "data/{id}/metadata.json"` used to treat `{id}` as `*`. It now matches +a directory literally named `{id}`. `{a,b}` alternation is unchanged. A column +named like a `{name}` no longer errors at load. + +#### Required changes + +| Before | After | +| --- | --- | +| `glob = "data/{id}/metadata.json"` | `glob = "data/*/metadata.json"` | + +#### Deprecations removed + +- The `CaptureColumnCollision` error: a `{name}` is no longer a capture, so + nothing can collide. + +#### Behavior changes without code changes + +- A config that kept `{name}` in a glob stops matching files and loads no rows. + +#### Verification + +```bash +cd "$(mktemp -d)" +mkdir -p 'data/{id}' data/x +echo '[{"p":"lit"}]' > 'data/{id}/m.json' +echo '[{"p":"other"}]' > data/x/m.json +cat > .dirsql.toml <<'TOML' +[[table]] +name = "t" +glob = "data/{id}/m.json" +ddl = "CREATE TABLE t (p TEXT)" +on-file = "jq -sc add" +TOML +dirsql query "SELECT p FROM t" -c .dirsql.toml +# expected: [{"p":"lit"}] +``` diff --git a/packages/python/tests/e2e/capture_collision_test.py b/packages/python/tests/e2e/capture_collision_test.py deleted file mode 100644 index caceabf83..000000000 --- a/packages/python/tests/e2e/capture_collision_test.py +++ /dev/null @@ -1,74 +0,0 @@ -"""CLI e2e: a glob `{name}` placeholder colliding with a declared DDL column -is a load-time error through the real launcher + binary. - -Captures no longer populate columns, so a placeholder whose name is also a -declared column would read NULL forever. The launcher must exit non-zero and -surface the collision diagnostic. No mocks: real launcher, real binary, real -process, real filesystem. -""" - -import os -import shutil -import subprocess -import sys - -import dirsql as _dirsql_pkg - -_REPO_ROOT = os.path.abspath( - os.path.join(os.path.dirname(__file__), "..", "..", "..", "..") -) -_RELEASE = os.path.join(_REPO_ROOT, "target", "release", "dirsql") -_DEBUG = os.path.join(_REPO_ROOT, "target", "debug", "dirsql") -_BINARY = _RELEASE if os.path.exists(_RELEASE) else _DEBUG - -_BINARY_STAGE_DIR = os.path.join(os.path.dirname(_dirsql_pkg.__file__), "_binary") - - -def describe_capture_column_collision(): - def it_exits_nonzero_and_names_the_collision(tmp_path): - assert os.path.exists(_BINARY), ( - f"dirsql binary not built at {_BINARY}; " - "run `cargo build -p dirsql --features cli` first" - ) - os.makedirs(_BINARY_STAGE_DIR, exist_ok=True) - staged_binary = os.path.join(_BINARY_STAGE_DIR, "dirsql") - shutil.copy(_BINARY, staged_binary) - os.chmod(staged_binary, 0o755) - - try: - root = tmp_path / "data" - (root / "_comments" / "abc123").mkdir(parents=True) - (root / "_comments" / "abc123" / "first.txt").write_text("hi") - cfg = root / ".dirsql.toml" - cfg.write_text( - "[[table]]\n" - 'name = "comments"\n' - 'ddl = "CREATE TABLE comments (thread_id TEXT, basename TEXT)"\n' - 'glob = "_comments/{thread_id}/*.txt"\n' - 'on-file = "cat"\n' - ) - - proc = subprocess.run( - [ - sys.executable, - "-c", - "import sys; from dirsql.cli.main import main; sys.exit(main())", - "query", - "SELECT * FROM comments", - "--config", - str(cfg), - ], - cwd=str(root), - capture_output=True, - text=True, - timeout=30, - ) - - assert proc.returncode != 0, ( - f"a colliding capture config must exit non-zero, got {proc!r}" - ) - assert "thread_id" in proc.stderr and "collides" in proc.stderr, ( - f"stderr must name the colliding placeholder/column, got {proc.stderr!r}" - ) - finally: - shutil.rmtree(_BINARY_STAGE_DIR, ignore_errors=True) diff --git a/packages/python/tests/integration/binding/fanout_test.py b/packages/python/tests/integration/binding/fanout_test.py index e5ebd65b9..a44518d35 100644 --- a/packages/python/tests/integration/binding/fanout_test.py +++ b/packages/python/tests/integration/binding/fanout_test.py @@ -65,20 +65,3 @@ async def it_populates_both_tables_with_overlapping_distinct_globs(tmp_dir): b_rows = await db.query("SELECT col_b FROM tb") assert len(b_rows) == 1, "second-declared table must be populated" assert b_rows[0]["col_b"] == "B" - - @pytest.mark.asyncio - async def it_errors_when_a_placeholder_collides_with_a_column(tmp_dir): - _write_fanout_file(tmp_dir) - db = DirSQL( - tmp_dir, - tables=[ - Table( - name="a", - ddl="CREATE TABLE a (id TEXT, col_a TEXT)", - glob="data/{id}/metadata.json", - on_file=lambda _path: [{"col_a": "A"}], - ), - ], - ) - with pytest.raises(Exception, match="id"): - await db.ready() diff --git a/packages/python/tests/integration/binding/from_config_test.py b/packages/python/tests/integration/binding/from_config_test.py index 8bb05f2e2..23eba2696 100644 --- a/packages/python/tests/integration/binding/from_config_test.py +++ b/packages/python/tests/integration/binding/from_config_test.py @@ -64,63 +64,6 @@ async def it_produces_one_row_per_matched_file(config_dir): assert results[1]["path"] == "items/b.csv" assert results[1]["basename"] == "b.csv" - def describe_glob_placeholders(): - @pytest.mark.asyncio - async def it_errors_when_a_placeholder_collides_with_a_column(config_dir): - _write( - os.path.join(config_dir, "comments", "thread-1", "a.txt"), - "hello", - ) - _write( - os.path.join(config_dir, ".dirsql.toml"), - """\ -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (thread_id TEXT, basename TEXT)" -glob = "comments/{thread_id}/*.txt" -""" - + _HOOK_PATH_BASENAME - + "\n", - ) - - db = DirSQL( - root=config_dir, config=os.path.join(config_dir, ".dirsql.toml") - ) - with pytest.raises(Exception, match="thread_id"): - await db.ready() - - @pytest.mark.asyncio - async def it_treats_a_non_colliding_placeholder_as_a_wildcard(config_dir): - _write( - os.path.join(config_dir, "comments", "thread-1", "a.txt"), - "hello", - ) - _write( - os.path.join(config_dir, "comments", "thread-2", "b.txt"), - "world", - ) - _write( - os.path.join(config_dir, ".dirsql.toml"), - """\ -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (path TEXT, basename TEXT)" -glob = "comments/{thread_id}/*.txt" -""" - + _HOOK_PATH_BASENAME - + "\n", - ) - - db = DirSQL( - root=config_dir, config=os.path.join(config_dir, ".dirsql.toml") - ) - await db.ready() - results = await db.query("SELECT basename FROM comments ORDER BY basename") - assert len(results) == 2 - assert results[0]["basename"] == "a.txt" - assert results[1]["basename"] == "b.txt" - assert "thread_id" not in results[0] - def describe_stat_virtuals(): @pytest.mark.asyncio async def it_exposes_stat_virtuals(config_dir): diff --git a/packages/rust/changelog.d/2026-10-06-config-braces.md b/packages/rust/changelog.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..35b093d67 --- /dev/null +++ b/packages/rust/changelog.d/2026-10-06-config-braces.md @@ -0,0 +1,3 @@ +**Changed** In a declared `[[table]] glob`, `{name}` is a literal, not a +wildcard, as in a path-table and in bash. The load-time error for a `{name}` +that collides with a declared column is gone. Write `*` for the old meaning. diff --git a/packages/rust/migrations.d/2026-10-06-config-braces.md b/packages/rust/migrations.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..5a3ef25a5 --- /dev/null +++ b/packages/rust/migrations.d/2026-10-06-config-braces.md @@ -0,0 +1,40 @@ +### A braced name in a config glob is a literal + +#### Summary + +`glob = "data/{id}/metadata.json"` used to treat `{id}` as `*`. It now matches +a directory literally named `{id}`. `{a,b}` alternation is unchanged. A column +named like a `{name}` no longer errors at load. + +#### Required changes + +| Before | After | +| --- | --- | +| `glob = "data/{id}/metadata.json"` | `glob = "data/*/metadata.json"` | + +#### Deprecations removed + +- The `CaptureColumnCollision` error: a `{name}` is no longer a capture, so + nothing can collide. + +#### Behavior changes without code changes + +- A config that kept `{name}` in a glob stops matching files and loads no rows. + +#### Verification + +```bash +cd "$(mktemp -d)" +mkdir -p 'data/{id}' data/x +echo '[{"p":"lit"}]' > 'data/{id}/m.json' +echo '[{"p":"other"}]' > data/x/m.json +cat > .dirsql.toml <<'TOML' +[[table]] +name = "t" +glob = "data/{id}/m.json" +ddl = "CREATE TABLE t (p TEXT)" +on-file = "jq -sc add" +TOML +dirsql query "SELECT p FROM t" -c .dirsql.toml +# expected: [{"p":"lit"}] +``` diff --git a/packages/rust/src/lib.rs b/packages/rust/src/lib.rs index 6efdbd19d..25b377748 100644 --- a/packages/rust/src/lib.rs +++ b/packages/rust/src/lib.rs @@ -199,14 +199,6 @@ pub enum DirSqlError { source: Option, }, - #[error( - "glob capture `{{{placeholder}}}` collides with declared column `{column}`: \ - captures no longer populate columns, so `{column}` would always be NULL. \ - Remove `{column}` from the table's DDL, or emit its value from the on-file \ - hook by splitting the path yourself." - )] - CaptureColumnCollision { placeholder: String, column: String }, - #[error( "query() only accepts read-only statements; SQLite classified this statement as a write" )] @@ -1323,18 +1315,6 @@ impl DirSQL { eprintln!("{warning}"); } } - // Reject a `{name}` glob placeholder whose name is also a declared - // column: captures no longer populate columns, so it would read - // NULL forever. The table exists here either way (freshly created - // or restored from cache), so its columns are knowable, and this - // runs before any file is ingested — a load-time failure. - let declared_columns = db.get_table_columns(&table_name).map_err(map_db_error)?; - if let Some(name) = find_capture_column_collision(&table.glob, &declared_columns) { - return Err(DirSqlError::CaptureColumnCollision { - placeholder: name.clone(), - column: name, - }); - } on_file_map.insert(table_name.clone(), table.hook); strict_map.insert(table_name.clone(), table.strict); ddl_map.insert(table_name, table.ddl); @@ -2519,18 +2499,6 @@ pub(crate) fn json_to_value(value: &serde_json::Value) -> Value { } } -/// Returns the first `{name}` placeholder in `glob` whose name is also one of -/// `declared_columns`. `None` when the glob has no placeholders or none of -/// them names a declared column. Pure: the sole input is the glob string and -/// the column list, so it is exhaustively unit-testable. -fn find_capture_column_collision(glob: &str, declared_columns: &[String]) -> Option { - let declared: std::collections::HashSet<&str> = - declared_columns.iter().map(String::as_str).collect(); - crate::matcher::placeholder_names(glob) - .into_iter() - .find(|name| declared.contains(name.as_str())) -} - /// Async wrapper around [`DirSQL`] whose constructor returns immediately while /// the initial scan runs on a background thread. /// @@ -4506,54 +4474,6 @@ mod internal_tests { assert_eq!(deleted, vec![("gone.txt".to_string(), "t".to_string())]); } - #[test] - fn find_capture_column_collision_flags_a_placeholder_naming_a_column() { - let declared = vec!["thread_id".to_string(), "basename".to_string()]; - assert_eq!( - find_capture_column_collision("_comments/{thread_id}/*.txt", &declared), - Some("thread_id".to_string()) - ); - } - - #[test] - fn find_capture_column_collision_ignores_a_placeholder_with_no_column() { - let declared = vec!["path".to_string(), "basename".to_string()]; - assert_eq!( - find_capture_column_collision("_comments/{thread_id}/*.txt", &declared), - None - ); - } - - #[test] - fn find_capture_column_collision_none_without_placeholders() { - let declared = vec!["thread_id".to_string()]; - assert_eq!( - find_capture_column_collision("_comments/*/*.txt", &declared), - None - ); - } - - #[test] - fn find_capture_column_collision_returns_first_colliding_placeholder() { - let declared = vec!["repo".to_string(), "org".to_string()]; - assert_eq!( - find_capture_column_collision("{org}/{repo}/data.json", &declared), - Some("org".to_string()) - ); - } - - #[test] - fn capture_column_collision_error_names_placeholder_and_fix() { - let err = DirSqlError::CaptureColumnCollision { - placeholder: "thread_id".to_string(), - column: "thread_id".to_string(), - }; - let msg = err.to_string(); - assert!(msg.contains("thread_id")); - assert!(msg.contains("collides")); - assert!(msg.contains("on-file")); - } - /// A `name` the batch never created: the error has to say what it *did* /// create, or a typo gives the reader nothing to compare against. #[test] diff --git a/packages/rust/src/matcher.rs b/packages/rust/src/matcher.rs index cda0180a4..ea5c6b76c 100644 --- a/packages/rust/src/matcher.rs +++ b/packages/rust/src/matcher.rs @@ -79,8 +79,6 @@ pub struct MatchResult { pub table_name: String, } -/// A compiled glob pattern. `{name}` placeholders are rewritten to `*` before -/// compilation, so they are pure match wildcards. #[derive(Clone)] struct PatternEntry { pattern: PathGlob, @@ -100,72 +98,13 @@ pub struct TableMatcher { gitignore: bool, } -/// Byte spans of the `{name}` placeholders in `pattern`, in order: `(start, -/// end, name)` with `end` past the closing brace. -/// -/// The grammar is exactly `{` `[a-zA-Z_][a-zA-Z0-9_]*` `}`. Anything else -- -/// `{}`, `{1a}`, `{a-b}`, an unclosed `{` -- is not a placeholder and is left -/// alone, matching the leftmost-first, non-overlapping scan the equivalent -/// regex performed. -fn placeholder_spans(pattern: &str) -> Vec<(usize, usize, String)> { - let bytes = pattern.as_bytes(); - let mut spans = Vec::new(); - let mut i = 0; - while i < bytes.len() { - if bytes[i] == b'{' { - let mut j = i + 1; - if j < bytes.len() && (bytes[j].is_ascii_alphabetic() || bytes[j] == b'_') { - j += 1; - while j < bytes.len() && (bytes[j].is_ascii_alphanumeric() || bytes[j] == b'_') { - j += 1; - } - if j < bytes.len() && bytes[j] == b'}' { - // Every byte in `i..=j` is ASCII, so these are char boundaries. - spans.push((i, j + 1, pattern[i + 1..j].to_string())); - i = j + 1; - continue; - } - } - } - i += 1; - } - spans -} - -/// Names of the `{name}` placeholders in `pattern`, in order of appearance. -pub fn placeholder_names(pattern: &str) -> Vec { - placeholder_spans(pattern) - .into_iter() - .map(|(_, _, name)| name) - .collect() -} - -/// Rewrite `{name}` placeholders in a glob to `*`, so they match a single path -/// segment without producing any captured value. -fn glob_with_placeholders_as_star(pattern: &str) -> String { - let spans = placeholder_spans(pattern); - if spans.is_empty() { - return pattern.to_string(); - } - let mut out = String::with_capacity(pattern.len()); - let mut last = 0; - for (start, end, _) in spans { - out.push_str(&pattern[last..start]); - out.push('*'); - last = end; - } - out.push_str(&pattern[last..]); - out -} - impl TableMatcher { /// Build a new matcher from (glob_pattern, table_name) pairs and ignore patterns. - /// Glob patterns may contain `{name}` placeholders, which match like `*`. pub fn new(mappings: &[(&str, &str)], ignore_patterns: &[&str]) -> Result { let mut entries = Vec::new(); for (pattern, table_name) in mappings { entries.push(PatternEntry { - pattern: compile_glob(&glob_with_placeholders_as_star(pattern))?, + pattern: compile_glob(pattern)?, table_name: table_name.to_string(), }); } @@ -273,20 +212,6 @@ mod tests { ); } - #[test] - fn placeholder_glob_matches_like_a_star() { - // A `{name}` placeholder and a `*` compile to the same matcher: both - // globs match exactly the same file. - let placeholder = TableMatcher::new(&[("data/{id}/metadata.json", "a")], &[]).unwrap(); - let star = TableMatcher::new(&[("data/*/metadata.json", "a")], &[]).unwrap(); - let path = Path::new("data/x/metadata.json"); - assert_eq!(names(&placeholder, "data/x/metadata.json"), vec!["a"]); - assert_eq!( - placeholder.match_all(path).is_empty(), - star.match_all(path).is_empty(), - ); - } - #[test] fn match_all_with_nested_path() { let matcher = TableMatcher::new(&[("**/*.jsonl", "events")], &[]).unwrap(); @@ -368,105 +293,6 @@ mod tests { assert_eq!(names(&matcher, "logs/deep/nested/b.txt"), vec!["t"]); } - #[test] - fn placeholder_matches_the_filled_segment() { - let matcher = - TableMatcher::new(&[("comments/{thread_id}/index.jsonl", "comments")], &[]).unwrap(); - assert_eq!( - names(&matcher, "comments/abc123/index.jsonl"), - vec!["comments"] - ); - } - - #[test] - fn multiple_placeholders_match() { - let matcher = TableMatcher::new(&[("{org}/{repo}/data.json", "repos")], &[]).unwrap(); - assert_eq!(names(&matcher, "acme/widgets/data.json"), vec!["repos"]); - } - - #[test] - fn placeholder_no_match_returns_empty() { - let matcher = - TableMatcher::new(&[("comments/{thread_id}/index.jsonl", "comments")], &[]).unwrap(); - assert!(matcher.match_all(Path::new("other/file.txt")).is_empty()); - } - - #[test] - fn placeholder_with_double_star_matches() { - let matcher = TableMatcher::new(&[("**/{category}/items.json", "items")], &[]).unwrap(); - assert_eq!( - names(&matcher, "shop/electronics/items.json"), - vec!["items"] - ); - } - - #[test] - fn placeholder_with_question_mark_matches() { - let matcher = TableMatcher::new(&[("{name}?.txt", "files")], &[]).unwrap(); - assert_eq!(names(&matcher, "ab.txt"), vec!["files"]); - } - - #[test] - fn placeholder_names_lists_names_in_order() { - assert_eq!( - placeholder_names("{org}/{repo}/data.json"), - vec!["org".to_string(), "repo".to_string()] - ); - } - - #[test] - fn placeholder_names_empty_when_none() { - assert!(placeholder_names("data/*/metadata.json").is_empty()); - } - - #[test] - fn placeholder_names_rejects_everything_outside_the_grammar() { - // The grammar is exactly `{` `[a-zA-Z_][a-zA-Z0-9_]*` `}`. Each of these - // fails it at a different point -- empty name, a leading digit, a - // disallowed interior byte, and a brace that runs off the end of the - // pattern -- and all are left alone rather than read as a placeholder. - for pattern in ["{}", "{1a}", "{a-b}", "{a b}", "{", "a{", "{a", "{_"] { - assert!( - placeholder_names(pattern).is_empty(), - "`{pattern}` is outside the grammar, so it is not a placeholder" - ); - assert_eq!( - glob_with_placeholders_as_star(pattern), - pattern, - "`{pattern}` is not a placeholder, so the glob is unrewritten" - ); - } - } - - #[test] - fn placeholder_names_accepts_the_whole_grammar() { - // A name starts with a letter or `_` and continues with those plus - // digits. - assert_eq!(placeholder_names("{a}"), vec!["a".to_string()]); - assert_eq!(placeholder_names("{_}"), vec!["_".to_string()]); - assert_eq!(placeholder_names("{_a1}"), vec!["_a1".to_string()]); - assert_eq!(placeholder_names("{A_1z}"), vec!["A_1z".to_string()]); - } - - #[test] - fn the_scan_resumes_at_the_byte_after_a_closing_brace() { - // Leftmost-first and non-overlapping: back-to-back placeholders both - // register, and every byte outside a span survives the rewrite. - assert_eq!( - placeholder_names("{a}{b}"), - vec!["a".to_string(), "b".to_string()] - ); - assert_eq!(glob_with_placeholders_as_star("x{a}y{b}z"), "x*y*z"); - } - - #[test] - fn an_unclosed_brace_leaves_a_later_placeholder_intact() { - // The `{` at 0 fails the grammar, so the scan advances one byte at a - // time and still finds the real placeholder behind it. - assert_eq!(placeholder_names("{a{b}"), vec!["b".to_string()]); - assert_eq!(glob_with_placeholders_as_star("{a{b}"), "{a*"); - } - /// Two files at depth 0, two under `folder/`, two under `folder/sub/`, one /// under a sibling directory. const FIXTURE: [&str; 7] = [ @@ -602,12 +428,12 @@ mod tests { } #[test] - fn a_placeholder_matches_exactly_one_path_segment() { + fn a_braced_name_matches_only_itself() { let matcher = TableMatcher::new(&[("data/{id}/metadata.json", "a")], &[]).unwrap(); - assert_eq!(names(&matcher, "data/x/metadata.json"), vec!["a"]); + assert_eq!(names(&matcher, "data/{id}/metadata.json"), vec!["a"]); assert!( matcher - .match_all(Path::new("data/x/y/metadata.json")) + .match_all(Path::new("data/x/metadata.json")) .is_empty() ); } diff --git a/packages/rust/tests/cli_e2e.rs b/packages/rust/tests/cli_e2e.rs index 7229f27c2..8a8bf32e3 100644 --- a/packages/rust/tests/cli_e2e.rs +++ b/packages/rust/tests/cli_e2e.rs @@ -936,39 +936,6 @@ fn query_subcommand_rejects_internal_table_read_with_the_http_error_message() { ); } -#[test] -fn query_subcommand_rejects_capture_column_collision() { - // A `{name}` glob placeholder that collides with a declared DDL column is - // a load-time error: captures no longer populate columns, so the column - // would read NULL forever. The CLI must exit non-zero and name the - // colliding placeholder/column. - let root = TempDir::new().unwrap(); - fs::create_dir_all(root.path().join("_comments/abc123")).unwrap(); - fs::write(root.path().join("_comments/abc123/first.txt"), "hi").unwrap(); - fs::write( - root.path().join(".dirsql.toml"), - r#" -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (thread_id TEXT, basename TEXT)" -glob = "_comments/{thread_id}/*.txt" -on-file = "cat" -"#, - ) - .unwrap(); - - let out = run_query_subcommand_with_config(root.path(), "SELECT * FROM comments"); - assert!( - !out.status.success(), - "a colliding capture config must be a non-zero exit, got {out:?}" - ); - let stderr = String::from_utf8(out.stderr).unwrap(); - assert!( - stderr.contains("thread_id") && stderr.contains("collides"), - "stderr must name the colliding placeholder/column, got {stderr:?}" - ); -} - #[test] fn query_subcommand_rejects_blank_sql_with_nonzero_exit() { // The subcommand synthesizes the same `{"sql": …}` intake the server diff --git a/packages/rust/tests/config_glob_parity.rs b/packages/rust/tests/config_glob_parity.rs index e11e923cd..48dc63cc1 100644 --- a/packages/rust/tests/config_glob_parity.rs +++ b/packages/rust/tests/config_glob_parity.rs @@ -124,3 +124,19 @@ fn a_file_name_still_names_that_file() { let root = tree(&["docs/a.md", "top.md"]); assert_eq!(config_paths(root.path(), "top.md"), vec!["top.md"]); } + +#[test] +fn a_braced_name_is_not_a_wildcard() { + let root = tree(&["{name}.md", "x.md"]); + let expected = vec!["{name}.md"]; + assert_eq!(path_table_paths(root.path(), "{name}.md"), expected); + assert_eq!(config_paths(root.path(), "{name}.md"), expected); +} + +#[test] +fn a_brace_alternation_still_matches_each_option() { + let root = tree(&["a.md", "b.md", "c.md"]); + let expected = vec!["a.md", "b.md"]; + assert_eq!(path_table_paths(root.path(), "{a,b}.md"), expected); + assert_eq!(config_paths(root.path(), "{a,b}.md"), expected); +} diff --git a/packages/rust/tests/from_config.rs b/packages/rust/tests/from_config.rs index 11ca06945..3fc6346a3 100644 --- a/packages/rust/tests/from_config.rs +++ b/packages/rust/tests/from_config.rs @@ -78,113 +78,6 @@ on-file = '''sh -c 'r=$(printf %s "$1" | tr "\\\\" /); shift; printf "["; sep="" assert_eq!(rows[0]["path"], Value::Text("data/a.csv".into())); } -// A `{name}` glob placeholder whose name is ALSO a declared DDL column is a -// load-time error: captures no longer populate columns, so the column would -// read NULL forever. The error must name the placeholder and the fix. -#[test] -fn from_config_capture_column_collision_errors() { - let root = TempDir::new().unwrap(); - - fs::write( - root.path().join(".dirsql.toml"), - r#" -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (thread_id TEXT, basename TEXT)" -glob = "_comments/{thread_id}/*.txt" -on-file = "cat" -"#, - ) - .unwrap(); - - fs::create_dir_all(root.path().join("_comments").join("abc123")).unwrap(); - fs::write( - root.path() - .join("_comments") - .join("abc123") - .join("first.txt"), - "hello", - ) - .unwrap(); - - let err = match DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - { - Ok(_) => { - panic!("a {{thread_id}} placeholder colliding with the thread_id column must error") - } - Err(e) => e, - }; - - let msg = err.to_string(); - assert!( - msg.contains("thread_id"), - "error must name the colliding placeholder/column, got: {msg}" - ); - assert!( - msg.contains("collides"), - "error must explain the collision, got: {msg}" - ); -} - -// A `{name}` placeholder whose name is NOT a declared column keeps working -// silently: matching is unchanged (it behaves like `*`), and the file's -// filesystem columns still populate. Proves matching semantics are preserved. -#[test] -fn from_config_capture_placeholder_without_column_still_matches() { - let root = TempDir::new().unwrap(); - - fs::write( - root.path().join(".dirsql.toml"), - r#" -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (path TEXT, basename TEXT)" -glob = "_comments/{thread_id}/*.txt" -on-file = '''sh -c 'r=$(printf %s "$1" | tr "\\\\" /); shift; printf "["; sep=""; for p; do p=$(printf %s "$p" | tr "\\\\" /); rel=${p#"$r"/}; printf "%s{\"path\":\"%s\",\"basename\":\"%s\"}" "$sep" "$rel" "${p##*/}"; sep=","; done; printf "]"' sh {root}''' -"#, - ) - .unwrap(); - - fs::create_dir_all(root.path().join("_comments").join("abc123")).unwrap(); - fs::create_dir_all(root.path().join("_comments").join("def456")).unwrap(); - fs::write( - root.path() - .join("_comments") - .join("abc123") - .join("first.txt"), - "hello", - ) - .unwrap(); - fs::write( - root.path() - .join("_comments") - .join("def456") - .join("second.txt"), - "world", - ) - .unwrap(); - - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .unwrap(); - let rows = db - .query("SELECT basename FROM comments ORDER BY basename") - .unwrap(); - - assert_eq!(rows.len(), 2, "{{thread_id}} still matches like `*`"); - assert_eq!(rows[0]["basename"], Value::Text("first.txt".into())); - assert_eq!(rows[1]["basename"], Value::Text("second.txt".into())); - assert!( - !rows[0].contains_key("thread_id"), - "the placeholder no longer produces a column value" - ); -} - #[test] fn from_config_exposes_stat_virtuals() { let root = TempDir::new().unwrap(); diff --git a/packages/rust/tests/sdk.rs b/packages/rust/tests/sdk.rs index 84a5e08f8..a684917b1 100644 --- a/packages/rust/tests/sdk.rs +++ b/packages/rust/tests/sdk.rs @@ -836,26 +836,6 @@ fn invalid_glob_errors() { assert!(matches!(result, Err(dirsql::DirSqlError::Matcher { .. }))); } -// A `{name}` placeholder that is not a declared column is a pure match -// wildcard: it produces no column value and no error. -#[test] -fn undeclared_capture_is_dropped() { - let root = TempDir::new().unwrap(); - fs::create_dir_all(root.path().join("logs")).unwrap(); - fs::write(root.path().join("logs").join("a.txt"), "x").unwrap(); - let table = Table::new( - "entries", - "CREATE TABLE entries (path TEXT)", - "logs/{kind}.txt", - |_| vec![Row::new()], - ); - let db = DirSQL::new(root.path(), vec![table]).unwrap(); - let rows = db.query("SELECT * FROM entries").unwrap(); - assert_eq!(rows.len(), 1); - assert!(!rows[0].contains_key("kind")); - assert!(rows[0].contains_key("path")); -} - #[test] fn duplicate_table_name_errors() { let root = TempDir::new().unwrap(); @@ -939,30 +919,6 @@ fn fanout_overlapping_distinct_globs_populate_both_tables() { assert_eq!(b_rows[0]["col_b"], Value::Text("B".into())); } -// A programmatic table whose glob declares a `{name}` placeholder colliding -// with one of its DDL columns is rejected at construction, just like a -// config-file table: captures no longer populate columns. -#[test] -fn capture_column_collision_errors_on_construction() { - let root = fanout_root(); - let a = Table::new( - "a", - "CREATE TABLE a (id TEXT, col_a TEXT)", - "data/{id}/metadata.json", - |_path| vec![HashMap::from([("col_a".into(), Value::Text("A".into()))])], - ); - - let err = match DirSQL::new(root.path(), vec![a]) { - Ok(_) => panic!("a {{id}} placeholder colliding with the id column must error"), - Err(e) => e, - }; - let msg = err.to_string(); - assert!( - msg.contains("id") && msg.contains("collides"), - "error must name the collision, got: {msg}" - ); -} - #[test] fn binary_file_under_glob_does_not_break_build() { // dirsql must not eagerly read matched files as UTF-8 text: a non-UTF-8 diff --git a/packages/ts/changelog.d/2026-10-06-config-braces.md b/packages/ts/changelog.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..35b093d67 --- /dev/null +++ b/packages/ts/changelog.d/2026-10-06-config-braces.md @@ -0,0 +1,3 @@ +**Changed** In a declared `[[table]] glob`, `{name}` is a literal, not a +wildcard, as in a path-table and in bash. The load-time error for a `{name}` +that collides with a declared column is gone. Write `*` for the old meaning. diff --git a/packages/ts/e2e-attestations/claude-1258-config-braces.json b/packages/ts/e2e-attestations/claude-1258-config-braces.json new file mode 100644 index 000000000..5f16b63d2 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1258-config-braces.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1791333575, + "exit_code": 0, + "commit": "7b00ccbf8bd2231a34eeb0e5a8ae355390752fd0", + "branch": "claude/1258-config-braces" +} diff --git a/packages/ts/migrations.d/2026-10-06-config-braces.md b/packages/ts/migrations.d/2026-10-06-config-braces.md new file mode 100644 index 000000000..5a3ef25a5 --- /dev/null +++ b/packages/ts/migrations.d/2026-10-06-config-braces.md @@ -0,0 +1,40 @@ +### A braced name in a config glob is a literal + +#### Summary + +`glob = "data/{id}/metadata.json"` used to treat `{id}` as `*`. It now matches +a directory literally named `{id}`. `{a,b}` alternation is unchanged. A column +named like a `{name}` no longer errors at load. + +#### Required changes + +| Before | After | +| --- | --- | +| `glob = "data/{id}/metadata.json"` | `glob = "data/*/metadata.json"` | + +#### Deprecations removed + +- The `CaptureColumnCollision` error: a `{name}` is no longer a capture, so + nothing can collide. + +#### Behavior changes without code changes + +- A config that kept `{name}` in a glob stops matching files and loads no rows. + +#### Verification + +```bash +cd "$(mktemp -d)" +mkdir -p 'data/{id}' data/x +echo '[{"p":"lit"}]' > 'data/{id}/m.json' +echo '[{"p":"other"}]' > data/x/m.json +cat > .dirsql.toml <<'TOML' +[[table]] +name = "t" +glob = "data/{id}/m.json" +ddl = "CREATE TABLE t (p TEXT)" +on-file = "jq -sc add" +TOML +dirsql query "SELECT p FROM t" -c .dirsql.toml +# expected: [{"p":"lit"}] +``` diff --git a/packages/ts/tests/integration/binding/fanout.test.ts b/packages/ts/tests/integration/binding/fanout.test.ts index 4cbdd1881..edbc7a079 100644 --- a/packages/ts/tests/integration/binding/fanout.test.ts +++ b/packages/ts/tests/integration/binding/fanout.test.ts @@ -69,20 +69,4 @@ describe("DirSQL fan-out", () => { const b = await db.query("SELECT col_b FROM tb"); expect(b).toEqual([{ col_b: "B" }]); }); - - it("rejects a glob placeholder that collides with a declared column", async () => { - const db = new DirSQL({ - root: dir, - tables: [ - { - name: "a", - ddl: "CREATE TABLE a (id TEXT, col_a TEXT)", - glob: "data/{id}/metadata.json", - onFile: () => [{ col_a: "A" }], - }, - ], - }); - - await expect(db.ready).rejects.toThrow(/id/); - }); }); diff --git a/packages/ts/tests/integration/binding/from-config.test.ts b/packages/ts/tests/integration/binding/from-config.test.ts index 4c448c676..fa1ba831a 100644 --- a/packages/ts/tests/integration/binding/from-config.test.ts +++ b/packages/ts/tests/integration/binding/from-config.test.ts @@ -69,48 +69,6 @@ ${pathBasenameHook} expect(rows[1].basename).toBe("b.csv"); }); - it("rejects a glob placeholder that collides with a declared column", async () => { - await seedFile(join(dir, "comments", "thread-1", "a.txt"), "x"); - await seedFile( - configPath, - ` -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (thread_id TEXT, basename TEXT)" -glob = "comments/{thread_id}/*.txt" -${pathBasenameHook} -`, - ); - - const db = new DirSQL({ root: dir, config: configPath }); - await expect(db.ready).rejects.toThrow(/thread_id/); - }); - - it("treats a non-colliding placeholder as a wildcard", async () => { - await seedFile(join(dir, "comments", "thread-1", "a.txt"), "x"); - await seedFile(join(dir, "comments", "thread-2", "b.txt"), "x"); - await seedFile( - configPath, - ` -[[table]] -name = "comments" -ddl = "CREATE TABLE comments (path TEXT, basename TEXT)" -glob = "comments/{thread_id}/*.txt" -${pathBasenameHook} -`, - ); - - const db = new DirSQL({ root: dir, config: configPath }); - await db.ready; - const rows = await db.query( - "SELECT basename FROM comments ORDER BY basename", - ); - expect(rows).toHaveLength(2); - expect(rows[0].basename).toBe("a.txt"); - expect(rows[1].basename).toBe("b.txt"); - expect(rows[0].thread_id).toBeUndefined(); - }); - it("exposes the full set of stat virtuals when declared in DDL", async () => { const body = "# title\nhello world\n"; await seedFile(join(dir, "docs", "readme.md"), body);