fix(file-service): reject invalid or duplicate dataset renames#6426
Merged
Conversation
updateDatasetName now applies the same name validation and per-owner duplicate check as createDataset, backed by a new UNIQUE (owner_uid, name) constraint on the dataset table (migration dedupes pre-existing rows). The frontend rename input mirrors the validation and surfaces backend error messages.
github-actions
Bot
added
ddl-change
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 452 | 0.276 | 21,034/32,041/32,041 us | 🔴 +14.7% / 🔴 +128.3% |
| 🔴 | bs=100 sw=10 sl=64 | 955 | 0.583 | 103,880/130,679/130,679 us | 🔴 +12.7% / 🔴 +33.6% |
| 🟢 | bs=1000 sw=10 sl=64 | 1,126 | 0.687 | 888,508/930,757/930,757 us | 🟢 -7.5% / ⚪ within ±5% |
Baseline details
Latest main e793be5 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 452 tuples/sec | 492 tuples/sec | 845.85 tuples/sec | -8.1% | -46.6% |
| bs=10 sw=10 sl=64 | MB/s | 0.276 MB/s | 0.301 MB/s | 0.516 MB/s | -8.3% | -46.5% |
| bs=10 sw=10 sl=64 | p50 | 21,034 us | 19,848 us | 11,663 us | +6.0% | +80.3% |
| bs=10 sw=10 sl=64 | p95 | 32,041 us | 27,930 us | 14,037 us | +14.7% | +128.3% |
| bs=10 sw=10 sl=64 | p99 | 32,041 us | 27,930 us | 18,666 us | +14.7% | +71.7% |
| bs=100 sw=10 sl=64 | throughput | 955 tuples/sec | 983 tuples/sec | 1,102 tuples/sec | -2.8% | -13.4% |
| bs=100 sw=10 sl=64 | MB/s | 0.583 MB/s | 0.6 MB/s | 0.673 MB/s | -2.8% | -13.3% |
| bs=100 sw=10 sl=64 | p50 | 103,880 us | 99,856 us | 91,441 us | +4.0% | +13.6% |
| bs=100 sw=10 sl=64 | p95 | 130,679 us | 115,903 us | 97,817 us | +12.7% | +33.6% |
| bs=100 sw=10 sl=64 | p99 | 130,679 us | 115,903 us | 108,384 us | +12.7% | +20.6% |
| bs=1000 sw=10 sl=64 | throughput | 1,126 tuples/sec | 1,064 tuples/sec | 1,128 tuples/sec | +5.8% | -0.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.687 MB/s | 0.649 MB/s | 0.688 MB/s | +5.9% | -0.2% |
| bs=1000 sw=10 sl=64 | p50 | 888,508 us | 942,066 us | 901,939 us | -5.7% | -1.5% |
| bs=1000 sw=10 sl=64 | p95 | 930,757 us | 1,006,395 us | 940,934 us | -7.5% | -1.1% |
| bs=1000 sw=10 sl=64 | p99 | 930,757 us | 1,006,395 us | 977,688 us | -7.5% | -4.8% |
Raw 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,442.23,200,128000,452,0.276,21034.25,32041.27,32041.27
1,100,10,64,20,2093.80,2000,1280000,955,0.583,103879.90,130678.84,130678.84
2,1000,10,64,20,17766.81,20000,12800000,1126,0.687,888508.04,930757.09,930757.09- Add length validation (max 128 chars) to validateDatasetName and fix its error message to accurately reflect the regex rules - Catch UNIQUE constraint violations (SQLState 23505) in both rename and create paths so concurrent races return 400 instead of 500 - Use ctx.fetchExists() instead of materializing full rows for duplicate checks, and use dataset.getDid instead of client-supplied modificator.did - Add client-side validation and error surfacing to the live rename UIs (card-item and list-item components) which are the actual rendered components users interact with - Use shared extractErrorMessage utility instead of inline error parsing - Fix DEFAULT_DATASET_NAME from "Untitled dataset" (contains space) to "Untitled-dataset" to comply with the new validation rules
Backend: - validateDatasetName now throws BadRequestException directly, eliminating the try/catch wrappers at both call sites - Extract failOnDuplicateDatasetName[T] helper that catches SQLState 23505 and maps to BadRequestException, replacing two identical 6-line catch blocks - Use jOOQ's built-in e.sqlState() instead of the manual Option(e.getCause).collect dance - Hoist regex to a class-level val (compiled once, not per call) - Merge redundant null/empty check into the regex check (regex + quantifier already rejects empty strings) Frontend: - Export validateDatasetName() from dataset.service.ts as the single source of truth for the name rule (regex + max length + error message) - All three components now call the shared validator instead of inlining the regex, magic number 128, and error string
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6426 +/- ##
============================================
+ Coverage 69.83% 70.30% +0.46%
- Complexity 3383 3402 +19
============================================
Files 1142 1142
Lines 44844 44876 +32
Branches 4949 4955 +6
============================================
+ Hits 31318 31551 +233
+ Misses 11874 11679 -195
+ Partials 1652 1646 -6
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
When a user enters an invalid dataset name, the input now reverts to the original name and exits editing mode, consistent with the behavior on backend rejection.
- On invalid dataset name: show error, revert to original name, exit editing mode (consistent with backend rejection behavior) - Fix import line lengths to comply with prettier's 120-char limit - Add unit tests for the shared validateDatasetName() function - Add dataset rename validation tests to card-item and list-item specs to cover all new code paths for 100% coverage on changed lines
The error handler now uses extractErrorMessage which extracts the
message property from Error instances ("boom") rather than showing the
old hardcoded "Update dataset name failed" string.
Add null and a 129-character string to the invalid-name test cases, covering both branches of validateDatasetName (null check and length check). The failOnDuplicateDatasetName catch block is a race-condition safety net that requires concurrent transactions to trigger and cannot be deterministically exercised in a single-threaded test.
… paths - Widen failOnDuplicateDatasetName to private[resource] so the spec can exercise it directly, since the race it guards cannot be triggered deterministically through the endpoint in a single-threaded test - Test all three paths of failOnDuplicateDatasetName: unique violation mapped to BadRequestException, non-23505 DataAccessException rethrown, and pass-through of the result on success - Cover the dataset rename error branch in card-item and list-item specs (error message surfaced, name reverted, editing mode exited)
…setName The catch clause's type test had a partial branch: exceptions that are not DataAccessException were never thrown through the helper. Throw an IllegalStateException to cover the fall-through path.
scoverage instruments a guarded catch pattern (case e: X if cond) as a multi-way branch with a synthetic arm that no test can reach, leaving a permanent partial on that line. Restructure to an unguarded catch with a plain if inside, whose branches are all deterministically exercised by the existing failOnDuplicateDatasetName tests. Also add a test for a DataAccessException whose cause carries no SQL state.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness gap in Texera’s dataset rename flow by enforcing the same name validation and per-owner uniqueness rules as dataset creation, and by adding a database-level uniqueness constraint to prevent concurrent races from reintroducing duplicates.
Changes:
- Enforce dataset-name validation + per-owner duplicate checks in the file-service rename endpoint, and translate DB unique-constraint violations into a user-facing 400 error.
- Add a DB migration that deduplicates existing
(owner_uid, name)collisions and then addsUNIQUE (owner_uid, name); reflect the constraint in the baseline DDL. - Update the Angular dashboard to validate dataset names client-side and surface backend error messages; add/extend unit tests accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sql/updates/28.sql | Deduplicates existing duplicate dataset names per owner and adds the (owner_uid, name) unique constraint. |
| sql/texera_ddl.sql | Adds UNIQUE (owner_uid, name) to the dataset table for fresh schema creation. |
| sql/changelog.xml | Registers migration 28.sql in the DB changelog. |
| frontend/src/app/dashboard/service/user/dataset/dataset.service.ts | Adds shared client-side dataset name validation and updates the default dataset name to a valid value. |
| frontend/src/app/dashboard/service/user/dataset/dataset.service.spec.ts | Adds unit tests for validateDatasetName. |
| frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.ts | Validates dataset rename input client-side and shows backend error details via extractErrorMessage. |
| frontend/src/app/dashboard/component/user/user-dataset/user-dataset-list-item/user-dataset-list-item.component.spec.ts | Adds tests for client-side rejection and backend-message surfacing during dataset rename. |
| frontend/src/app/dashboard/component/user/list-item/list-item.component.ts | Validates dataset renames in the shared list-item UI and surfaces backend error messages. |
| frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts | Adds tests covering invalid/valid dataset renames and rename failure behavior. |
| frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts | Applies the same dataset rename validation and improved error messaging in the card-item UI. |
| frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts | Adds tests for dataset rename validation and error handling in card-item. |
| file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala | Adds regression tests for rename validation, duplicate handling, DB uniqueness enforcement, and race-condition translation. |
| file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala | Enforces validation + duplicate checks on rename, wraps writes to translate unique-constraint violations into 400s, and aligns validation rules with DB limits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
xuang7
approved these changes
Jul 16, 2026
xuang7
left a comment
xuang7
left a comment
Contributor
There was a problem hiding this comment.
LGTM! One comment: 28.sql silently renames pre-existing duplicate datasets. Since this is a user-visible data change, and workflows that reference a renamed dataset by path may silently resolve to the surviving dataset afterward, we could consider adding a DO block with a RAISE NOTICE to make these renames visible.
xuang7
added
release/v1.2
xuang7
enabled auto-merge
July 16, 2026 23:55
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
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.
What changes were proposed in this PR?
The dataset rename endpoint previously skipped both checks that dataset creation enforces: name validation and the per owner duplicate name check. A rename could therefore produce two datasets with the same
(owner, name)pair, which breaks every lookup that resolves a dataset by(owner email, dataset name). File uploads and workflow file reads fail with aTooManyRowsExceptionfor both datasets.This PR adds the missing guards to the backend rename endpoint, and a
UNIQUE (owner_uid, name)constraint at the database level so that concurrent writes racing past the application-level check cannot reintroduce duplicates. The migration scriptsql/updates/28.sqldeduplicates pre-existing duplicates before adding the constraint (the oldest dataset keeps its name, later ones get a-<did>suffix). The frontend rename input now applies the same name validation client-side and shows the detailed reason from the backend when a rename is rejected, instead of the generic "Update dataset name failed".Some additional changes:
validateDatasetNamenow also enforces the 128-character column limitAny related issues, documentation, discussions?
Closes #6424
How was this PR tested?
Screen.Recording.2026-07-14.at.8.23.22.PM.mov
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code, Claude Fable 5