You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yaooqinn (via GitHub)" <gi...@apache.org> on 2026/03/07 06:13:32 UTC
[PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
yaooqinn opened a new pull request, #54671:
URL: https://github.com/apache/spark/pull/54671
### What changes were proposed in this pull request?
Replace the server-rendered PagedTable SQL listing with client-side DataTables fetching from the REST API, following the same pattern as SPARK-55837 (Environment page).
### Changes:
- **New file**: `sql-tab.js` - fetches from `/api/v1/applications/{appId}/sql/` and renders a single DataTable with all queries
- **Columns**: ID (link to detail), Status (BS5 badge), Description (truncated with tooltip), Submitted, Duration (formatted), Job IDs (links)
- **Status filter**: dropdown above the table to filter by RUNNING/COMPLETED/FAILED
- **Simplified `AllExecutionsPage.scala`**: minimal skeleton with loading spinner, DataTables header, and script includes
- **Removed**: `ExecutionPagedTable`, `ExecutionDataSource`, `ExecutionTableRowData` classes (server-side pagination no longer needed)
- **Updated tests**: adapted to verify the new skeleton page renders correctly
### Why are the changes needed?
The server-side PagedTable approach requires full page reloads for pagination, sorting, and filtering. The client-side DataTables approach provides:
- Instant search/filter/sort without server round-trips
- Consistent UX with the Environment page (SPARK-55837)
- Simpler Scala code (50 lines vs 590 lines)
### Does this PR introduce _any_ user-facing change?
Yes - the SQL tab now uses client-side DataTables with a unified table (instead of 3 separate collapsible tables), a status filter dropdown, and instant search/sort.
### How was this patch tested?
- `./dev/lint-js` - passed
- `./dev/scalastyle` - passed
- `./build/sbt sql/compile` - passed
- Updated `AllExecutionsPageSuite` to verify the new skeleton page rendering
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4091571855
https://github.com/yaooqinn/spark/actions/runs/23302901532 interesting this action is still running, but the 'Workflow run detection failed'
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4107484371
@yaooqinn
Could you add equivalent tests in a follow-up PR to cover the regression tests that were removed as I mentioned [here](https://github.com/apache/spark/pull/54671#pullrequestreview-3953664241) ?
- SPARK-27019: correctly display SQL page when event reordering happens
- SPARK-40834: prioritize errorMessage over job failure messages
- SPARK-42754: backward compatibility when rootExecutionId is missing from old event logs
- group sub executions: sub-executions grouped under root, and still displayed when root is missing
- sorting should be successful
Also, do you think the serverSide: true approach (like stagepage.js) can make it into Spark 4.2?
If both answers are yes, this PR itself looks good to me. Let's merge it first so the follow-up PRs have a base to build
on.
BTW, could you re-trigger GA?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak closed pull request #54671: [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables
URL: https://github.com/apache/spark/pull/54671
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4107787542
Thanks @sarutak for the thorough review and approval!
**Regression tests**: Yes, I will add them in a follow-up PR. Filed [SPARK-56137](https://issues.apache.org/jira/browse/SPARK-56137) to track:
- SPARK-27019: event reordering
- SPARK-40834: errorMessage priority over job failure
- SPARK-42754: backward compat for old event logs without rootExecutionId
- Sub-execution grouping (with missing root)
- Column sorting
**Server-side pagination**: Yes, planned for 4.2. Filed [SPARK-56140](https://issues.apache.org/jira/browse/SPARK-56140) — will follow the `stagepage.js` pattern with a new `/sql/sqlTable` REST endpoint.
Will re-trigger CI now.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4089562907
Thanks for the follow-up @sarutak! Addressed in latest push:
1. **Unused `goToSQLPage()`** — Removed entirely.
2. **Duplicate `CollectionConverters` imports** — Removed local imports; using the top-level import at line 21.
3. **`length=10000` limit** — Removed the hardcoded limit. The REST API now returns all executions (matching the old server-rendered behavior). `deferRender: true` in DataTables avoids rendering all DOM rows at once.
Re: server-side pagination — I agree that for ThriftServer/SparkConnect with 10K+ queries, a `serverSide: true` DataTables approach (like `stagepage.js`) would be ideal. That requires a new `/sql/sqlTable` REST endpoint with DataTables-compatible params (offset, length, sort, search). I think that could be a good follow-up JIRA. For now, removing the limit preserves the old behavior (show all).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #54671:
URL: https://github.com/apache/spark/pull/54671#discussion_r2957929640
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +49,39 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
Review Comment:
It seems this method isn't used anywhere.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +49,39 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td .stacktrace-details"""))
- .map(_.underlying.findElement(By.xpath("..")).getText).toList
+ eventually(timeout(10.seconds), interval(1.second)) {
+ assert(find(cssSelector("#sql-executions-table")).isDefined)
+ }
}
- private def findErrorSummaryOnSQLUI(): String = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.last
+ private def findErrorMessageViaAPI(): List[String] = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = Utils.tryWithResource(scala.io.Source.fromURL(url))(_.mkString)
+ val mapper = new com.fasterxml.jackson.databind.ObjectMapper()
+ val executions = mapper.readTree(json)
+ import scala.jdk.CollectionConverters._
+ executions.elements().asScala.toList
+ .filter(e => e.has("errorMessage") && !e.get("errorMessage").isNull)
+ .map(_.get("errorMessage").asText)
}
- private def findExecutionIDOnSQLUI(): Int = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.head.toInt
+ private def findExecutionIDViaAPI(): Int = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = Utils.tryWithResource(scala.io.Source.fromURL(url))(_.mkString)
+ val mapper = new com.fasterxml.jackson.databind.ObjectMapper()
+ val executions = mapper.readTree(json)
+ import scala.jdk.CollectionConverters._
Review Comment:
ditto.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +49,39 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td .stacktrace-details"""))
- .map(_.underlying.findElement(By.xpath("..")).getText).toList
+ eventually(timeout(10.seconds), interval(1.second)) {
+ assert(find(cssSelector("#sql-executions-table")).isDefined)
+ }
}
- private def findErrorSummaryOnSQLUI(): String = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.last
+ private def findErrorMessageViaAPI(): List[String] = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = Utils.tryWithResource(scala.io.Source.fromURL(url))(_.mkString)
+ val mapper = new com.fasterxml.jackson.databind.ObjectMapper()
+ val executions = mapper.readTree(json)
+ import scala.jdk.CollectionConverters._
Review Comment:
`scala.jdk.CollectionConverters._` is already imported at the top level in this file.
##########
sql/core/src/main/resources/org/apache/spark/sql/execution/ui/static/allexecutionspage.js:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* global $, uiRoot, appBasePath */
+/* eslint-disable no-unused-vars */
+
+function formatDurationSql(milliseconds) {
+ if (milliseconds < 100) return parseInt(milliseconds).toFixed(1) + " ms";
+ var seconds = milliseconds / 1000;
+ if (seconds < 1) return seconds.toFixed(1) + " s";
+ if (seconds < 60) return seconds.toFixed(0) + " s";
+ var minutes = seconds / 60;
+ if (minutes < 10) return minutes.toFixed(1) + " min";
+ if (minutes < 60) return minutes.toFixed(0) + " min";
+ var hours = minutes / 60;
+ return hours.toFixed(1) + " h";
+}
+
+function formatDateSql(dateStr) {
+ if (!dateStr) return "";
+ try { return new Date(dateStr).toLocaleString(); } catch (e) { return dateStr; }
+}
+function escapeHtml(str) {
+ if (!str) return str;
+ var div = document.createElement("div");
+ div.appendChild(document.createTextNode(str));
+ return div.innerHTML;
+}
+/* eslint-enable no-unused-vars */
+
+function createRESTEndPointForSQLTab(appId) {
+ var words = document.baseURI.split("/");
+ var ind = words.indexOf("proxy");
+ var newBaseURI;
+ if (ind > 0) {
+ appId = words[ind + 1];
+ newBaseURI = words.slice(0, ind + 2).join("/");
+ return newBaseURI + "/api/v1/applications/" + appId +
+ // Fetch up to 10000 executions; DataTables handles client-side pagination
+ // and deferRender:true avoids rendering all rows at once.
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
Review Comment:
Sorry, I didn't convey my meaning clearly. I'm concerned:
1. Strain on browsers
2. Even if showing the `10000` SQL executions doesn't strain on browsers, it seems to break the current behavior. Different from the current implementation, long-running applications (e.g. ThriftServer, Spark Connect) or History Server that exceed 10000 SQL executions will have data silently truncated with no indication to the user.
SQL execution count grows unboundedly over time, similar to tasks in `stagepage.js`. I'd suggest adopting the same `serverSide: true` pattern that `stagepage.js` uses for tasks.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4060766560
Thank you for pinging me, @yaooqinn !
I'll leave my comment next Monday.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4091468649
BTW, is the CI failing still for some reasons?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #54671:
URL: https://github.com/apache/spark/pull/54671#discussion_r2940193287
##########
sql/core/src/main/resources/org/apache/spark/sql/execution/ui/static/allexecutionspage.js:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* global $, uiRoot, appBasePath */
+/* eslint-disable no-unused-vars */
+
+function formatDurationSql(milliseconds) {
+ if (milliseconds < 100) return parseInt(milliseconds).toFixed(1) + " ms";
+ var seconds = milliseconds / 1000;
+ if (seconds < 1) return seconds.toFixed(1) + " s";
+ if (seconds < 60) return seconds.toFixed(0) + " s";
+ var minutes = seconds / 60;
+ if (minutes < 10) return minutes.toFixed(1) + " min";
+ if (minutes < 60) return minutes.toFixed(0) + " min";
+ var hours = minutes / 60;
+ return hours.toFixed(1) + " h";
+}
+
+function formatDateSql(dateStr) {
+ if (!dateStr) return "";
+ try { return new Date(dateStr).toLocaleString(); } catch (e) { return dateStr; }
+}
+/* eslint-enable no-unused-vars */
+
+function createRESTEndPointForSQLTab(appId) {
+ var words = document.baseURI.split("/");
+ var ind = words.indexOf("proxy");
+ var newBaseURI;
+ if (ind > 0) {
+ appId = words[ind + 1];
+ newBaseURI = words.slice(0, ind + 2).join("/");
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ ind = words.indexOf("history");
+ if (ind > 0) {
+ appId = words[ind + 1];
+ var attemptId = words[ind + 2];
+ newBaseURI = words.slice(0, ind).join("/");
+ if (isNaN(attemptId)) {
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ } else {
+ return newBaseURI + "/api/v1/applications/" + appId + "/" + attemptId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ }
+ return uiRoot + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+}
+
+function statusBadge(status) {
+ var cls = "bg-secondary";
+ if (status === "COMPLETED") cls = "bg-success";
+ else if (status === "RUNNING") cls = "bg-primary";
+ else if (status === "FAILED") cls = "bg-danger";
+ return '<span class="badge ' + cls + '">' + status + '</span>';
+}
+
+function jobIdLinks(ids) {
+ if (!ids || ids.length === 0) return "";
+ var basePath = uiRoot + appBasePath;
+ return ids.map(function (id) {
+ return '<a href="' + basePath + '/jobs/job/?id=' + id + '">' + id + '</a>';
+ }).join(", ");
+}
+
+function descriptionHtml(exec) {
+ var desc = exec.description || "";
+ var basePath = uiRoot + appBasePath;
+ var url = basePath + "/SQL/execution/?id=" + exec.id;
+ if (desc.length > 100) {
+ var short = desc.substring(0, 100) + "...";
+ return '<a href="' + url + '" title="' + desc.replace(/"/g, """) + '">' +
+ short + '</a>';
+ }
+ return '<a href="' + url + '">' + (desc || exec.id) + '</a>';
+}
+
+$.fn.dataTable.ext.search.push(function (settings, data) {
+ if (settings.nTable.id !== "sql-table") return true;
+ var sel = $("#status-filter").val();
+ return !sel || data[2] === sel;
+});
+
+$(document).ready(function () {
+ var appId = "";
+ var endPoint = createRESTEndPointForSQLTab(appId);
+
+
+
+
+
+ $.getJSON(endPoint, function (response) {
+ var tableData = response.map(function (exec) {
+ return [
+ exec.id,
+ exec.queryId || "",
+ exec.status,
+ exec.description || "",
+ exec.submissionTime || "",
+ exec.duration,
+ exec.runningJobIds || [],
+ exec.successJobIds || [],
+ exec.failedJobIds || [],
+ exec.errorMessage || "",
+ exec.rootExecutionId
+ ];
+ });
+
+ // Group sub-executions under their root execution
+ var subExecMap = {};
+ var rootRows = [];
+ tableData.forEach(function (row) {
+ var id = row[0];
+ var rootId = row[10];
+ if (rootId !== id && subExecMap[rootId] !== undefined) {
+ // This is a sub-execution — add to parent's children
+ subExecMap[rootId].push(row);
+ } else {
+ // Root execution
+ subExecMap[id] = [];
+ rootRows.push(row);
+ }
+ });
+ // Second pass: attach orphaned sub-executions that appeared before their root
+ tableData.forEach(function (row) {
+ var id = row[0];
+ var rootId = row[10];
+ if (rootId !== id && subExecMap[rootId] !== undefined
+ && subExecMap[rootId].indexOf(row) === -1) {
+ subExecMap[rootId].push(row);
+ }
+ });
+
+ var container = document.getElementById("sql-executions-table");
+ container.innerHTML =
+ '<select id="status-filter" class="form-select form-select-sm ' +
+ 'd-inline-block w-auto mb-2">' +
+ '<option value="">All Statuses</option>' +
+ '<option value="RUNNING">Running</option>' +
+ '<option value="COMPLETED">Completed</option>' +
+ '<option value="FAILED">Failed</option>' +
+ '</select>' +
+ '<table id="sql-table" class="table table-striped compact cell-border" ' +
+ 'style="width:100%"></table>';
+
+ var table = $("#sql-table").DataTable({
+ data: rootRows,
+ columns: [
+ {
+ title: "ID",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ var basePath = uiRoot + appBasePath;
+ return '<a href="' + basePath + '/SQL/execution/?id=' + data + '">' +
+ data + '</a>';
+ }
+ },
+ {
+ title: "Query ID",
+ render: function (data, type) {
+ if (type !== "display" || !data) return data;
+ return '<span title="' + data + '">' + data.substring(0, 8) + '...</span>';
+ }
+ },
+ {
+ title: "Status",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return statusBadge(data);
+ }
+ },
+ {
+ title: "Description",
+ render: function (data, type, row) {
+ if (type !== "display") return data;
+ return descriptionHtml({ id: row[0], description: data });
+ }
+ },
+ {
+ title: "Submitted",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return formatDateSql(data);
+ }
+ },
+ {
+ title: "Duration",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return formatDurationSql(data);
+ }
+ },
+ {
+ title: "Running Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Succeeded Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Failed Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Error Message",
+ render: function (data, type) {
+ if (type !== "display" || !data) return data;
+ if (data.length > 100) {
+ return '<span title="' + data.replace(/"/g, """) + '">' +
+ data.substring(0, 100) + '...</span>';
+ }
+ return data;
Review Comment:
Should `data` and `data.substring` be escaped using `escapeHtml` to prevent from XSS?
##########
sql/core/src/main/resources/org/apache/spark/sql/execution/ui/static/allexecutionspage.js:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* global $, uiRoot, appBasePath */
+/* eslint-disable no-unused-vars */
+
+function formatDurationSql(milliseconds) {
+ if (milliseconds < 100) return parseInt(milliseconds).toFixed(1) + " ms";
+ var seconds = milliseconds / 1000;
+ if (seconds < 1) return seconds.toFixed(1) + " s";
+ if (seconds < 60) return seconds.toFixed(0) + " s";
+ var minutes = seconds / 60;
+ if (minutes < 10) return minutes.toFixed(1) + " min";
+ if (minutes < 60) return minutes.toFixed(0) + " min";
+ var hours = minutes / 60;
+ return hours.toFixed(1) + " h";
+}
+
+function formatDateSql(dateStr) {
+ if (!dateStr) return "";
+ try { return new Date(dateStr).toLocaleString(); } catch (e) { return dateStr; }
+}
+/* eslint-enable no-unused-vars */
+
+function createRESTEndPointForSQLTab(appId) {
+ var words = document.baseURI.split("/");
+ var ind = words.indexOf("proxy");
+ var newBaseURI;
+ if (ind > 0) {
+ appId = words[ind + 1];
+ newBaseURI = words.slice(0, ind + 2).join("/");
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
Review Comment:
Current UI doesn't limit the number of queries right? Also, I'm concerned that fetching many entries might strain on browsers.
Can we make tables like `stagepage.js` does?
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +48,37 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td .stacktrace-details"""))
- .map(_.underlying.findElement(By.xpath("..")).getText).toList
+ Thread.sleep(5000)
Review Comment:
May I ask the reason why `Thread.sleep` is used here? Can we use `eventually` block instead?
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/AllExecutionsPageSuite.scala:
##########
@@ -56,7 +47,7 @@ abstract class AllExecutionsPageSuite extends SharedSparkSession with BeforeAndA
}
}
- test("SPARK-27019: correctly display SQL page when event reordering happens") {
+ test("render skeleton page with DataTables and script includes") {
Review Comment:
Should we prepend `SPARK-55875: `?
##########
sql/core/src/main/resources/org/apache/spark/sql/execution/ui/static/allexecutionspage.js:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* global $, uiRoot, appBasePath */
+/* eslint-disable no-unused-vars */
+
+function formatDurationSql(milliseconds) {
+ if (milliseconds < 100) return parseInt(milliseconds).toFixed(1) + " ms";
+ var seconds = milliseconds / 1000;
+ if (seconds < 1) return seconds.toFixed(1) + " s";
+ if (seconds < 60) return seconds.toFixed(0) + " s";
+ var minutes = seconds / 60;
+ if (minutes < 10) return minutes.toFixed(1) + " min";
+ if (minutes < 60) return minutes.toFixed(0) + " min";
+ var hours = minutes / 60;
+ return hours.toFixed(1) + " h";
+}
+
+function formatDateSql(dateStr) {
+ if (!dateStr) return "";
+ try { return new Date(dateStr).toLocaleString(); } catch (e) { return dateStr; }
+}
+/* eslint-enable no-unused-vars */
+
+function createRESTEndPointForSQLTab(appId) {
+ var words = document.baseURI.split("/");
+ var ind = words.indexOf("proxy");
+ var newBaseURI;
+ if (ind > 0) {
+ appId = words[ind + 1];
+ newBaseURI = words.slice(0, ind + 2).join("/");
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ ind = words.indexOf("history");
+ if (ind > 0) {
+ appId = words[ind + 1];
+ var attemptId = words[ind + 2];
+ newBaseURI = words.slice(0, ind).join("/");
+ if (isNaN(attemptId)) {
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ } else {
+ return newBaseURI + "/api/v1/applications/" + appId + "/" + attemptId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ }
+ return uiRoot + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+}
+
+function statusBadge(status) {
+ var cls = "bg-secondary";
+ if (status === "COMPLETED") cls = "bg-success";
+ else if (status === "RUNNING") cls = "bg-primary";
+ else if (status === "FAILED") cls = "bg-danger";
+ return '<span class="badge ' + cls + '">' + status + '</span>';
+}
+
+function jobIdLinks(ids) {
+ if (!ids || ids.length === 0) return "";
+ var basePath = uiRoot + appBasePath;
+ return ids.map(function (id) {
+ return '<a href="' + basePath + '/jobs/job/?id=' + id + '">' + id + '</a>';
+ }).join(", ");
+}
+
+function descriptionHtml(exec) {
+ var desc = exec.description || "";
+ var basePath = uiRoot + appBasePath;
+ var url = basePath + "/SQL/execution/?id=" + exec.id;
+ if (desc.length > 100) {
+ var short = desc.substring(0, 100) + "...";
+ return '<a href="' + url + '" title="' + desc.replace(/"/g, """) + '">' +
+ short + '</a>';
+ }
+ return '<a href="' + url + '">' + (desc || exec.id) + '</a>';
+}
+
+$.fn.dataTable.ext.search.push(function (settings, data) {
+ if (settings.nTable.id !== "sql-table") return true;
+ var sel = $("#status-filter").val();
+ return !sel || data[2] === sel;
+});
+
+$(document).ready(function () {
+ var appId = "";
+ var endPoint = createRESTEndPointForSQLTab(appId);
+
+
+
+
+
+ $.getJSON(endPoint, function (response) {
+ var tableData = response.map(function (exec) {
+ return [
+ exec.id,
+ exec.queryId || "",
+ exec.status,
+ exec.description || "",
+ exec.submissionTime || "",
+ exec.duration,
+ exec.runningJobIds || [],
+ exec.successJobIds || [],
+ exec.failedJobIds || [],
+ exec.errorMessage || "",
+ exec.rootExecutionId
+ ];
+ });
+
+ // Group sub-executions under their root execution
+ var subExecMap = {};
+ var rootRows = [];
+ tableData.forEach(function (row) {
+ var id = row[0];
+ var rootId = row[10];
+ if (rootId !== id && subExecMap[rootId] !== undefined) {
+ // This is a sub-execution — add to parent's children
+ subExecMap[rootId].push(row);
+ } else {
+ // Root execution
+ subExecMap[id] = [];
+ rootRows.push(row);
+ }
+ });
+ // Second pass: attach orphaned sub-executions that appeared before their root
+ tableData.forEach(function (row) {
+ var id = row[0];
+ var rootId = row[10];
+ if (rootId !== id && subExecMap[rootId] !== undefined
+ && subExecMap[rootId].indexOf(row) === -1) {
+ subExecMap[rootId].push(row);
+ }
+ });
+
+ var container = document.getElementById("sql-executions-table");
+ container.innerHTML =
+ '<select id="status-filter" class="form-select form-select-sm ' +
+ 'd-inline-block w-auto mb-2">' +
+ '<option value="">All Statuses</option>' +
+ '<option value="RUNNING">Running</option>' +
+ '<option value="COMPLETED">Completed</option>' +
+ '<option value="FAILED">Failed</option>' +
+ '</select>' +
+ '<table id="sql-table" class="table table-striped compact cell-border" ' +
+ 'style="width:100%"></table>';
+
+ var table = $("#sql-table").DataTable({
+ data: rootRows,
+ columns: [
+ {
+ title: "ID",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ var basePath = uiRoot + appBasePath;
+ return '<a href="' + basePath + '/SQL/execution/?id=' + data + '">' +
+ data + '</a>';
+ }
+ },
+ {
+ title: "Query ID",
+ render: function (data, type) {
+ if (type !== "display" || !data) return data;
+ return '<span title="' + data + '">' + data.substring(0, 8) + '...</span>';
+ }
+ },
+ {
+ title: "Status",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return statusBadge(data);
+ }
+ },
+ {
+ title: "Description",
+ render: function (data, type, row) {
+ if (type !== "display") return data;
+ return descriptionHtml({ id: row[0], description: data });
+ }
+ },
+ {
+ title: "Submitted",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return formatDateSql(data);
+ }
+ },
+ {
+ title: "Duration",
+ render: function (data, type) {
+ if (type !== "display") return data;
+ return formatDurationSql(data);
+ }
+ },
+ {
+ title: "Running Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Succeeded Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Failed Jobs",
+ orderable: false,
+ render: function (data, type) {
+ if (type !== "display") return data.join(",");
+ return jobIdLinks(data);
+ }
+ },
+ {
+ title: "Error Message",
+ render: function (data, type) {
+ if (type !== "display" || !data) return data;
+ if (data.length > 100) {
+ return '<span title="' + data.replace(/"/g, """) + '">' +
+ data.substring(0, 100) + '...</span>';
+ }
+ return data;
+ }
+ },
+ {
+ title: "Sub Executions",
+ orderable: false,
+ render: function (_data, type, row) {
+ var children = subExecMap[row[0]] || [];
+ if (type !== "display" || children.length === 0) return children.length || "";
+ return '<a href="#" class="toggle-sub-exec">' +
+ '+' + children.length + ' sub</a>';
+ }
+ }
+ ],
+ order: [[0, "desc"]],
+ pageLength: 20,
+ deferRender: true,
+ language: { search: "Search: " }
+ });
+
+ // Child row expansion for sub-executions
+ $("#sql-table tbody").on("click", "a.toggle-sub-exec", function (e) {
+ e.preventDefault();
+ var tr = $(this).closest("tr");
+ var dtRow = table.row(tr);
+ if (dtRow.child.isShown()) {
+ dtRow.child.hide();
+ tr.removeClass("shown");
+ } else {
+ var parentId = dtRow.data()[0];
+ var children = subExecMap[parentId] || [];
+ var html = '<table class="table table-sm table-bordered mb-0 ms-4">';
+ html += '<thead><tr><th>ID</th><th>Status</th><th>Description</th>' +
+ '<th>Duration</th><th>Succeeded Jobs</th></tr></thead><tbody>';
+ children.forEach(function (child) {
+ var basePath = uiRoot + appBasePath;
+ html += '<tr><td><a href="' + basePath + '/SQL/execution/?id=' + child[0] +
+ '">' + child[0] + '</a></td>';
+ html += '<td>' + statusBadge(child[2]) + '</td>';
+ html += '<td>' + (child[3] || '') + '</td>';
Review Comment:
Should `child[3]` also be escaped?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala:
##########
@@ -17,573 +17,32 @@
package org.apache.spark.sql.execution.ui
-import java.net.URLEncoder
-import java.nio.charset.StandardCharsets.UTF_8
-
-import scala.collection.mutable
-import scala.xml.{Node, NodeSeq}
+import scala.xml.Node
import jakarta.servlet.http.HttpServletRequest
-import org.apache.spark.JobExecutionStatus
-import org.apache.spark.internal.Logging
-import org.apache.spark.internal.config.UI.UI_SQL_GROUP_SUB_EXECUTION_ENABLED
-import org.apache.spark.sql.errors.QueryExecutionErrors
-import org.apache.spark.ui.{PagedDataSource, PagedTable, UIUtils, WebUIPage}
-import org.apache.spark.util.Utils
-
-private[ui] class AllExecutionsPage(parent: SQLTab) extends WebUIPage("") with Logging {
-
- private val sqlStore = parent.sqlStore
- private val groupSubExecutionEnabled = parent.conf.get(UI_SQL_GROUP_SUB_EXECUTION_ENABLED)
Review Comment:
Is `UI_SQL_GROUP_SUB_EXECUTION_ENABLED` now ignored?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4108052830
Merged to `master`. Thank you @yaooqinn and @dongjoon-hyun !
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4022374395
**Screenshots — SQL tab with DataTables:**
**All queries with status badges and columns:**
<img src="https://raw.githubusercontent.com/yaooqinn/spark/3186983eed9/docs/sql-tab-all.png" width="800">
**Status filter (Failed only):**
<img src="https://raw.githubusercontent.com/yaooqinn/spark/3186983eed9/docs/sql-tab-failed.png" width="800">
**Sub-executions expanded (CTAS creates parent + child executions):**
<img src="https://raw.githubusercontent.com/yaooqinn/spark/3186983eed9/docs/sql-tab-subexec.png" width="800">
**Search filter (instant search across all columns):**
<img src="https://raw.githubusercontent.com/yaooqinn/spark/3186983eed9/docs/sql-tab-search.png" width="800">
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #54671:
URL: https://github.com/apache/spark/pull/54671#discussion_r2941009039
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +48,37 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td .stacktrace-details"""))
- .map(_.underlying.findElement(By.xpath("..")).getText).toList
+ Thread.sleep(5000)
}
- private def findErrorSummaryOnSQLUI(): String = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.last
+ private def findErrorMessageViaAPI(): List[String] = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = scala.io.Source.fromURL(url).mkString
+ val mapper = new com.fasterxml.jackson.databind.ObjectMapper()
+ val executions = mapper.readTree(json)
+ import scala.jdk.CollectionConverters._
+ executions.elements().asScala.toList
+ .filter(e => e.has("errorMessage") && !e.get("errorMessage").isNull)
+ .map(_.get("errorMessage").asText)
}
- private def findExecutionIDOnSQLUI(): Int = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.head.toInt
+ private def findExecutionIDViaAPI(): Int = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = scala.io.Source.fromURL(url).mkString
Review Comment:
Same comment as https://github.com/apache/spark/pull/54671/changes#diff-525465a6c6c1382bf8165da748522ec2387581ebd397fe25f5aa72107a45752aR62
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4107788565
CI is already green on the latest push (run [23302901532](https://github.com/yaooqinn/spark/actions/runs/23302901532) ✅).
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #54671:
URL: https://github.com/apache/spark/pull/54671#discussion_r2940213083
##########
sql/core/src/main/resources/org/apache/spark/sql/execution/ui/static/allexecutionspage.js:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* global $, uiRoot, appBasePath */
+/* eslint-disable no-unused-vars */
+
+function formatDurationSql(milliseconds) {
+ if (milliseconds < 100) return parseInt(milliseconds).toFixed(1) + " ms";
+ var seconds = milliseconds / 1000;
+ if (seconds < 1) return seconds.toFixed(1) + " s";
+ if (seconds < 60) return seconds.toFixed(0) + " s";
+ var minutes = seconds / 60;
+ if (minutes < 10) return minutes.toFixed(1) + " min";
+ if (minutes < 60) return minutes.toFixed(0) + " min";
+ var hours = minutes / 60;
+ return hours.toFixed(1) + " h";
+}
+
+function formatDateSql(dateStr) {
+ if (!dateStr) return "";
+ try { return new Date(dateStr).toLocaleString(); } catch (e) { return dateStr; }
+}
+/* eslint-enable no-unused-vars */
+
+function createRESTEndPointForSQLTab(appId) {
+ var words = document.baseURI.split("/");
+ var ind = words.indexOf("proxy");
+ var newBaseURI;
+ if (ind > 0) {
+ appId = words[ind + 1];
+ newBaseURI = words.slice(0, ind + 2).join("/");
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ ind = words.indexOf("history");
+ if (ind > 0) {
+ appId = words[ind + 1];
+ var attemptId = words[ind + 2];
+ newBaseURI = words.slice(0, ind).join("/");
+ if (isNaN(attemptId)) {
+ return newBaseURI + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ } else {
+ return newBaseURI + "/api/v1/applications/" + appId + "/" + attemptId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+ }
+ }
+ return uiRoot + "/api/v1/applications/" + appId +
+ "/sql/?details=false&planDescription=false&offset=0&length=10000";
+}
+
+function statusBadge(status) {
+ var cls = "bg-secondary";
+ if (status === "COMPLETED") cls = "bg-success";
+ else if (status === "RUNNING") cls = "bg-primary";
+ else if (status === "FAILED") cls = "bg-danger";
+ return '<span class="badge ' + cls + '">' + status + '</span>';
+}
+
+function jobIdLinks(ids) {
+ if (!ids || ids.length === 0) return "";
+ var basePath = uiRoot + appBasePath;
+ return ids.map(function (id) {
+ return '<a href="' + basePath + '/jobs/job/?id=' + id + '">' + id + '</a>';
+ }).join(", ");
+}
+
+function descriptionHtml(exec) {
+ var desc = exec.description || "";
+ var basePath = uiRoot + appBasePath;
+ var url = basePath + "/SQL/execution/?id=" + exec.id;
+ if (desc.length > 100) {
+ var short = desc.substring(0, 100) + "...";
+ return '<a href="' + url + '" title="' + desc.replace(/"/g, """) + '">' +
+ short + '</a>';
+ }
+ return '<a href="' + url + '">' + (desc || exec.id) + '</a>';
Review Comment:
Similar to [this](https://github.com/apache/spark/pull/54671/changes#r2940193287), should returning values should be escaped?
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/UISeleniumSuite.scala:
##########
@@ -49,26 +48,37 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser {
getWebClient.setCssErrorHandler(new SparkUICssErrorHandler)
}
- private def findErrorMessageOnSQLUI(): List[String] = {
+ private def goToSQLPage(): Unit = {
val webUrl = spark.sparkContext.uiWebUrl
assert(webUrl.isDefined, "please turn on spark.ui.enabled")
go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td .stacktrace-details"""))
- .map(_.underlying.findElement(By.xpath("..")).getText).toList
+ Thread.sleep(5000)
}
- private def findErrorSummaryOnSQLUI(): String = {
- val webUrl = spark.sparkContext.uiWebUrl
- assert(webUrl.isDefined, "please turn on spark.ui.enabled")
- go to s"${webUrl.get}/SQL"
- findAll(cssSelector("""#failed-table td""")).map(_.text).toList.last
+ private def findErrorMessageViaAPI(): List[String] = {
+ val webUrl = spark.sparkContext.uiWebUrl.get
+ val url = s"$webUrl/api/v1/applications/${spark.sparkContext.applicationId}" +
+ "/sql/?details=false&planDescription=false"
+ val json = scala.io.Source.fromURL(url).mkString
Review Comment:
Should we use `tryWithResource` here?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4074252145
Thanks for the thorough review @sarutak! All 9 comments addressed in the latest push:
1. **XSS escaping** (comments #1, #2, #3): Added `escapeHtml()` helper using DOM `createTextNode` — applied to error messages, descriptions, and sub-execution child rows.
2. **`UI_SQL_GROUP_SUB_EXECUTION_ENABLED`** (#4): Config value is now passed via a hidden `<div data-value="...">` and read by JS to conditionally show the Sub Executions column.
3. **`Thread.sleep` → `eventually`** (#5): Replaced with `eventually(timeout(10.seconds), interval(1.second))` checking that `#sql-executions-table` is present.
4. **`tryWithResource`** (#6, #7): Both `Source.fromURL` calls now wrapped with `Utils.tryWithResource`.
5. **Fetch limit** (#8): Added comment explaining the `length=10000` limit. DataTables `deferRender: true` avoids rendering all rows at once, so browser performance should be fine for typical workloads.
6. **Test name prefix** (#9): Added `SPARK-55875:` prefix.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55875][UI] Switch SQL tab query listing to client-side DataTables [spark]
Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on PR #54671:
URL: https://github.com/apache/spark/pull/54671#issuecomment-4087973561
Hi @yaooqinn , have you considered my comment [here](https://github.com/apache/spark/pull/54671#pullrequestreview-3953664241)?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org