This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8333-662387be647006356ed45e45ab988140bf178b94 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 5bc7f49a75bcaee257fe24bf7dfd91a83224f9d0 Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Sep 27 22:08:33 2026 +0000 chore(workflow-operator): remove the unreferenced PropertyNameConstants (#8333) ### What changes were proposed in this PR? `PropertyNameConstants` declares 21 constants; only two are referenced anywhere outside its own spec. This removes the other 19. Pure deletion, no behaviour change: **−95 lines**. `LogicalOp` uses `OPERATOR_ID` and `OPERATOR_VERSION` as `@JsonProperty` annotation arguments — those are live and stay. Each of the other 19 has exactly three references, all inside `PropertyNameConstantsSpec`. ### History | | | | --- | --- | | **Introduced by** | the initial amber import, commit `41a8a92017` (2020-08-20) — predates the PR workflow | | **Usage removed by** | **never adopted** — operator descriptors always spelled their JSON keys as raw string literals, and the wire format is anchored elsewhere (`@JsonTypeInfo(property = "operatorType")` in `LogicalOp`, plus literals in `OperatorMetadataGenerator` and `WorkflowResource`) | These constants were never the single source of truth they were meant to be, which is why deleting them cannot change the serialized format. > Reviewer note: `TABLE_NAME` **is** included. Its only qualified references are in the spec; the bare `TABLE_NAME` hits elsewhere are an unrelated SQL placeholder in `sql/misc/tweets.sql`. The spec keeps its three tests, narrowed to the two surviving constants — including the distinctness and whitespace checks, which still hold. ### Any related issues, documentation, discussions? Closes #8330 ### How was this PR tested? Existing tests only — this PR adds none; it narrows the assertions to the constants that remain. Locally, from the repo root with Java 17: - `sbt "WorkflowExecutionService/Test/compile"` — success. - `sbt "WorkflowOperator/testOnly *PropertyNameConstantsSpec"` — 3 tests, all pass. - `sbt scalafmtCheckAll "scalafixAll --check"` — clean. Verification, re-runnable by a reviewer: ``` git grep -oh "PropertyNameConstants\.[A-Z_]*" -- . ':!*PropertyNameConstants.scala' | sort | uniq -c ``` Before this change every constant shows exactly 3 hits (its spec assertions) except `OPERATOR_ID` and `OPERATOR_VERSION`, which show 4 — the extra one being `LogicalOp`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --- .../operator/metadata/PropertyNameConstants.scala | 24 ------- .../metadata/PropertyNameConstantsSpec.scala | 75 +--------------------- 2 files changed, 2 insertions(+), 97 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala index 52bb041469..01c02a6e3e 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstants.scala @@ -27,29 +27,5 @@ package org.apache.texera.amber.operator.metadata */ object PropertyNameConstants { // logical plan property names final val OPERATOR_ID = "operatorID" - final val OPERATOR_TYPE = "operatorType" - final val ORIGIN_OPERATOR_ID = "origin" - final val DESTINATION_OPERATOR_ID = "destination" - final val OPERATOR_LIST = "operators" - final val OPERATOR_LINK_LIST = "links" final val OPERATOR_VERSION = "operatorVersion" - // common operator property names - final val ATTRIBUTE_NAMES = "attributes" - final val ATTRIBUTE_NAME = "attribute" - final val RESULT_ATTRIBUTE_NAME = "resultAttribute" - final val SPAN_LIST_NAME = "spanListName" - final val TABLE_NAME = "tableName" - - // physical plan property names - final val WORKFLOW_ID = "workflowID" - final val EXECUTION_ID = "executionID" - final val PARALLELIZABLE = "parallelizable" - final val LOCATION_PREFERENCE = "locationPreference" - final val PARTITION_REQUIREMENT = "partitionRequirement" - // derivePartition is a function type that cannot be serialized - final val INPUT_PORTS = "inputPorts" - final val OUTPUT_PORTS = "outputPorts" - // propagateSchema is a function type that cannot be serialized - final val IS_ONE_TO_MANY_OP = "isOneToManyOp" - final val SUGGESTED_WORKER_NUM = "suggestedWorkerNum" } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala index 9afd3b7d0b..455837efe2 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/metadata/PropertyNameConstantsSpec.scala @@ -30,42 +30,9 @@ class PropertyNameConstantsSpec extends AnyFlatSpec { "PropertyNameConstants logical-plan keys" should "have their canonical String values" in { assert(PropertyNameConstants.OPERATOR_ID == "operatorID") - assert(PropertyNameConstants.OPERATOR_TYPE == "operatorType") - assert(PropertyNameConstants.ORIGIN_OPERATOR_ID == "origin") - assert(PropertyNameConstants.DESTINATION_OPERATOR_ID == "destination") - assert(PropertyNameConstants.OPERATOR_LIST == "operators") - assert(PropertyNameConstants.OPERATOR_LINK_LIST == "links") assert(PropertyNameConstants.OPERATOR_VERSION == "operatorVersion") } - // --------------------------------------------------------------------------- - // Common operator-property keys - // --------------------------------------------------------------------------- - - "PropertyNameConstants common-property keys" should "have their canonical String values" in { - assert(PropertyNameConstants.ATTRIBUTE_NAMES == "attributes") - assert(PropertyNameConstants.ATTRIBUTE_NAME == "attribute") - assert(PropertyNameConstants.RESULT_ATTRIBUTE_NAME == "resultAttribute") - assert(PropertyNameConstants.SPAN_LIST_NAME == "spanListName") - assert(PropertyNameConstants.TABLE_NAME == "tableName") - } - - // --------------------------------------------------------------------------- - // Physical-plan keys - // --------------------------------------------------------------------------- - - "PropertyNameConstants physical-plan keys" should "have their canonical String values" in { - assert(PropertyNameConstants.WORKFLOW_ID == "workflowID") - assert(PropertyNameConstants.EXECUTION_ID == "executionID") - assert(PropertyNameConstants.PARALLELIZABLE == "parallelizable") - assert(PropertyNameConstants.LOCATION_PREFERENCE == "locationPreference") - assert(PropertyNameConstants.PARTITION_REQUIREMENT == "partitionRequirement") - assert(PropertyNameConstants.INPUT_PORTS == "inputPorts") - assert(PropertyNameConstants.OUTPUT_PORTS == "outputPorts") - assert(PropertyNameConstants.IS_ONE_TO_MANY_OP == "isOneToManyOp") - assert(PropertyNameConstants.SUGGESTED_WORKER_NUM == "suggestedWorkerNum") - } - // --------------------------------------------------------------------------- // Distinctness — no two constants alias to the same string // --------------------------------------------------------------------------- @@ -73,26 +40,7 @@ class PropertyNameConstantsSpec extends AnyFlatSpec { "PropertyNameConstants" should "have all constants distinct (no accidental aliases)" in { val all = List( PropertyNameConstants.OPERATOR_ID, - PropertyNameConstants.OPERATOR_TYPE, - PropertyNameConstants.ORIGIN_OPERATOR_ID, - PropertyNameConstants.DESTINATION_OPERATOR_ID, - PropertyNameConstants.OPERATOR_LIST, - PropertyNameConstants.OPERATOR_LINK_LIST, - PropertyNameConstants.OPERATOR_VERSION, - PropertyNameConstants.ATTRIBUTE_NAMES, - PropertyNameConstants.ATTRIBUTE_NAME, - PropertyNameConstants.RESULT_ATTRIBUTE_NAME, - PropertyNameConstants.SPAN_LIST_NAME, - PropertyNameConstants.TABLE_NAME, - PropertyNameConstants.WORKFLOW_ID, - PropertyNameConstants.EXECUTION_ID, - PropertyNameConstants.PARALLELIZABLE, - PropertyNameConstants.LOCATION_PREFERENCE, - PropertyNameConstants.PARTITION_REQUIREMENT, - PropertyNameConstants.INPUT_PORTS, - PropertyNameConstants.OUTPUT_PORTS, - PropertyNameConstants.IS_ONE_TO_MANY_OP, - PropertyNameConstants.SUGGESTED_WORKER_NUM + PropertyNameConstants.OPERATOR_VERSION ) assert(all.distinct.size == all.size, s"duplicate constant value(s) in: $all") } @@ -100,26 +48,7 @@ class PropertyNameConstantsSpec extends AnyFlatSpec { it should "carry no leading/trailing whitespace on any constant" in { val all = List( PropertyNameConstants.OPERATOR_ID, - PropertyNameConstants.OPERATOR_TYPE, - PropertyNameConstants.ORIGIN_OPERATOR_ID, - PropertyNameConstants.DESTINATION_OPERATOR_ID, - PropertyNameConstants.OPERATOR_LIST, - PropertyNameConstants.OPERATOR_LINK_LIST, - PropertyNameConstants.OPERATOR_VERSION, - PropertyNameConstants.ATTRIBUTE_NAMES, - PropertyNameConstants.ATTRIBUTE_NAME, - PropertyNameConstants.RESULT_ATTRIBUTE_NAME, - PropertyNameConstants.SPAN_LIST_NAME, - PropertyNameConstants.TABLE_NAME, - PropertyNameConstants.WORKFLOW_ID, - PropertyNameConstants.EXECUTION_ID, - PropertyNameConstants.PARALLELIZABLE, - PropertyNameConstants.LOCATION_PREFERENCE, - PropertyNameConstants.PARTITION_REQUIREMENT, - PropertyNameConstants.INPUT_PORTS, - PropertyNameConstants.OUTPUT_PORTS, - PropertyNameConstants.IS_ONE_TO_MANY_OP, - PropertyNameConstants.SUGGESTED_WORKER_NUM + PropertyNameConstants.OPERATOR_VERSION ) all.foreach(c => assert(c == c.trim, s"constant has surrounding whitespace: '$c'")) }
