Skip to content

Support '*' wildcards in --actions selectors as documented (#2224) - #2234

Open
ihistand wants to merge 2 commits into
dataform-co:mainfrom
ihistand:fix/actions-wildcards-2224
Open

Support '*' wildcards in --actions selectors as documented (#2224)#2234
ihistand wants to merge 2 commits into
dataform-co:mainfrom
ihistand:fix/actions-wildcards-2224

Conversation

@ihistand

@ihistand ihistand commented Jul 28, 2026

Copy link
Copy Markdown

Fixes #2224 — following up on #2224 (comment) ("Feel free to send a PR :)").

Problem

The --actions help text for run and compile says patterns "can include '*' wildcards", but matchPatterns in core/utils.ts does plain string equality only, so no wildcard pattern ever matches — dataform run --actions "*" reports No actions to run. even when the compiled graph has actions.

Fix

Wildcard patterns are compiled to an anchored RegExp:

  • every non-* character matches literally (regex metacharacters are escaped, so . stays a literal dot);
  • each * matches any run of characters (mrd*/^mrd.*$/, *features*/^.*features.*$/).

Scoping mirrors the existing exact-match branches: a pattern containing . matches against the fully-qualified action name; otherwise it matches against the unqualified last segment. Wildcard matches bypass the ambiguous-name error since selecting many actions is the intent. Exact (non-wildcard) selection behavior is completely unchanged.

Tests

Adds a matchPatterns suite to core/utils_test.ts (the function was previously untested) covering: exact unqualified/qualified selection, the ambiguity error, bare *, prefix and substring wildcards, qualified wildcards (schema.*), no-match returning empty, and the literal-dot escaping edge case.

Understood that GCP Dataform's hosted actions filter doesn't use this implementation — this change only brings the open-source CLI in line with its own documented behavior.

🤖 Generated with Claude Code

https://claude.ai/code/session_0171FwKo8gRQQ35VoYDtSHNU

@ihistand
ihistand requested a review from a team as a code owner July 28, 2026 03:05
@ihistand
ihistand requested review from andrzej-grudzien and removed request for a team July 28, 2026 03:05
@google-cla

google-cla Bot commented Jul 28, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@apilaskowski

Copy link
Copy Markdown
Collaborator

/gcbrun

@apilaskowski

Copy link
Copy Markdown
Collaborator

PTAL at following error: ERROR: /workspace/core/utils.ts:35:10 - Found non-literal argument to RegExp Constructor

Comment thread core/utils.ts Outdated
// escaped); each '*' matches any run of characters, so "mrd*" -> /^mrd.*$/ and
// "*features*" -> /^.*features.*$/.
function globToRegExp(pattern: string): RegExp {
const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&").replace(/\\\*/g, ".*");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you simplify this logic?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, that was doing too much at once — escaping every metacharacter including *, then
un-escaping \* back into .*. You had to read both replaces together to see what it did.

Rewritten to split on the wildcards, escape the literal parts, and rejoin:

function globToRegExp(pattern: string): RegExp {
  const escapeLiteral = (literal: string) => literal.replace(/[.+?^${}()|[\]\\]/g, "\\$&");
  return new RegExp(`^${pattern.split("*").map(escapeLiteral).join(".*")}$`);
}

Behaviour is unchanged — the existing matchPatterns tests pass, and I diffed the two
implementations exhaustively over patterns covering every regex metacharacter plus ** and a**b.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note that this change isn't pushed yet, awaiting response)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds ok, but let's comment the choice of characters and why first replace is needed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Turns a selector pattern into an anchored RegExp in which "" is a wildcard and
// everything else is literal text, e.g. "mrd
" -> /^mrd.$/ and
// "features" -> /^.features.$/.
//
// Splitting on "
" first is what keeps the rest simple: every "" in a pattern is a
// wildcard by definition, so the pieces between them are pure literal text and can be
// escaped wholesale, then rejoined with ".
".
//
// The escape is needed because an action name is not regex-safe. Names are
// dot-separated ("project.dataset.name"), and "." in a regex matches any character, so
// without escaping "schema." would also select "schemaXtable" — see the "literal dot"
// case in utils_test.ts. The set below is the usual list of JavaScript regex
// metacharacters with one deliberate omission: "
", which is left out because the split
// above has already consumed every "*", so none can reach here. ("]" and "" carry
// backslashes for the character class's own syntax; "-" and "/" are not metacharacters
// outside a class and so need no escaping.)
function globToRegExp(pattern: string): RegExp {
const escapeLiteral = (literal: string) => literal.replace(/[.+?^${}()|[]\]/g, "\$&");
return new RegExp(^${pattern.split("*").map(escapeLiteral).join(".*")}$);
}

Comment thread core/utils.ts
// fully-qualified action name; otherwise it matches against the unqualified
// name (last segment), mirroring the exact-match branches below. Wildcards
// are expected to select many actions, so no ambiguity error applies here.
const regExp = globToRegExp(pattern);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you first to split by components and then apply regexes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to switch if that's the semantics you want, but it isn't behaviour-neutral, so I'd rather
confirm the intent than guess.

I implemented component-wise matching (split pattern and value on ., require the same number of
parts, apply one regex per part) and diffed it against the current implementation. One case
changes: whether * may cross a dot.

The values here are targetAsReadableString output — project.dataset.name, or dataset.name
when defaultProject isn't set. On the usual three-part name:

pattern current component-wise
*, orders, orders* same same
*.dataset.orders, *.dataset.* same same
*.orders every …orders action nothing

Component-wise is the conventional glob rule (* stops at the separator, like shell * and /),
and I'm not against it. The one thing that gives me pause is that --actions "*.orders" reads like
"the orders table in whichever dataset", and component-wise it selects nothing on a three-part
name — the user has to know to write *.*.orders. Since wildcards deliberately don't raise the
no-match/ambiguity error, that failure is silent. Letting * span dots avoids it, at the cost of
being less strict.

Which would you prefer? If component-wise, I'll push it with tests pinning the part-count
behaviour explicitly.

(Probably out of scope here, but worth separating out: dataset.* matches nothing under either
scheme on a three-part name, because a pattern containing . is matched against the fully-qualified
name. Exact dataset.orders behaves the same way today, so the wildcard branch is at least
consistent with the existing rule — happy to look at that separately if it's worth changing.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, after your explanation I'm ok with matching the whole target without splitting by components

…co#2224)

matchPatterns did plain string equality only, so the '*' wildcards the
--actions help text (run + compile) advertises never matched anything —
`run --actions "*"` / "mrd*" reported "No actions to run."

Compile wildcard patterns to an anchored RegExp: non-'*' characters
match literally (regex metacharacters escaped, so '.' stays a literal
dot), each '*' becomes '.*'. A pattern containing '.' matches the
fully-qualified action name, otherwise the unqualified last segment —
mirroring the existing exact-match branches. Wildcards bypass the
ambiguous-name error since matching many actions is the intent; exact
selection is unchanged.

Adds a matchPatterns test suite (previously untested).

Fixes dataform-co#2224
@ihistand
ihistand force-pushed the fix/actions-wildcards-2224 branch from 18e4fb3 to 7df93b7 Compare August 3, 2026 01:05
@kolina

kolina commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

/gcbrun

@kolina kolina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix issues from #2234 (comment)

Comment thread core/utils.ts Outdated
// escaped); each '*' matches any run of characters, so "mrd*" -> /^mrd.*$/ and
// "*features*" -> /^.*features.*$/.
function globToRegExp(pattern: string): RegExp {
const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, "\\$&").replace(/\\\*/g, ".*");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds ok, but let's comment the choice of characters and why first replace is needed

Comment thread core/utils.ts
// fully-qualified action name; otherwise it matches against the unqualified
// name (last segment), mirroring the exact-match branches below. Wildcards
// are expected to select many actions, so no ambiguity error applies here.
const regExp = globToRegExp(pattern);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, after your explanation I'm ok with matching the whole target without splitting by components

Escaping every metacharacter (including '*') and then un-escaping '\*' back
into '.*' meant reading the pair of replaces together to see what it does, and
made the literal-asterisk case hard to check by eye.

Split on the wildcards instead, escape the literal parts, and rejoin with '.*'.
Same behaviour: verified against the existing matchPatterns tests, and
exhaustively against the previous implementation over patterns covering every
regex metacharacter plus '**' and 'a**b'.
ihistand added a commit to SQLAnvil/sqlanvil that referenced this pull request Aug 11, 2026
Port of the upstream review outcome on dataform-co/dataform#2234. Escaping
every metacharacter (including '*') and then un-escaping '\*' back into '.*'
meant reading the pair of replaces together to see what it does. Split on the
wildcards instead, escape the literal parts, and rejoin with '.*'.

Behaviour is unchanged. The comment now explains why the escape is needed and
why '*' is excluded from the character set, and records that '*' spans dots on
purpose -- SQLAnvil action names have two parts on Postgres/MySQL and three on
BigQuery, so component-wise matching would make "*.orders" silently select
nothing depending on the warehouse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W86VUf4ptKnmhxWoqgA7gb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--actions flag does not support * wildcards despite CLI help text claiming it does

3 participants