You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "sshiv012 (via GitHub)" <gi...@apache.org> on 2026/09/23 01:01:58 UTC
[PR] feat: add a delimiter picker with regex validation [texera]
sshiv012 opened a new pull request, #8644:
URL: https://github.com/apache/texera/pull/8644
### What changes were proposed in this PR?
Delimiter properties were plain text boxes. A stored tab looked empty, typing `\t` stopped at the backslash, and Unnest String accepted invalid or degenerate regexes without a word. This replaces them with a **delimiter picker**: common delimiters by name, plus a **Custom…** entry. Each operator picks a mode through a schema hint (`@JsonSchemaInject(json = UIWidget.UIWidgetCharDelimiter / UIWidgetRegexDelimiter)`).
```
CSV scans (char) Comma Tab Semicolon Pipe Space | Custom… (1 char; typed \t -> tab)
Unnest String (regex) Comma Tab Semicolon Pipe (\|) Whitespace (\s+) New line | Custom regex… (validated)
```
| Area | Change |
|---|---|
| Frontend | New formly type `delimiter` (`frontend/src/app/common/formly/delimiter/`). Works in the canvas property panel and the Form View. Delimiter errors show immediately, including for invalid values loaded from a saved workflow. |
| Regex check (browser) | Must compile, must not match the empty string (`\|`, `\s*`), must not match every character (`.`). The last is **exact**: it tests every BMP character other than line breaks, stopping at the first miss, so `[a0-]` or `[^,]` pass. Patterns using Java-only syntax (inline flags, possessive quantifiers, `\p{..}`, `\x{..}`, `\Q..\E`, `\e`, `\h`, `\z`, …) are left to the backend, because JavaScript can't parse them or reads them differently. |
| Regex check (backend) | `UnnestStringOpDesc` makes the same checks with Java's engine during schema propagation, so a bad pattern is a **compile error** instead of a runtime `PatternSyntaxException` or an empty result. |
| CSV scans | `CSVFileScan`, `ParallelCSVFileScan`, `CSVOldFileScan` use the char-mode widget. `maxLength: 1` is kept. |
**Saved workflows:** the stored value is still the delimiter itself, so existing workflows load unchanged and a stored tab shows as "Tab".
**One intentional behavior change:** a saved Unnest String delimiter that matches the empty string or every character (e.g. `|`, `.`) now fails to compile. Previously it ran and silently split every character apart, or output only line breaks.
#### Before / after
<!-- drag comparison/C1-csv-stored-tab.png here -->
<!-- drag comparison/C2-csv-typing-tab.png here -->
<!-- drag comparison/C3-csv-choices.png here -->
<!-- drag comparison/C4-csv-legacy-value.png here -->
<!-- drag comparison/C5-unnest-choices.png here -->
<!-- drag comparison/C6-unnest-pipe.png here -->
<!-- drag comparison/C7-unnest-dot.png here -->
<!-- drag comparison/C8-unnest-invalid.png here -->
<!-- drag comparison/C9-unnest-compile.png here -->
_Screenshots C1–C9: to be attached._
### Any related issues, documentation, discussions?
Closes #8638
### How was this PR tested?
**Automated.** Tests were written first and seen failing, then made to pass.
| Suite | Result |
|---|---|
| Frontend, full (`ng test`) | 232 files, 6,376 passed |
| Delimiter / formly specs (included above) | presets for both modes, custom-regex validation (valid, invalid, empty-match, every-character, Java-only syntax), edge cases, and a rendered test that feeds the backend's exact schema through formly-jsonschema |
| `WorkflowOperator/test`, full | 2,602 passed |
| New backend specs | each CSV preset read by all three CSV readers; Unnest String split with every regex preset and custom patterns; compile-time rejection with Java semantics |
| Lint / format / types / build | eslint, prettier-eslint, `tsc`, `ng build`, scalafmt: clean |
**Manual, in the browser.** Local stack (`bin/local-dev.sh up`), Chrome driven by Playwright, 1600×1000:
1. Open a workflow with CSV File Scan and Unnest String, click each operator, and open the Delimiter picker.
2. Pick each preset; choose Custom… and type `#`, `\t`, `ab`.
3. For Unnest String, choose Custom regex… and type `[,;]\s*`, `(`, `[a-`, `a{2,1}`, `|`, `\s*`, `.`, `\.`, `,++`, then clear the field.
4. Type `(?i)(` and `(?s).`: the browser leaves them to the backend, and the Result Panel shows the compile error.
All 33 scripted checks pass. The before/after screenshots above come from this run and from the same flow on the base commit.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "parshimers (via GitHub)" <gi...@apache.org>.
parshimers merged PR #8644:
URL: https://github.com/apache/texera/pull/8644
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "sshiv012 (via GitHub)" <gi...@apache.org>.
sshiv012 commented on code in PR #8644:
URL: https://github.com/apache/texera/pull/8644#discussion_r4169532576
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -94,6 +97,23 @@ export const TEXERA_FORMLY_CONFIG = {
{ name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] },
{ name: "constrainedvalue", component: ConstrainedValueComponent, wrappers: ["form-field"] },
{ name: "resourcevalue", component: ResourceValueSelectorComponent },
+ {
+ name: "delimiter",
+ component: DelimiterTypeComponent,
+ wrappers: ["form-field"],
+ defaultOptions: {
+ // Shown without waiting for a touch: a delimiter loaded from a saved workflow can already be
+ // invalid (an old multi-character one), and nobody touches a field they do not know is wrong.
+ validation: { show: true },
+ validators: {
+ delimiter: {
+ expression: (control: AbstractControl, field: FormlyFieldConfig) =>
+ delimiterError(control.value, delimiterModeOf(field)) === undefined,
+ message: delimiterValidationMessage,
Review Comment:
Thanks, good catch. Agreed it's a double evaluation on the error path. I'd keep it as is: the full scan only happens for match-everything patterns, few ms at most if measured correctly and every other pattern exits at the first character it misses. A module-level memo would add shared state for a negligible gain. So I believe we can accept the current cost as negligible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8644:
URL: https://github.com/apache/texera/pull/8644#issuecomment-5787187540
### :warning: JUnit XML file not found
The CLI was unable to find any JUnit XML files to upload.
For more help, visit our [troubleshooting guide](https://docs.codecov.com/docs/test-analytics#troubleshooting).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #8644:
URL: https://github.com/apache/texera/pull/8644#discussion_r4110234532
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -94,6 +97,23 @@ export const TEXERA_FORMLY_CONFIG = {
{ name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] },
{ name: "constrainedvalue", component: ConstrainedValueComponent, wrappers: ["form-field"] },
{ name: "resourcevalue", component: ResourceValueSelectorComponent },
+ {
+ name: "delimiter",
+ component: DelimiterTypeComponent,
+ wrappers: ["form-field"],
+ defaultOptions: {
+ // Shown without waiting for a touch: a delimiter loaded from a saved workflow can already be
+ // invalid (an old multi-character one), and nobody touches a field they do not know is wrong.
+ validation: { show: true },
+ validators: {
+ delimiter: {
+ expression: (control: AbstractControl, field: FormlyFieldConfig) =>
+ delimiterError(control.value, delimiterModeOf(field)) === undefined,
+ message: delimiterValidationMessage,
Review Comment:
Nit: the `expression` validator computes `delimiterError(...)` only to compare it against `undefined` and discard the string, then `delimiterValidationMessage` (line 166) recomputes the very same `delimiterError(...)` to render the message.
Why it matters: for a match-everything pattern (`.`, `[\s\S]`), `delimiterError` runs the full ~65k-character `matchesEveryCharacter` scan, and in the invalid case that scan now runs twice per validation pass (once here, once in the message). It is only the error path and only for that pattern class, so it is minor, not a correctness issue.
No suggestion since Formly's split of validator predicate and message function makes a clean single-evaluation awkward: if you want to avoid the double scan, cache the last `{value, mode, error}` in a small module-level memo that both the expression and `delimiterValidationMessage` read, or accept the current cost as negligible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8644:
URL: https://github.com/apache/texera/pull/8644#issuecomment-5787215263
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 5 worse · ⚪ 8 noise (<±5%) · 0 without baseline
> Compared against main `54fba31` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/35804596752)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 488 | 0.298 | 19,352/30,807/30,807 us | 🔴 +25.5% / 🔴 +98.1% |
| 🟢 | bs=100 sw=10 sl=64 | 1,173 | 0.716 | 85,112/104,155/104,155 us | 🟢 -14.3% / 🟢 +19.3% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,400 | 0.854 | 717,932/770,725/770,725 us | ⚪ within ±5% / 🟢 +38.3% |
<details><summary>Baseline details</summary>
Latest main `54fba31` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 488 tuples/sec | 584 tuples/sec | 771.42 tuples/sec | -16.4% | -36.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.298 MB/s | 0.356 MB/s | 0.471 MB/s | -16.3% | -36.7% |
| bs=10 sw=10 sl=64 | p50 | 19,352 us | 15,414 us | 12,757 us | +25.5% | +51.7% |
| bs=10 sw=10 sl=64 | p95 | 30,807 us | 25,470 us | 15,551 us | +21.0% | +98.1% |
| bs=10 sw=10 sl=64 | p99 | 30,807 us | 25,470 us | 19,296 us | +21.0% | +59.7% |
| bs=100 sw=10 sl=64 | throughput | 1,173 tuples/sec | 1,155 tuples/sec | 983.07 tuples/sec | +1.6% | +19.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.716 MB/s | 0.705 MB/s | 0.6 MB/s | +1.6% | +19.3% |
| bs=100 sw=10 sl=64 | p50 | 85,112 us | 81,207 us | 102,799 us | +4.8% | -17.2% |
| bs=100 sw=10 sl=64 | p95 | 104,155 us | 121,580 us | 109,018 us | -14.3% | -4.5% |
| bs=100 sw=10 sl=64 | p99 | 104,155 us | 121,580 us | 121,110 us | -14.3% | -14.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,400 tuples/sec | 1,429 tuples/sec | 1,012 tuples/sec | -2.0% | +38.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.854 MB/s | 0.872 MB/s | 0.618 MB/s | -2.1% | +38.2% |
| bs=1000 sw=10 sl=64 | p50 | 717,932 us | 699,517 us | 1,006,484 us | +2.6% | -28.7% |
| bs=1000 sw=10 sl=64 | p95 | 770,725 us | 806,024 us | 1,046,167 us | -4.4% | -26.3% |
| bs=1000 sw=10 sl=64 | p99 | 770,725 us | 806,024 us | 1,074,106 us | -4.4% | -28.2% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,409.43,200,128000,488,0.298,19351.66,30806.60,30806.60
1,100,10,64,20,1704.48,2000,1280000,1173,0.716,85111.88,104154.95,104154.95
2,1000,10,64,20,14288.10,20000,12800000,1400,0.854,717932.42,770725.13,770725.13
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "sshiv012 (via GitHub)" <gi...@apache.org>.
sshiv012 commented on PR #8644:
URL: https://github.com/apache/texera/pull/8644#issuecomment-5787419252
/request-review @Ma77Ball
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8644:
URL: https://github.com/apache/texera/pull/8644#issuecomment-5787169221
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@kz930`, `@Yicong-Huang`, `@Ma77Ball`
You can notify them by mentioning `@kz930`, `@Yicong-Huang`, `@Ma77Ball` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add a delimiter picker with regex validation [texera]
Posted by "sshiv012 (via GitHub)" <gi...@apache.org>.
sshiv012 commented on code in PR #8644:
URL: https://github.com/apache/texera/pull/8644#discussion_r4169532576
##########
frontend/src/app/common/formly/formly-config.ts:
##########
@@ -94,6 +97,23 @@ export const TEXERA_FORMLY_CONFIG = {
{ name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] },
{ name: "constrainedvalue", component: ConstrainedValueComponent, wrappers: ["form-field"] },
{ name: "resourcevalue", component: ResourceValueSelectorComponent },
+ {
+ name: "delimiter",
+ component: DelimiterTypeComponent,
+ wrappers: ["form-field"],
+ defaultOptions: {
+ // Shown without waiting for a touch: a delimiter loaded from a saved workflow can already be
+ // invalid (an old multi-character one), and nobody touches a field they do not know is wrong.
+ validation: { show: true },
+ validators: {
+ delimiter: {
+ expression: (control: AbstractControl, field: FormlyFieldConfig) =>
+ delimiterError(control.value, delimiterModeOf(field)) === undefined,
+ message: delimiterValidationMessage,
Review Comment:
Thanks, good catch. Agreed it's a double evaluation on the error path. I'd keep it as is: the full scan only happens for match-everything patterns, few ms at most if measured correctly and every other pattern exits at the first character it misses. A module-level memo would add shared state for a negligible gain. So I believe we can accept the current cost as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org