You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ZiyaZa (via GitHub)" <gi...@apache.org> on 2026/04/20 17:04:52 UTC
[PR] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
ZiyaZa opened a new pull request, #55430:
URL: https://github.com/apache/spark/pull/55430
### What changes were proposed in this pull request?
Added `numDeletedRows` metric for metadata-only DELETEs.
### Why are the changes needed?
For better visibility into what happened as a result of an DELETE query.
### Does this PR introduce _any_ user-facing change?
Yes.
### How was this patch tested?
Added metric value validation to most DELETE unit tests.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3113707327
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
I am not sure I fully understand this. I thought the goal was to allow connectors to expose a set of metrics (whatever those may be) as output without defining any specific custom metrics on the Spark side?
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
I am not sure I fully understand this.
I thought the goal was to allow connectors to expose a set of metrics (whatever those may be) as output without defining any specific custom metrics on the Spark side?
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3113672655
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TruncatableTable.java:
##########
@@ -34,4 +36,21 @@ public interface TruncatableTable extends Table {
* @since 3.2.0
*/
boolean truncateTable();
+
+ /**
+ * Returns an array of supported custom metrics with name and description.
+ * By default it returns empty array.
+ */
+ default CustomMetric[] supportedCustomMetrics() {
Review Comment:
Are these the metrics that will be routed to output of the commands?
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "ZiyaZa (via GitHub)" <gi...@apache.org>.
ZiyaZa commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3123896460
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
Spark will need to send this value to the command output when we implement that functionality.
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3123678672
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
hm. yea i also wondering, does this particular metric need to flow to Spark?
connector knows how many rows were removed by metadata-only delete and can just persist it in the summary? Is it needed in spark somewhere
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #55430: [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2
URL: https://github.com/apache/spark/pull/55430
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3124182941
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
got it, thanks
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #55430:
URL: https://github.com/apache/spark/pull/55430#issuecomment-5148666495
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "ZiyaZa (via GitHub)" <gi...@apache.org>.
ZiyaZa commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3116300782
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/metric/NumDeletedRowsMetric.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.
+ */
+
+package org.apache.spark.sql.connector.metric;
+
+import org.apache.spark.annotation.Evolving;
+
+/**
+ * Standard {@link CustomMetric} for the number of rows deleted by a DELETE or TRUNCATE
+ * operation. Connectors implementing
+ * {@link org.apache.spark.sql.connector.catalog.SupportsDeleteV2} or
+ * {@link org.apache.spark.sql.connector.catalog.TruncatableTable} can declare this metric from
+ * their {@code supportedCustomMetrics()} to have Spark surface the value uniformly in the SQL UI
+ * and elsewhere.
+ *
+ * @since 4.2.0
+ */
+@Evolving
+public class NumDeletedRowsMetric extends CustomSumMetric {
Review Comment:
Connectors can still expose a set of metrics, this is just one that Spark understands and will expose as command output. Spark needs to specify the name of the metric somehow for all connectors to use, otherwise each connector can come up with it's own naming scheme and Spark wouldn't know what to look for.
Here we could just have a String constant somewhere to store the expected metric name `numDeletedRows`. Instead I went with this class definition because it seems all connectors will want to use some class like this. But if I misunderstood, I can replace it with just a String constant.
--
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] [WIP][SPARK-56551][SQL] Add operation metrics for metadata-only DELETE queries in DSv2 [spark]
Posted by "ZiyaZa (via GitHub)" <gi...@apache.org>.
ZiyaZa commented on code in PR #55430:
URL: https://github.com/apache/spark/pull/55430#discussion_r3116314247
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TruncatableTable.java:
##########
@@ -34,4 +36,21 @@ public interface TruncatableTable extends Table {
* @since 3.2.0
*/
boolean truncateTable();
+
+ /**
+ * Returns an array of supported custom metrics with name and description.
+ * By default it returns empty array.
+ */
+ default CustomMetric[] supportedCustomMetrics() {
Review Comment:
Not to the output, just to plan node metrics. I took a similar approach to what we already had in couple of places (hence the refactored code in this PR). This allows connectors to expose any metrics that will be visible in the plan node.
As for the output of the command, Spark will only use the metrics it understands and expects. In this case, we have only `numDeletedRows` metric to be exposed as command output, and for this connectors need to use `NumDeletedRowsMetric`.
--
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