Re: [PR] fix(mcp): fix UserInfo validation and bytes encoding in MCP tools [superset]

["aminghadersohi (via GitHub)" <gi...@apache.org>]

aminghadersohi commented on code in PR #38697:
URL: https://github.com/apache/superset/pull/38697#discussion_r2980543490


##########
superset/mcp_service/sql_lab/tool/execute_sql.py:
##########
@@ -159,6 +159,28 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: Context) -> ExecuteSqlRes
         raise
 
 
+def _sanitize_bytes_values(rows: list[dict[str, Any]]) -> None:
+    """Sanitize bytes/memoryview values in rows for JSON serialization."""
+    for row in rows:
+        for key, value in row.items():
+            if isinstance(value, (bytes, memoryview)):
+                raw = bytes(value) if isinstance(value, memoryview) else value
+                try:
+                    row[key] = raw.decode("utf-8")
+                except (UnicodeDecodeError, AttributeError):
+                    row[key] = raw.hex()
+
+
+def _build_statement_data(df: Any) -> StatementData:
+    """Build StatementData from a DataFrame, sanitizing binary values."""
+    rows = df.to_dict(orient="records")
+    _sanitize_bytes_values(rows)
+    return StatementData(
+        rows=rows,
+        columns=[ColumnInfo(name=col, type=str(df[col].dtype)) for col in df.columns],

Review Comment:
   Fixed — changed from df[col].dtype to iterating data.dtypes.items() which handles duplicate column labels safely by accessing dtypes directly from the index.



##########
superset/sql/execution/executor.py:
##########
@@ -817,6 +817,21 @@ def _get_from_cache(self, sql: str, opts: QueryOptions) -> QueryResult | None:
 
         return None
 
+    @staticmethod
+    def _deserialize_cached_data(data: Any) -> Any:
+        """Deserialize cached statement data back to a DataFrame."""
+        import pandas as pd
+
+        if data is None:
+            return None
+        if isinstance(data, pd.DataFrame):
+            return data
+        if isinstance(data, dict) and "records" in data and "columns" in data:
+            return pd.DataFrame(data["records"], columns=data["columns"])
+        if isinstance(data, (bytes, memoryview)):
+            return None
+        return data

Review Comment:
   Fixed — added list handling (converts list[dict] back to DataFrame, matching the format from _store_in_cache), replaced the catch-all return data with return None for unrecognized types, and updated the return type annotation from Any to pd.DataFrame | None.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org