You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "malinjawi (via GitHub)" <gi...@apache.org> on 2026/10/18 11:35:00 UTC
[PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
malinjawi opened a new pull request, #13357:
URL: https://github.com/apache/gluten/pull/13357
## What changes are proposed in this pull request?
Part of #9003. This is the native half of #13165, split out to make review easier.
Delta's `IncrementMetric` counts how many rows an expression is evaluated on, and Velox already tracks that per expression (`ExprStats::numProcessedRows`). This adds the native side of counting it there:
- 32 fixed counter functions, `increment_metric_0` to `increment_metric_31`. Each one is a pass-through, marked non-deterministic so Velox never constant-folds or dictionary-peels it, and without default null behavior so null inputs still count.
- For a plan that calls a counter, `WholeStageResultIterator` forces `operator_track_expression_stats=true` and `expression.eval_simplified=false` after the user's overrides (simplified evaluation doesn't count rows), and exports the counters' processed-row counts with the operator metrics.
Nothing calls these functions yet, so plans and metrics don't change. #C_PR reads the counts on the JVM side, and #13165 makes the Delta transformers emit the calls.
## How was this patch tested?
`IncrementMetricFunctionTest`: pass-through including nulls, per-branch counts inside `if`, detecting counters in a plan, and that the forced configs keep counting under a user's `eval_simplified=true`.
## Was this patch authored or co-authored using generative AI tooling?
Generated-by: IBM Bob
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13357:
URL: https://github.com/apache/gluten/pull/13357#discussion_r4360646469
##########
cpp/velox/operators/functions/IncrementMetricFunction.cc:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+
+#include "operators/functions/IncrementMetricFunction.h"
+
+#include <memory>
+#include <vector>
+
+#include "velox/core/Expressions.h"
+#include "velox/core/QueryConfig.h"
+#include "velox/expression/EvalCtx.h"
+#include "velox/expression/FunctionSignature.h"
+#include "velox/expression/VectorFunction.h"
+
+namespace gluten {
+namespace {
+
+using namespace facebook::velox;
+
+/// Returns its only argument. See the header for why the name, not the body, carries the meaning.
+class IncrementMetricFunction : public exec::VectorFunction {
+ public:
+ void apply(
+ const SelectivityVector& rows,
+ std::vector<VectorPtr>& args,
+ const TypePtr& /*outputType*/,
+ exec::EvalCtx& context,
+ VectorPtr& result) const override {
+ VELOX_CHECK_EQ(args.size(), 1);
+ context.moveOrCopyResult(args[0], rows, result);
+ }
+};
+
+bool callsIncrementMetric(const core::ITypedExpr& expr) {
+ if (const auto* call = dynamic_cast<const core::CallTypedExpr*>(&expr)) {
+ if (isIncrementMetricFunction(call->name())) {
+ return true;
+ }
+ }
+ if (const auto* lambda = dynamic_cast<const core::LambdaTypedExpr*>(&expr)) {
+ return lambda->body() != nullptr && callsIncrementMetric(*lambda->body());
+ }
+ for (const auto& input : expr.inputs()) {
+ if (input != nullptr && callsIncrementMetric(*input)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+} // namespace
+
+std::string incrementMetricFunctionName(int32_t slot) {
+ return std::string(kIncrementMetricFunctionPrefix) + std::to_string(slot);
+}
+
+bool isIncrementMetricFunction(const std::string& functionName) {
+ return functionName.rfind(kIncrementMetricFunctionPrefix, 0) == 0;
+}
Review Comment:
`isIncrementMetricFunction` currently treats any name with the `increment_metric_` prefix as a counter (e.g., `increment_metric_foo`, `increment_metric_999`, or even `increment_metric_`). This can incorrectly (a) force query configs for plans that aren’t using the fixed slots and (b) export unrelated expression stats under `expressionStats`. Consider validating that the suffix is an integer and within `[0, kIncrementMetricFunctionSlots)` (and optionally rejecting negative / missing suffix).
##########
cpp/velox/operators/functions/IncrementMetricFunction.cc:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+
+#include "operators/functions/IncrementMetricFunction.h"
+
+#include <memory>
+#include <vector>
+
+#include "velox/core/Expressions.h"
+#include "velox/core/QueryConfig.h"
+#include "velox/expression/EvalCtx.h"
+#include "velox/expression/FunctionSignature.h"
+#include "velox/expression/VectorFunction.h"
+
+namespace gluten {
+namespace {
+
+using namespace facebook::velox;
+
+/// Returns its only argument. See the header for why the name, not the body, carries the meaning.
+class IncrementMetricFunction : public exec::VectorFunction {
+ public:
+ void apply(
+ const SelectivityVector& rows,
+ std::vector<VectorPtr>& args,
+ const TypePtr& /*outputType*/,
+ exec::EvalCtx& context,
+ VectorPtr& result) const override {
+ VELOX_CHECK_EQ(args.size(), 1);
+ context.moveOrCopyResult(args[0], rows, result);
+ }
+};
+
+bool callsIncrementMetric(const core::ITypedExpr& expr) {
+ if (const auto* call = dynamic_cast<const core::CallTypedExpr*>(&expr)) {
+ if (isIncrementMetricFunction(call->name())) {
+ return true;
+ }
+ }
+ if (const auto* lambda = dynamic_cast<const core::LambdaTypedExpr*>(&expr)) {
+ return lambda->body() != nullptr && callsIncrementMetric(*lambda->body());
+ }
+ for (const auto& input : expr.inputs()) {
+ if (input != nullptr && callsIncrementMetric(*input)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+} // namespace
+
+std::string incrementMetricFunctionName(int32_t slot) {
+ return std::string(kIncrementMetricFunctionPrefix) + std::to_string(slot);
+}
+
+bool isIncrementMetricFunction(const std::string& functionName) {
+ return functionName.rfind(kIncrementMetricFunctionPrefix, 0) == 0;
+}
+
+void registerIncrementMetricFunctions() {
+ // Not deterministic: keeps Velox from constant-folding or dictionary-peeling the call, either of
+ // which would change how many rows it is evaluated on. No default null behavior: a null input
+ // still counts, as it does on Spark.
+ const auto metadata = exec::VectorFunctionMetadataBuilder().deterministic(false).defaultNullBehavior(false).build();
+ for (int32_t slot = 0; slot < kIncrementMetricFunctionSlots; ++slot) {
+ exec::registerVectorFunction(
+ incrementMetricFunctionName(slot),
+ {exec::FunctionSignatureBuilder().typeVariable("T").returnType("T").argumentType("T").build()},
+ std::make_unique<IncrementMetricFunction>(),
+ metadata);
+ }
+}
+
+bool usesIncrementMetricFunctions(const core::PlanNode& plan) {
+ if (const auto* project = dynamic_cast<const core::ProjectNode*>(&plan)) {
+ for (const auto& projection : project->projections()) {
+ if (projection != nullptr && callsIncrementMetric(*projection)) {
+ return true;
+ }
+ }
+ } else if (const auto* filter = dynamic_cast<const core::FilterNode*>(&plan)) {
+ if (filter->filter() != nullptr && callsIncrementMetric(*filter->filter())) {
+ return true;
+ }
+ }
+ for (const auto& source : plan.sources()) {
+ if (source != nullptr && usesIncrementMetricFunctions(*source)) {
+ return true;
+ }
+ }
+ return false;
+}
Review Comment:
Plan detection only inspects `ProjectNode` and `FilterNode`. If counters ever appear in other plan nodes that carry expressions (e.g., aggregations, joins, table writes, etc.), the required configs won’t be forced and counts can silently be wrong. If the intent is truly limited to project/filter, consider making that constraint explicit (e.g., fail fast when a counter call is found elsewhere). Otherwise, consider extending detection to inspect expressions for additional node types (or using a more generic plan/expr visitor where available).
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -497,17 +499,26 @@ void WholeStageResultIterator::collectMetrics() {
customStats[customMetric.first] = folly::dynamic::object("sum", customMetric.second.sum)(
"count", customMetric.second.count)("min", customMetric.second.min)("max", customMetric.second.max);
}
+ // Rows processed per Delta counter function, keyed by its slot name. Project and filter
+ // operators export these only when the plan calls a counter.
+ folly::dynamic expressionStats = folly::dynamic::object();
+ for (const auto& exprStats : opStats->expressionStats) {
+ if (isIncrementMetricFunction(exprStats.first)) {
+ expressionStats[exprStats.first] = exprStats.second.numProcessedRows;
+ }
+ }
- operatorStats.push_back(folly::dynamic::object("inputRows", opStats->inputRows)(
- "inputVectors", opStats->inputVectors)("inputBytes", opStats->inputBytes)(
- "rawInputRows", opStats->rawInputRows)("rawInputBytes", opStats->rawInputBytes)(
- "outputRows", opStats->outputRows)("outputVectors", opStats->outputVectors)(
- "outputBytes", opStats->outputBytes)("cpuCount", opStats->cpuWallTiming.count)(
- "wallNanos", opStats->cpuWallTiming.wallNanos)("peakMemoryBytes", opStats->peakMemoryBytes)(
- "numMemoryAllocations", opStats->numMemoryAllocations)("spilledInputBytes", opStats->spilledInputBytes)(
- "spilledBytes", opStats->spilledBytes)("spilledRows", opStats->spilledRows)(
- "spilledPartitions", opStats->spilledPartitions)("spilledFiles", opStats->spilledFiles)(
- "physicalWrittenBytes", opStats->physicalWrittenBytes)("customStats", customStats));
+ operatorStats.push_back(
+ folly::dynamic::object("inputRows", opStats->inputRows)("inputVectors", opStats->inputVectors)(
+ "inputBytes", opStats->inputBytes)("rawInputRows", opStats->rawInputRows)(
+ "rawInputBytes", opStats->rawInputBytes)("outputRows", opStats->outputRows)(
+ "outputVectors", opStats->outputVectors)("outputBytes", opStats->outputBytes)(
+ "cpuCount", opStats->cpuWallTiming.count)("wallNanos", opStats->cpuWallTiming.wallNanos)(
+ "peakMemoryBytes", opStats->peakMemoryBytes)("numMemoryAllocations", opStats->numMemoryAllocations)(
+ "spilledInputBytes", opStats->spilledInputBytes)("spilledBytes", opStats->spilledBytes)(
+ "spilledRows", opStats->spilledRows)("spilledPartitions", opStats->spilledPartitions)(
+ "spilledFiles", opStats->spilledFiles)("physicalWrittenBytes", opStats->physicalWrittenBytes)(
+ "customStats", customStats)("expressionStats", expressionStats));
Review Comment:
This adds `expressionStats` to the operator metrics JSON unconditionally (often as an empty object). If any downstream consumer expects a strict schema, this may be a breaking change. If compatibility is a concern, consider only adding the `expressionStats` key when it’s non-empty (or when `trackExpressionStats_` is true), which also reduces payload size.
##########
cpp/velox/compute/WholeStageResultIterator.h:
##########
@@ -136,6 +136,9 @@ class WholeStageResultIterator : public SplitAwareColumnarBatchIterator {
std::unique_ptr<facebook::velox::exec::TaskCursor> cursor_;
facebook::velox::exec::Task* task_ = nullptr;
std::shared_ptr<const facebook::velox::core::PlanNode> veloxPlan_;
+ /// Whether the plan calls a Delta counter and so needs the query configs set by
+ /// setIncrementMetricQueryConfigs().
+ const bool trackExpressionStats_;
Review Comment:
`trackExpressionStats_` reads like a direct reflection of the query config, but the comment indicates it’s really ‘does this plan use IncrementMetric counters (and therefore require forcing config)?’. A name like `usesIncrementMetricFunctions_` or `needsIncrementMetricQueryConfigs_` would more precisely describe the stored value and reduce confusion.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on code in PR #13357:
URL: https://github.com/apache/gluten/pull/13357#discussion_r4365467506
##########
cpp/velox/operators/functions/IncrementMetricFunction.cc:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+
+#include "operators/functions/IncrementMetricFunction.h"
+
+#include <memory>
+#include <vector>
+
+#include "velox/core/Expressions.h"
+#include "velox/core/QueryConfig.h"
+#include "velox/expression/EvalCtx.h"
+#include "velox/expression/FunctionSignature.h"
+#include "velox/expression/VectorFunction.h"
+
+namespace gluten {
+namespace {
+
+using namespace facebook::velox;
+
+/// Returns its only argument. See the header for why the name, not the body, carries the meaning.
+class IncrementMetricFunction : public exec::VectorFunction {
+ public:
+ void apply(
+ const SelectivityVector& rows,
+ std::vector<VectorPtr>& args,
+ const TypePtr& /*outputType*/,
+ exec::EvalCtx& context,
+ VectorPtr& result) const override {
+ VELOX_CHECK_EQ(args.size(), 1);
+ context.moveOrCopyResult(args[0], rows, result);
+ }
+};
+
+bool callsIncrementMetric(const core::ITypedExpr& expr) {
+ if (const auto* call = dynamic_cast<const core::CallTypedExpr*>(&expr)) {
+ if (isIncrementMetricFunction(call->name())) {
+ return true;
+ }
+ }
+ if (const auto* lambda = dynamic_cast<const core::LambdaTypedExpr*>(&expr)) {
+ return lambda->body() != nullptr && callsIncrementMetric(*lambda->body());
+ }
+ for (const auto& input : expr.inputs()) {
+ if (input != nullptr && callsIncrementMetric(*input)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+} // namespace
+
+std::string incrementMetricFunctionName(int32_t slot) {
+ return std::string(kIncrementMetricFunctionPrefix) + std::to_string(slot);
+}
+
+bool isIncrementMetricFunction(const std::string& functionName) {
+ return functionName.rfind(kIncrementMetricFunctionPrefix, 0) == 0;
+}
+
+void registerIncrementMetricFunctions() {
+ // Not deterministic: keeps Velox from constant-folding or dictionary-peeling the call, either of
+ // which would change how many rows it is evaluated on. No default null behavior: a null input
+ // still counts, as it does on Spark.
+ const auto metadata = exec::VectorFunctionMetadataBuilder().deterministic(false).defaultNullBehavior(false).build();
+ for (int32_t slot = 0; slot < kIncrementMetricFunctionSlots; ++slot) {
+ exec::registerVectorFunction(
+ incrementMetricFunctionName(slot),
+ {exec::FunctionSignatureBuilder().typeVariable("T").returnType("T").argumentType("T").build()},
+ std::make_unique<IncrementMetricFunction>(),
+ metadata);
+ }
+}
+
+bool usesIncrementMetricFunctions(const core::PlanNode& plan) {
+ if (const auto* project = dynamic_cast<const core::ProjectNode*>(&plan)) {
+ for (const auto& projection : project->projections()) {
+ if (projection != nullptr && callsIncrementMetric(*projection)) {
+ return true;
+ }
+ }
+ } else if (const auto* filter = dynamic_cast<const core::FilterNode*>(&plan)) {
+ if (filter->filter() != nullptr && callsIncrementMetric(*filter->filter())) {
+ return true;
+ }
+ }
+ for (const auto& source : plan.sources()) {
+ if (source != nullptr && usesIncrementMetricFunctions(*source)) {
+ return true;
+ }
+ }
+ return false;
+}
Review Comment:
That's intentional. Gluten only puts counters in project and filter nodes, since the JVM emits them from Delta's project and filter transformers, so those are the only nodes to check. I made that explicit in the header comment.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on PR #13357:
URL: https://github.com/apache/gluten/pull/13357#issuecomment-6249432777
cc: @zhouyuan @rui-mo @felipepessoto
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on code in PR #13357:
URL: https://github.com/apache/gluten/pull/13357#discussion_r4366308385
##########
cpp/velox/compute/WholeStageResultIterator.h:
##########
@@ -136,6 +136,9 @@ class WholeStageResultIterator : public SplitAwareColumnarBatchIterator {
std::unique_ptr<facebook::velox::exec::TaskCursor> cursor_;
facebook::velox::exec::Task* task_ = nullptr;
std::shared_ptr<const facebook::velox::core::PlanNode> veloxPlan_;
+ /// Whether the plan calls a Delta counter and so needs the query configs set by
+ /// setIncrementMetricQueryConfigs().
+ const bool trackExpressionStats_;
Review Comment:
Makes sense, renamed to usesIncrementMetricFunctions_ in f5248ebfd.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on code in PR #13357:
URL: https://github.com/apache/gluten/pull/13357#discussion_r4365734425
##########
cpp/velox/compute/WholeStageResultIterator.h:
##########
@@ -136,6 +136,9 @@ class WholeStageResultIterator : public SplitAwareColumnarBatchIterator {
std::unique_ptr<facebook::velox::exec::TaskCursor> cursor_;
facebook::velox::exec::Task* task_ = nullptr;
std::shared_ptr<const facebook::velox::core::PlanNode> veloxPlan_;
+ /// Whether the plan calls a Delta counter and so needs the query configs set by
+ /// setIncrementMetricQueryConfigs().
+ const bool trackExpressionStats_;
Review Comment:
Makes sense, renamed to usesIncrementMetricFunctions_ in f5248ebfd.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-9003][VL] Add native counter functions for Delta's IncrementMetric [gluten]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on code in PR #13357:
URL: https://github.com/apache/gluten/pull/13357#discussion_r4361906182
##########
cpp/velox/compute/WholeStageResultIterator.cc:
##########
@@ -497,17 +499,26 @@ void WholeStageResultIterator::collectMetrics() {
customStats[customMetric.first] = folly::dynamic::object("sum", customMetric.second.sum)(
"count", customMetric.second.count)("min", customMetric.second.min)("max", customMetric.second.max);
}
+ // Rows processed per Delta counter function, keyed by its slot name. Project and filter
+ // operators export these only when the plan calls a counter.
+ folly::dynamic expressionStats = folly::dynamic::object();
+ for (const auto& exprStats : opStats->expressionStats) {
+ if (isIncrementMetricFunction(exprStats.first)) {
+ expressionStats[exprStats.first] = exprStats.second.numProcessedRows;
+ }
+ }
- operatorStats.push_back(folly::dynamic::object("inputRows", opStats->inputRows)(
- "inputVectors", opStats->inputVectors)("inputBytes", opStats->inputBytes)(
- "rawInputRows", opStats->rawInputRows)("rawInputBytes", opStats->rawInputBytes)(
- "outputRows", opStats->outputRows)("outputVectors", opStats->outputVectors)(
- "outputBytes", opStats->outputBytes)("cpuCount", opStats->cpuWallTiming.count)(
- "wallNanos", opStats->cpuWallTiming.wallNanos)("peakMemoryBytes", opStats->peakMemoryBytes)(
- "numMemoryAllocations", opStats->numMemoryAllocations)("spilledInputBytes", opStats->spilledInputBytes)(
- "spilledBytes", opStats->spilledBytes)("spilledRows", opStats->spilledRows)(
- "spilledPartitions", opStats->spilledPartitions)("spilledFiles", opStats->spilledFiles)(
- "physicalWrittenBytes", opStats->physicalWrittenBytes)("customStats", customStats));
+ operatorStats.push_back(
+ folly::dynamic::object("inputRows", opStats->inputRows)("inputVectors", opStats->inputVectors)(
+ "inputBytes", opStats->inputBytes)("rawInputRows", opStats->rawInputRows)(
+ "rawInputBytes", opStats->rawInputBytes)("outputRows", opStats->outputRows)(
+ "outputVectors", opStats->outputVectors)("outputBytes", opStats->outputBytes)(
+ "cpuCount", opStats->cpuWallTiming.count)("wallNanos", opStats->cpuWallTiming.wallNanos)(
+ "peakMemoryBytes", opStats->peakMemoryBytes)("numMemoryAllocations", opStats->numMemoryAllocations)(
+ "spilledInputBytes", opStats->spilledInputBytes)("spilledBytes", opStats->spilledBytes)(
+ "spilledRows", opStats->spilledRows)("spilledPartitions", opStats->spilledPartitions)(
+ "spilledFiles", opStats->spilledFiles)("physicalWrittenBytes", opStats->physicalWrittenBytes)(
+ "customStats", customStats)("expressionStats", expressionStats));
Review Comment:
Good point. It's added only when the operator has counter stats now (f5248ebfd), so other operators' JSON is unchanged. The JVM side already reads a missing key as empty.
##########
cpp/velox/operators/functions/IncrementMetricFunction.cc:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.
+ */
+
+#include "operators/functions/IncrementMetricFunction.h"
+
+#include <memory>
+#include <vector>
+
+#include "velox/core/Expressions.h"
+#include "velox/core/QueryConfig.h"
+#include "velox/expression/EvalCtx.h"
+#include "velox/expression/FunctionSignature.h"
+#include "velox/expression/VectorFunction.h"
+
+namespace gluten {
+namespace {
+
+using namespace facebook::velox;
+
+/// Returns its only argument. See the header for why the name, not the body, carries the meaning.
+class IncrementMetricFunction : public exec::VectorFunction {
+ public:
+ void apply(
+ const SelectivityVector& rows,
+ std::vector<VectorPtr>& args,
+ const TypePtr& /*outputType*/,
+ exec::EvalCtx& context,
+ VectorPtr& result) const override {
+ VELOX_CHECK_EQ(args.size(), 1);
+ context.moveOrCopyResult(args[0], rows, result);
+ }
+};
+
+bool callsIncrementMetric(const core::ITypedExpr& expr) {
+ if (const auto* call = dynamic_cast<const core::CallTypedExpr*>(&expr)) {
+ if (isIncrementMetricFunction(call->name())) {
+ return true;
+ }
+ }
+ if (const auto* lambda = dynamic_cast<const core::LambdaTypedExpr*>(&expr)) {
+ return lambda->body() != nullptr && callsIncrementMetric(*lambda->body());
+ }
+ for (const auto& input : expr.inputs()) {
+ if (input != nullptr && callsIncrementMetric(*input)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+} // namespace
+
+std::string incrementMetricFunctionName(int32_t slot) {
+ return std::string(kIncrementMetricFunctionPrefix) + std::to_string(slot);
+}
+
+bool isIncrementMetricFunction(const std::string& functionName) {
+ return functionName.rfind(kIncrementMetricFunctionPrefix, 0) == 0;
+}
Review Comment:
Agreed, fixed in f5248ebfd: only increment_metric_0 to increment_metric_31 match now. The test covers empty, out-of-range, negative, leading-zero and non-numeric names.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org