(texera) 01/01: fix(TablesPlot): join selected columns with a comma to emit valid Python (#6806)

[gi...@apache.org]

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-6806-00c16217d2942d05c2ffa18018000a7c1a44af75
in repository https://gitbox.apache.org/repos/asf/texera.git

commit 634e657b1b76201dc230811b73c8eba18a891fae
Author: Kary Zheng <15...@users.noreply.github.com>
AuthorDate: Wed Jul 22 18:21:39 2026 -0700

    fix(TablesPlot): join selected columns with a comma to emit valid Python (#6806)
    
    ### What changes were proposed in this PR?
    
    Fixes invalid generated Python in `TablesPlotOpDesc`. `getAttributes`
    built the selected-column list by joining the rendered column names with
    the literal `','`:
    
    ```scala
    private def getAttributes: String =
      includedColumns.map(c => pyb"""${c.attributeName}""").mkString("','")
    ```
    
    used as `table.dropna(subset=[$attributes])` and `table[[$attributes]]`.
    
    Each `pyb"""${c.attributeName}"""` renders to a runtime-decoded call
    `self.decode_python_template('<base64>')`. Joining those with the
    literal `','` places a string literal immediately after a function call,
    which is a **Python SyntaxError**, so Tables Plot fails to run for any
    input:
    
    ```python
    table = table.dropna(subset=[self.decode_python_template('YQ==')','self.decode_python_template('Yg==')])
    ```
    
    (`YQ==`/`Yg==` decode to `a`/`b`.)
    
    **Fix:** join with a plain comma:
    
    ```diff
    -  includedColumns.map(c => pyb"""${c.attributeName}""").mkString("','")
    +  includedColumns.map(c => pyb"""${c.attributeName}""").mkString(",")
    ```
    
    which yields the valid list:
    
    ```python
    subset=[self.decode_python_template('YQ=='),self.decode_python_template('Yg==')]
    ```
    
    ### Any related issues, documentation, discussions?
    
    Closes #6791
    
    ### How was this PR tested?
    
    Added a regression test to `TablesPlotOpDescSpec` that configures two
    columns, calls `generatePythonCode()`, and asserts:
    - the columns are comma-joined
    (`...('<b64>'),self.decode_python_template('<b64>')...`), and
    - the invalid `')','` sequence is absent.
    
    Both assertions fail on `main` and pass with this change.
    
    Ran the suite locally:
    
    ```
    sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.visualization.tablesChart.TablesPlotOpDescSpec"
    ...
    Tests: succeeded 5, failed 0, canceled 0, ignored 0, pending 0
    All tests passed.
    ```
    
    ### Was this PR authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Claude Opus 4.8)
    
    🤖 Generated with [Claude Code](https://claude.com/claude-code)
    
    ---------
    
    Co-authored-by: Claude Opus 4.8 (1M context) <no...@anthropic.com>
---
 .../operator/visualization/tablesChart/TablesPlotOpDesc.scala |  5 ++++-
 .../visualization/tablesChart/TablesPlotOpDescSpec.scala      | 11 +++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDesc.scala
index dcd4c39929..bf23016275 100644
--- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDesc.scala
+++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDesc.scala
@@ -36,7 +36,10 @@ class TablesPlotOpDesc extends PythonOperatorDescriptor {
   var includedColumns: List[TablesConfig] = List()
 
   private def getAttributes: String =
-    includedColumns.map(c => pyb"""${c.attributeName}""").mkString("','")
+    // Join with a plain comma: each column renders to a decode_python_template(...)
+    // call, so joining with the literal ',' would put a string right after a call
+    // and produce invalid Python.
+    includedColumns.map(c => pyb"""${c.attributeName}""").mkString(",")
 
   def manipulateTable(): PythonTemplateBuilder = {
     assert(includedColumns.nonEmpty, "Included Columns cannot be empty")
diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDescSpec.scala
index ea77252109..cc19113906 100644
--- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDescSpec.scala
+++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/tablesChart/TablesPlotOpDescSpec.scala
@@ -77,4 +77,15 @@ class TablesPlotOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matchers
     assert(carries(code, "col_two"))
     code should include("class TableChartOperator(UDFTableOperator)")
   }
+
+  it should "join multiple columns with a comma, not the literal ',' (valid Python)" in {
+    // Each column renders to a decode(...) call, so they must be comma-joined;
+    // joining with the literal ',' puts a string right after a call (invalid Python).
+    opDesc.includedColumns = List(column("col_one"), column("col_two"))
+    val code = opDesc.generatePythonCode()
+    code should include(
+      s"self.decode_python_template('${b64("col_one")}'),self.decode_python_template('${b64("col_two")}')"
+    )
+    code should not include "')','"
+  }
 }