aglinxinyuan commented on code in PR #7875:
URL: https://github.com/apache/texera/pull/7875#discussion_r3894986226
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala:
##########
@@ -491,14 +492,37 @@ class DatasetSearchQueryBuilderSpec
sql should include("dataset.is_downloadable as is_dataset_downloadable")
sql should include("dataset_user_access.privilege as user_dataset_access")
sql should include("dataset.cover_image as cover_image")
+ // The one projected column that is not a DATASET column. It is also the
slot this schema used to
+ // leave at its `DSL.inline("")` default, which is why it gets an
assertion of its own rather
+ // than trusting the entry-level test: a slot dropped back to a literal
renders `'' as email`
+ // here, and the failure names the projection instead of a null three
layers downstream.
+ sql should include("user.email as email")
Review Comment:
Not changed — the assertion already passes (verified: `succeeded 24, failed
0`). `sqlFor` lower-cases and strips quoting, and `should include` is a
substring match, so `user.email as email` matches the rendered
`texera_db.user.email as email`; every neighbouring assertion in this test is
unqualified in exactly the same way (`dataset.name as resourcename`), so
qualifying only this one would make it the odd one out.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala:
##########
@@ -491,14 +492,37 @@ class DatasetSearchQueryBuilderSpec
sql should include("dataset.is_downloadable as is_dataset_downloadable")
sql should include("dataset_user_access.privilege as user_dataset_access")
sql should include("dataset.cover_image as cover_image")
+ // The one projected column that is not a DATASET column. It is also the
slot this schema used to
+ // leave at its `DSL.inline("")` default, which is why it gets an
assertion of its own rather
+ // than trusting the entry-level test: a slot dropped back to a literal
renders `'' as email`
+ // here, and the failure names the projection instead of a null three
layers downstream.
+ sql should include("user.email as email")
+ }
+
+ it should "stay union-compatible with the workflow and project branches" in {
+ // `DashboardResource.searchAllResources` stacks the three builders with
`unionAll` for a
+ // resourceType of "" — the dashboard's default view — so every branch
must project the same
+ // aliases in the same order with types Postgres will unify. Each slot one
builder fills is a
+ // literal placeholder in the other two (`'' as email` here until
`userEmail` was named), which
+ // makes the contract easy to break from inside a single builder and
impossible to break loudly:
+ // nothing fails to compile, and the mismatch surfaces only as a failed
query at runtime. This is
+ // the only test that executes the union; the rest of the suite renders or
fetches one branch.
+ val union = WorkflowSearchQueryBuilder
+ .constructQuery(uid, params(), includePublic = true)
+ .unionAll(ProjectSearchQueryBuilder.constructQuery(uid, params(),
includePublic = true))
+ .unionAll(DatasetSearchQueryBuilder.constructQuery(uid, params(),
includePublic = true))
+
+ // Both seeded datasets are public, so both reach `uid`; no workflow or
project rows are seeded.
+ // The row count is incidental — that Postgres accepts the union at all is
the assertion.
+ getDSLContext.fetch(union).size() shouldBe 2
Review Comment:
Done — the count now derives from the fixture: added a `seededDids` val next
to `sizedDid`/`goneDid` and the assertion reads `shouldBe seededDids.size`, so
adding a seeded dataset no longer silently breaks this test. Kept it as a count
rather than a does-not-throw check, since a real assertion still catches a
branch that stops returning rows.
_🤖 Addressed by [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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
