From ff2fe9f11299318df82502172a8a979fba839272 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 31 May 2026 02:14:50 -0700 Subject: [PATCH 1/5] added require uid : Integer --- .../ExecutionsMetadataPersistService.scala | 8 +++++--- .../texera/web/service/WorkflowService.scala | 2 +- .../ExecutionsMetadataPersistServiceSpec.scala | 17 ++++++----------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala index b9b29dff72f..7659e2b6972 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala @@ -46,17 +46,19 @@ object ExecutionsMetadataPersistService extends LazyLogging { * This method inserts a new entry of a workflow execution in the database and returns the generated eId * * @param workflowId the given workflow - * @param uid user id that initiated the execution + * @param uid user id that initiated the execution; must be non-null (uid is NOT NULL) * @return generated execution ID */ def insertNewExecution( workflowId: WorkflowIdentity, - uid: Option[Integer], + uid: Integer, executionName: String, environmentVersion: String, computingUnitId: Integer ): ExecutionIdentity = { + // uid is NOT NULL; reject up front instead of a cryptic jOOQ violation. + require(uid != null, "uid must be provided to persist a new workflow execution") // first retrieve the latest version of this workflow val vid = getLatestVersion(workflowId.id.toInt) val newExecution = new WorkflowExecutions() @@ -64,7 +66,7 @@ object ExecutionsMetadataPersistService extends LazyLogging { newExecution.setName(executionName) } newExecution.setVid(vid) - newExecution.setUid(uid.orNull) + newExecution.setUid(uid) newExecution.setStartingTime(new Timestamp(System.currentTimeMillis())) newExecution.setEnvironmentVersion(environmentVersion) diff --git a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala index 809faf6a520..925b736f201 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala @@ -204,7 +204,7 @@ class WorkflowService( workflowContext.executionId = ExecutionsMetadataPersistService.insertNewExecution( workflowContext.workflowId, - uidOpt, + uidOpt.orNull, req.executionName, convertToJson(req.engineVersion), req.computingUnitId diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala index 5c7a687728f..26578fda2a3 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala @@ -166,7 +166,7 @@ class ExecutionsMetadataPersistServiceSpec "insertNewExecution" should "insert a row tied to the latest workflow version" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "named-execution", environmentVersion = "env-1", computingUnitId = seededCuid @@ -185,7 +185,7 @@ class ExecutionsMetadataPersistServiceSpec it should "skip setName when executionName is the empty string" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "", environmentVersion = "env-2", computingUnitId = seededCuid @@ -198,17 +198,12 @@ class ExecutionsMetadataPersistServiceSpec stored.getName shouldBe "Untitled Execution" } - it should "throw a DB constraint violation when uid is None" in { - // The method signature accepts Option[Integer] for uid and calls - // `newExecution.setUid(uid.orNull)`, but workflow_executions.uid is - // NOT NULL per texera_ddl.sql, so passing None propagates a jOOQ - // DataAccessException. Pinning the current behavior so a future fix — - // either tightening the signature to a required Integer or making the - // column nullable — breaks the spec deliberately. See follow-up bug. - val ex = intercept[org.jooq.exception.DataAccessException] { + it should "reject a null uid up front with a clear error rather than a jOOQ violation" in { + // uid is NOT NULL, so require(...) rejects null before the insert. + val ex = intercept[IllegalArgumentException] { ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - None, + null, executionName = "anonymous", environmentVersion = "env-3", computingUnitId = seededCuid From f5144c04168442777e3e3fd54cc61c5f7e2c990c Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 31 May 2026 14:03:41 -0700 Subject: [PATCH 2/5] fix(amber): raise on None uid in insertNewExecution instead of writing null --- .../service/ExecutionsMetadataPersistService.scala | 13 ++++++++----- .../apache/texera/web/service/WorkflowService.scala | 2 +- .../ExecutionsMetadataPersistServiceSpec.scala | 10 +++++----- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala index 7659e2b6972..f84a3a6b137 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala @@ -46,19 +46,17 @@ object ExecutionsMetadataPersistService extends LazyLogging { * This method inserts a new entry of a workflow execution in the database and returns the generated eId * * @param workflowId the given workflow - * @param uid user id that initiated the execution; must be non-null (uid is NOT NULL) + * @param uid user id that initiated the execution; required (uid is NOT NULL) * @return generated execution ID */ def insertNewExecution( workflowId: WorkflowIdentity, - uid: Integer, + uid: Option[Integer], executionName: String, environmentVersion: String, computingUnitId: Integer ): ExecutionIdentity = { - // uid is NOT NULL; reject up front instead of a cryptic jOOQ violation. - require(uid != null, "uid must be provided to persist a new workflow execution") // first retrieve the latest version of this workflow val vid = getLatestVersion(workflowId.id.toInt) val newExecution = new WorkflowExecutions() @@ -66,7 +64,12 @@ object ExecutionsMetadataPersistService extends LazyLogging { newExecution.setName(executionName) } newExecution.setVid(vid) - newExecution.setUid(uid) + // uid is NOT NULL in the DB; reject a missing uid rather than writing null. + newExecution.setUid( + uid.getOrElse( + throw new IllegalArgumentException("uid is required to persist a workflow execution") + ) + ) newExecution.setStartingTime(new Timestamp(System.currentTimeMillis())) newExecution.setEnvironmentVersion(environmentVersion) diff --git a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala index 925b736f201..809faf6a520 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala @@ -204,7 +204,7 @@ class WorkflowService( workflowContext.executionId = ExecutionsMetadataPersistService.insertNewExecution( workflowContext.workflowId, - uidOpt.orNull, + uidOpt, req.executionName, convertToJson(req.engineVersion), req.computingUnitId diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala index 26578fda2a3..a42aff1c3b0 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala @@ -166,7 +166,7 @@ class ExecutionsMetadataPersistServiceSpec "insertNewExecution" should "insert a row tied to the latest workflow version" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - testUid, + Some(testUid), executionName = "named-execution", environmentVersion = "env-1", computingUnitId = seededCuid @@ -185,7 +185,7 @@ class ExecutionsMetadataPersistServiceSpec it should "skip setName when executionName is the empty string" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - testUid, + Some(testUid), executionName = "", environmentVersion = "env-2", computingUnitId = seededCuid @@ -198,12 +198,12 @@ class ExecutionsMetadataPersistServiceSpec stored.getName shouldBe "Untitled Execution" } - it should "reject a null uid up front with a clear error rather than a jOOQ violation" in { - // uid is NOT NULL, so require(...) rejects null before the insert. + it should "raise on a None uid rather than writing null to a NOT NULL column" in { + // uid is NOT NULL, so a None is rejected before the insert. val ex = intercept[IllegalArgumentException] { ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - null, + None, executionName = "anonymous", environmentVersion = "env-3", computingUnitId = seededCuid From f1fcc6db2ce714027f017b782c00155bfc3d5d96 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 31 May 2026 16:28:18 -0700 Subject: [PATCH 3/5] switched from using Option to explicit Integer for uid --- .../ExecutionsMetadataPersistService.scala | 23 ++++++++++++------- .../texera/web/service/WorkflowService.scala | 2 +- ...ExecutionsMetadataPersistServiceSpec.scala | 21 +++++++++++++---- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala index f84a3a6b137..3d0878033b5 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala @@ -22,6 +22,7 @@ package org.apache.texera.web.service import com.typesafe.scalalogging.LazyLogging import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.dao.SqlServer +import org.jooq.exception.DataAccessException import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowExecutionsDao import org.apache.texera.dao.jooq.generated.tables.pojos.WorkflowExecutions import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowVersionResource._ @@ -52,7 +53,7 @@ object ExecutionsMetadataPersistService extends LazyLogging { def insertNewExecution( workflowId: WorkflowIdentity, - uid: Option[Integer], + uid: Integer, executionName: String, environmentVersion: String, computingUnitId: Integer @@ -64,19 +65,25 @@ object ExecutionsMetadataPersistService extends LazyLogging { newExecution.setName(executionName) } newExecution.setVid(vid) - // uid is NOT NULL in the DB; reject a missing uid rather than writing null. - newExecution.setUid( - uid.getOrElse( - throw new IllegalArgumentException("uid is required to persist a workflow execution") - ) - ) + newExecution.setUid(uid) newExecution.setStartingTime(new Timestamp(System.currentTimeMillis())) newExecution.setEnvironmentVersion(environmentVersion) // Set computing unit ID if provided newExecution.setCuid(computingUnitId) - workflowExecutionsDao.insert(newExecution) + try { + workflowExecutionsDao.insert(newExecution) + } catch { + // A NOT NULL column (e.g. uid, vid, cuid) was null. Postgres reports this + // as SQLState 23502; surface a readable message instead of the raw + // jOOQ/JDBC stack trace, while preserving the original as the cause. + case e: DataAccessException if e.sqlState() == "23502" => + throw new IllegalArgumentException( + "Cannot persist workflow execution: a required field (uid, vid, or cuid) was null.", + e + ) + } ExecutionIdentity(newExecution.getEid.longValue()) } diff --git a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala index 809faf6a520..925b736f201 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala @@ -204,7 +204,7 @@ class WorkflowService( workflowContext.executionId = ExecutionsMetadataPersistService.insertNewExecution( workflowContext.workflowId, - uidOpt, + uidOpt.orNull, req.executionName, convertToJson(req.engineVersion), req.computingUnitId diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala index a42aff1c3b0..1d67fa03310 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala @@ -166,7 +166,7 @@ class ExecutionsMetadataPersistServiceSpec "insertNewExecution" should "insert a row tied to the latest workflow version" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "named-execution", environmentVersion = "env-1", computingUnitId = seededCuid @@ -185,7 +185,7 @@ class ExecutionsMetadataPersistServiceSpec it should "skip setName when executionName is the empty string" in { val id = ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - Some(testUid), + testUid, executionName = "", environmentVersion = "env-2", computingUnitId = seededCuid @@ -198,18 +198,29 @@ class ExecutionsMetadataPersistServiceSpec stored.getName shouldBe "Untitled Execution" } - it should "raise on a None uid rather than writing null to a NOT NULL column" in { - // uid is NOT NULL, so a None is rejected before the insert. + it should "let the DB NOT NULL constraint reject a null uid (surfaced as a readable error)" in { + // No more pre-insert require: a null uid is passed straight to jOOQ and the + // DB's NOT NULL constraint rejects it. insertNewExecution catches the + // resulting DataAccessException (SQLState 23502) and rethrows a readable + // IllegalArgumentException. Assert both the readable message and that the + // original DB exception is preserved as the cause, and that nothing was + // written. + val before = workflowExecutionsDao.fetchByVid(seededVid).size() + val ex = intercept[IllegalArgumentException] { ExecutionsMetadataPersistService.insertNewExecution( WorkflowIdentity(testWid.toLong), - None, + null, // uid is NOT NULL in the DB executionName = "anonymous", environmentVersion = "env-3", computingUnitId = seededCuid ) } ex.getMessage should include("uid") + ex.getCause shouldBe a[org.jooq.exception.DataAccessException] + + // The failed insert must not have persisted a row. + workflowExecutionsDao.fetchByVid(seededVid).size() shouldBe before } // -- tryGetExistingExecution ------------------------------------------------ From 24e5bb84e6aa2096b8c685e9d35cf3378d9ffb90 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Sun, 31 May 2026 17:34:11 -0700 Subject: [PATCH 4/5] removed .orNull --- .../org/apache/texera/web/service/WorkflowService.scala | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala index 925b736f201..0044c959678 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala @@ -192,6 +192,13 @@ class WorkflowService( val (uidOpt, userEmailOpt) = userOpt.map(user => (user.getUid, user.getEmail)).unzip + // uid is NOT NULL in the DB; fail early here rather than letting the insert fail downstream. + val uid = uidOpt.getOrElse( + throw new IllegalArgumentException( + "Cannot start execution: a user id (uid) is required but none was provided." + ) + ) + val workflowContext: WorkflowContext = createWorkflowContext() var controllerConf = ControllerConfig.default @@ -204,7 +211,7 @@ class WorkflowService( workflowContext.executionId = ExecutionsMetadataPersistService.insertNewExecution( workflowContext.workflowId, - uidOpt.orNull, + uid, req.executionName, convertToJson(req.engineVersion), req.computingUnitId From 9035618b4e5f2ffd0b3f0be8abb6b5f87b6325ca Mon Sep 17 00:00:00 2001 From: "Matthew B." Date: Mon, 1 Jun 2026 12:18:22 -0700 Subject: [PATCH 5/5] Clarify uid parameter requirement in documentation Updated documentation to clarify that uid is required. Signed-off-by: Matthew B. --- .../texera/web/service/ExecutionsMetadataPersistService.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala index 3d0878033b5..bafe7a178a5 100644 --- a/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala +++ b/amber/src/main/scala/org/apache/texera/web/service/ExecutionsMetadataPersistService.scala @@ -47,7 +47,7 @@ object ExecutionsMetadataPersistService extends LazyLogging { * This method inserts a new entry of a workflow execution in the database and returns the generated eId * * @param workflowId the given workflow - * @param uid user id that initiated the execution; required (uid is NOT NULL) + * @param uid user id that initiated the execution * @return generated execution ID */