Allow dashes in bundle init catalog/schema names and escape them in generated SQL - #6293
Merged
Conversation
Collaborator
Integration test reportCommit: d764e7f
8 interesting tests: 4 SKIP, 3 KNOWN, 1 RECOVERED
Top 6 slowest tests (at least 2 minutes):
|
lennartkats-db
requested changes
Aug 18, 2026
| "spark.sql(f\"USE CATALOG {catalog}\")\n", | ||
| "spark.sql(f\"USE SCHEMA {schema}\")" | ||
| "spark.sql(\"USE CATALOG IDENTIFIER(:catalog)\", args={\"catalog\": catalog})\n", | ||
| "spark.sql(\"USE SCHEMA IDENTIFIER(:schema)\", args={\"schema\": schema})" |
Contributor
There was a problem hiding this comment.
I like the idea of supporting -! But this is a very unfortunate change, it makes it seem more complex for first-time users and agents who might copy this pattern.
Could you look at alternatives
- Perhaps backticks around the catalog/schema name also fix this?
- Perhaps we should take the complexity to us as maintainers and only conditionally as needed, for dashes?
Contributor
Author
There was a problem hiding this comment.
Thank for your review.
I replaced IDENTIFIER with backticks as you suggested.
As of adding backticks conditionally I think it would add complexity to the code and for end users might cause issues when catalog is a ${var.catalog} variable and later they can point it to a dashed name. With applying backticks for all cases we mitigate this scenario. Let me know what you think.
Unity Catalog names may contain dashes, but `databricks bundle init` rejected them (pattern `^\w*$`). Loosen the `default_catalog` pattern to `^(\w[\w-]*)?$` (dashes allowed, blank still valid, leading dash rejected) and the `shared_schema` pattern to `^\w[\w-]*$` across the built-in templates. Backtick-quote the catalog and schema identifiers in the SQL generated by the default-sql, default-python, and default-scala templates so dashed names work at runtime. The names are already validated to word characters and dashes, so backticks are sufficient and keep the generated code readable. Tests: new acceptance test default-sql-catalog-dash inits with dashed catalog/schema names, diffs the generated databricks.yml, and asserts leading-dash names are rejected at init. Co-authored-by: Isaac
pavloKozlov
force-pushed
the
uc-catalog-dash
branch
from
August 18, 2026 08:01
ab497cb to
d764e7f
Compare
lennartkats-db
approved these changes
Aug 18, 2026
denik
approved these changes
Aug 18, 2026
| @@ -0,0 +1,5 @@ | |||
| [[Repls]] | |||
| # On Windows $TESTDIR expands with backslashes; normalize them so the diff.py | |||
Contributor
There was a problem hiding this comment.
best practice is to find the place where slashes are output and evaluate if we can make it forward slash on all platforms
pavloKozlov
added a commit
that referenced
this pull request
Aug 18, 2026
Normalize the TESTDIR env var with filepath.ToSlash so paths built from it (e.g. the diff.py invocation echoed in trace output) are stable across OSes. Previously TESTDIR carried Windows backslashes, so a test echoing it needed a per-test `\\`->`/` replacement in test.toml. Remove that now-redundant replacement from default-sql-catalog-dash, whose only purpose was normalizing the TESTDIR path. Other backslash replacements in the suite address paths emitted by the CLI itself and stay. Follow-up to review feedback on #6293. Co-authored-by: Isaac
bjornkpu
pushed a commit
to bjornkpu/databricks-cli
that referenced
this pull request
Aug 19, 2026
) ## Changes Normalize the `TESTDIR` env var in the acceptance harness with `filepath.ToSlash`, so paths built from `$TESTDIR` (e.g. the `diff.py` invocation echoed in `trace` output) use forward slashes on every OS. The other backslash replacements across the suite address paths emitted by the CLI itself (bundle roots, sync paths, artifact globs) and are left in place. ## Why Follow-up to review feedback from @denik on databricks#6293: rather than papering over the Windows backslashes with a per-test replacement, fix it at the source where the path is produced. This matches the repo rule "Always output file paths with forward slashes, even on Windows. Use `filepath.ToSlash`." The `[TESTROOT]` replacement already registers both slash forms on Windows (`SetPathNoEval`), so the forward-slash `TESTDIR` prefix is still collapsed to `[TESTROOT]` — no golden changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unity Catalog names may contain dashes, but
databricks bundle initrejectedthem (pattern
^\w*$). This PR:default_catalogprompt to^(\w[\w-]*)?$(dashes allowed, blank still valid, leading-dash rejected) andshared_schemato^\w[\w-]*$across the built-in templates.IDENTIFIER()in the SQL generated by the default-sql, default-python, and default-scala templates, so dashed names work at runtime.Why
UC allows any character except
., space,/, and control chars, somy-catalogis valid — but init blocked it, and rawUSE CATALOG my-catalogwould fail at runtime.IDENTIFIER()is the safe form already used for the schema in default-sql.Tests
New acceptance test
default-sql-catalog-dashinits with dashed catalog/schema names, diffs the generateddatabricks.yml, and asserts leading-dash names are rejected at init. Existing template goldens regenerated; passes on bothterraformanddirectengines.