aglinxinyuan opened a new pull request, #7875:
URL: https://github.com/apache/texera/pull/7875
### What changes were proposed in this PR?
**Root cause.** `DatasetSearchQueryBuilder` joins the owner row and then
never projects a column from it. `constructFromClause` writes the join, but the
select list comes entirely from `mappedResourceSchema`, and
`UnifiedResourceSchema.apply` defaults `userEmail` to `DSL.inline("")`
(`UnifiedResourceSchema:74`) — a slot the dataset schema never named. So
`toEntryImpl` reads `owner.getEmail` out of a record that has no owner in it.
Two steps turn the missing projection into a `null`:
```
schema slot userEmail = DSL.inline("") <- default, never overridden
|
SQL '' as email <- USER is joined, never read
|
translateRecord dedupes by ORIGINAL field, and jOOQ compares fields by
rendered SQL, so projectsOfWorkflow / userName / userEmail /
projectColor -- all DSL.inline("") -- collapse to ONE entry
keyed on the first of them
|
record no `email` column at all -> record.into(USER) = empty User
|
entry DashboardDataset.ownerEmail = null (not "")
```
**Fix.** Name the slot. One line in the schema; `toEntryImpl` is untouched.
```scala
ownerId = DATASET.OWNER_UID,
userEmail = USER.EMAIL,
did = DATASET.DID,
```
Before → after, on the rendered projection and the entry:
| | before | after |
|---|---|---|
| projection | `'' as email` | `texera_db.user.email as email` |
| `DashboardDataset.ownerEmail` | `null`, every row | the dataset owner's
address |
| `leftJoin(USER)` | joined, selected from, never read | read |
`WorkflowSearchQueryBuilder` is the sibling that shows the step that was
missed: it opts into the USER column it reads (`userName = USER.NAME`, plus
`USER.NAME` in `getGroupByFields`), and it already filters on `USER.EMAIL` for
the `owners` query param — the email was reachable through the join all along,
only the projection was absent. `HubResource:316` and file-service's
`/dataset/list` both populate the same field correctly; dataset search was the
one producer that did not.
Scope of the impact, stated precisely because it is narrower than it looks:
`DashboardEntry.ownerEmail` (`dashboard-entry.ts:132`) receives the null, and
no frontend code reads that field for datasets today, so no screen was visibly
wrong. It was a trap rather than a broken page —
`dataset-selection-modal.component.ts:127` builds the storage logical path
`/${ResourceType.Dataset}/${ownerEmail}/${name}/${version}` out of a
`DashboardDataset`, and only escapes `/dataset/null/...` because it lists via
file-service.
### Any related issues, documentation, discussions?
Closes #7874
### How was this PR tested?
`DatasetSearchQueryBuilderSpec` goes from 18 tests to 21, reusing the
fixture and lakeFS loopback stub #7855 built.
| test | what it pins |
|---|---|
| `carry the owner's email address` | the value on the entry — the assertion
that kills the mutant below |
| `take the email from the dataset's owner, not from the caller` | fetched
as `otherUid`, who reaches the dataset only because it is public, so a lookup
that echoed the signed-in caller back would fail |
| `project every dataset column under the alias its schema slot names`
(extended) | `user.email as email` in the SELECT, so a slot dropped back to a
literal fails at the projection instead of as a null three layers down |
| `stay union-compatible with the workflow and project branches` | new — see
below |
The mutation #7855 recorded as surviving now dies. Replacing
`record.into(USER).into(classOf[User])` with a fresh `User`:
| | before this PR | after |
|---|---|---|
| `new User` mutant | survives the whole suite (equivalent mutant, given the
defect) | `succeeded 19, failed 2` — both owner-email tests |
Verified in order, one sbt JVM each:
| run | result |
|---|---|
| new tests against the unfixed builder | `succeeded 18, failed 3, canceled
0` — projection missing `user.email`, plus `null was not equal to
"[email protected]"` twice |
| with the fix | `succeeded 21, failed 0, canceled 0` |
| fix + `new User` mutant | `succeeded 19, failed 2, canceled 0` |
| the 7 dashboard suites together | `Suites: completed 7, Tests: succeeded
108, failed 0, canceled 0` |
| `scalafmtCheckAll` + `Test/scalafixAll --check` | clean |
```bash
sbt "WorkflowExecutionService/testOnly
org.apache.texera.web.resource.dashboard.DatasetSearchQueryBuilderSpec"
```
Two notes for a reviewer:
**The union test earns its place.** This change swaps a column in one branch
of the three-way `unionAll` that `DashboardResource.searchAllResources` builds
for the dashboard's default view, from an untyped `''` literal to `varchar`.
Nothing in the repo executed that union, so a type incompatibility would have
shipped as a runtime failure on the default view with nothing failing to
compile. The new test executes it.
**`canceled` is the failure mode to watch, and it moved.** The stub binds
the configured port (`localhost:8000`) rather than an ephemeral one, because
`LakeFSStorageClient.apiClient` is a `lazy val` capturing
`StorageConfig.lakefsEndpoint` once per JVM and amber runs every suite in one
unforked JVM. If something else holds that port — a local `bin/local-dev.sh up`
— the stub-dependent tests cancel, and that count goes from 4 to 6 with the
owner-email pair added. A cancel is quiet: sbt prints `All tests passed` at
exit 0. Measured here, with the port held and `size` mutated to `0L`, the run
reports `succeeded 15, failed 0, canceled 6` and still exits green — so the
owner-email pair is disarmed alongside the rest of the `toEntry` half. The
class comment carries these numbers and I updated them. CI has no lakeFS in
this job, so it is deterministic there.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-5)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
