This is an automated email from the ASF dual-hosted git repository.
gian pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 5cfd114cb32 Fix metric handling in WindowOperatorQuery and
QueryLogicCompat toolchests. (#18168)
5cfd114cb32 is described below
commit 5cfd114cb32cdf6f370b1293ccfb39105c46e575
Author: Gian Merlino <[email protected]>
AuthorDate: Thu Jun 26 06:17:01 2025 -0700
Fix metric handling in WindowOperatorQuery and QueryLogicCompat toolchests.
(#18168)
Both toolchests were returning "new DefaultQueryMetrics" from their
"makeMetrics"
method. This caused two issues:
1) The "query(query)" method was not called, so standard dimensions like
dataSource,
type, etc, were not included.
2) Site-specific DefaultQueryMetrics customizations were not respected.
---
.../query/movingaverage/MovingAverageQueryTest.java | 4 +++-
.../apache/druid/query/QueryLogicCompatToolChest.java | 8 +++++---
.../operator/WindowOperatorQueryQueryRunnerFactory.java | 11 +++++++++--
.../operator/WindowOperatorQueryQueryToolChest.java | 12 ++++++++++--
.../operator/WindowOperatorQueryQueryToolChestTest.java | 4 +++-
.../apache/druid/server/ClientQuerySegmentWalker.java | 17 +++++++++++++----
.../java/org/apache/druid/server/QueryStackTests.java | 12 +++++++++---
.../apache/druid/sql/calcite/util/SqlTestFramework.java | 4 +++-
8 files changed, 55 insertions(+), 17 deletions(-)
diff --git
a/extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java
b/extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java
index caa402a142e..41bd3db31ad 100644
---
a/extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java
+++
b/extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java
@@ -53,6 +53,7 @@ import org.apache.druid.java.util.common.guava.Accumulators;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.guava.Sequences;
import org.apache.druid.query.BrokerParallelMergeConfig;
+import org.apache.druid.query.DefaultGenericQueryMetricsFactory;
import org.apache.druid.query.Query;
import org.apache.druid.query.QueryPlus;
import org.apache.druid.query.QueryRunner;
@@ -379,7 +380,8 @@ public class MovingAverageQueryTest extends
InitializedNullHandlingTest
null,
new CacheConfig(),
new SubqueryGuardrailHelper(null,
JvmUtils.getRuntimeInfo().getMaxHeapSizeBytes(), 1),
- new SubqueryCountStatsProvider()
+ new SubqueryCountStatsProvider(),
+ new DefaultGenericQueryMetricsFactory()
);
defineMocks();
diff --git
a/processing/src/main/java/org/apache/druid/query/QueryLogicCompatToolChest.java
b/processing/src/main/java/org/apache/druid/query/QueryLogicCompatToolChest.java
index c8c47d30e2e..e7ba317bc71 100644
---
a/processing/src/main/java/org/apache/druid/query/QueryLogicCompatToolChest.java
+++
b/processing/src/main/java/org/apache/druid/query/QueryLogicCompatToolChest.java
@@ -31,11 +31,13 @@ import java.util.Optional;
public class QueryLogicCompatToolChest extends QueryToolChest<Object,
Query<Object>>
{
- private RowSignature resultRowSignature;
+ private final RowSignature resultRowSignature;
+ private final GenericQueryMetricsFactory queryMetricsFactory;
- public QueryLogicCompatToolChest(RowSignature resultRowSignature)
+ public QueryLogicCompatToolChest(RowSignature resultRowSignature,
GenericQueryMetricsFactory queryMetricsFactory)
{
this.resultRowSignature = resultRowSignature;
+ this.queryMetricsFactory = queryMetricsFactory;
}
@Override
@@ -47,7 +49,7 @@ public class QueryLogicCompatToolChest extends
QueryToolChest<Object, Query<Obje
@Override
public QueryMetrics<? super Query<Object>> makeMetrics(Query<Object> query)
{
- return new DefaultQueryMetrics<>();
+ return queryMetricsFactory.makeMetrics(query);
}
@Override
diff --git
a/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryRunnerFactory.java
b/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryRunnerFactory.java
index 22688f9dc2b..515cfbdf7ad 100644
---
a/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryRunnerFactory.java
+++
b/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryRunnerFactory.java
@@ -20,6 +20,7 @@
package org.apache.druid.query.operator;
import com.google.common.base.Function;
+import com.google.inject.Inject;
import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.guava.Sequence;
@@ -39,7 +40,13 @@ import java.util.List;
public class WindowOperatorQueryQueryRunnerFactory implements
QueryRunnerFactory<RowsAndColumns, WindowOperatorQuery>
{
- public static final WindowOperatorQueryQueryToolChest TOOLCHEST = new
WindowOperatorQueryQueryToolChest();
+ public final WindowOperatorQueryQueryToolChest toolChest;
+
+ @Inject
+ public
WindowOperatorQueryQueryRunnerFactory(WindowOperatorQueryQueryToolChest
toolChest)
+ {
+ this.toolChest = toolChest;
+ }
@Override
public QueryRunner<RowsAndColumns> createRunner(Segment segment)
@@ -114,7 +121,7 @@ public class WindowOperatorQueryQueryRunnerFactory
implements QueryRunnerFactory
@Override
public QueryToolChest<RowsAndColumns, WindowOperatorQuery> getToolchest()
{
- return TOOLCHEST;
+ return toolChest;
}
}
diff --git
a/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChest.java
b/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChest.java
index e5c115fd2d7..cc03a5d5220 100644
---
a/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChest.java
+++
b/processing/src/main/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChest.java
@@ -23,12 +23,13 @@ import com.fasterxml.jackson.core.type.TypeReference;
import com.google.common.base.Function;
import com.google.common.base.Functions;
import com.google.common.collect.ImmutableMap;
+import com.google.inject.Inject;
import org.apache.druid.error.DruidException;
import org.apache.druid.frame.allocation.MemoryAllocatorFactory;
import org.apache.druid.java.util.common.guava.Sequence;
import org.apache.druid.java.util.common.guava.Sequences;
-import org.apache.druid.query.DefaultQueryMetrics;
import org.apache.druid.query.FrameSignaturePair;
+import org.apache.druid.query.GenericQueryMetricsFactory;
import org.apache.druid.query.QueryMetrics;
import org.apache.druid.query.QueryPlus;
import org.apache.druid.query.QueryRunner;
@@ -51,6 +52,13 @@ import java.util.function.Supplier;
public class WindowOperatorQueryQueryToolChest extends
QueryToolChest<RowsAndColumns, WindowOperatorQuery>
{
+ private final GenericQueryMetricsFactory queryMetricsFactory;
+
+ @Inject
+ public WindowOperatorQueryQueryToolChest(GenericQueryMetricsFactory
queryMetricsFactory)
+ {
+ this.queryMetricsFactory = queryMetricsFactory;
+ }
@Override
@SuppressWarnings("unchecked")
@@ -85,7 +93,7 @@ public class WindowOperatorQueryQueryToolChest extends
QueryToolChest<RowsAndCol
@Override
public QueryMetrics<? super WindowOperatorQuery>
makeMetrics(WindowOperatorQuery query)
{
- return new DefaultQueryMetrics<>();
+ return queryMetricsFactory.makeMetrics(query);
}
@Override
diff --git
a/processing/src/test/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChestTest.java
b/processing/src/test/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChestTest.java
index 2100b36e57d..c2b0ba85b16 100644
---
a/processing/src/test/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChestTest.java
+++
b/processing/src/test/java/org/apache/druid/query/operator/WindowOperatorQueryQueryToolChestTest.java
@@ -25,6 +25,7 @@ import org.apache.druid.frame.read.FrameReader;
import org.apache.druid.frame.testutil.FrameTestUtil;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.guava.Sequences;
+import org.apache.druid.query.DefaultGenericQueryMetricsFactory;
import org.apache.druid.query.Druids;
import org.apache.druid.query.FrameSignaturePair;
import org.apache.druid.query.QueryDataSource;
@@ -51,7 +52,8 @@ import java.util.List;
public class WindowOperatorQueryQueryToolChestTest extends
InitializedNullHandlingTest
{
- private final WindowOperatorQueryQueryToolChest toolchest = new
WindowOperatorQueryQueryToolChest();
+ private final WindowOperatorQueryQueryToolChest toolchest =
+ new
WindowOperatorQueryQueryToolChest(DefaultGenericQueryMetricsFactory.instance());
@Test
public void mergeResultsWithRowResultSerializationMode()
diff --git
a/server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
b/server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
index 4d284f442c3..5e02a550dd0 100644
--- a/server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
+++ b/server/src/main/java/org/apache/druid/server/ClientQuerySegmentWalker.java
@@ -45,6 +45,7 @@ import org.apache.druid.query.DruidMetrics;
import org.apache.druid.query.FluentQueryRunner;
import org.apache.druid.query.FrameBasedInlineDataSource;
import org.apache.druid.query.FrameSignaturePair;
+import org.apache.druid.query.GenericQueryMetricsFactory;
import org.apache.druid.query.GlobalTableDataSource;
import org.apache.druid.query.InlineDataSource;
import org.apache.druid.query.Query;
@@ -116,6 +117,7 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
private final CacheConfig cacheConfig;
private final SubqueryGuardrailHelper subqueryGuardrailHelper;
private final SubqueryCountStatsProvider subqueryStatsProvider;
+ private final GenericQueryMetricsFactory genericQueryMetricsFactory;
public ClientQuerySegmentWalker(
ServiceEmitter emitter,
@@ -129,7 +131,8 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
Cache cache,
CacheConfig cacheConfig,
SubqueryGuardrailHelper subqueryGuardrailHelper,
- SubqueryCountStatsProvider subqueryStatsProvider
+ SubqueryCountStatsProvider subqueryStatsProvider,
+ GenericQueryMetricsFactory genericQueryMetricsFactory
)
{
this.emitter = emitter;
@@ -144,6 +147,7 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
this.cacheConfig = cacheConfig;
this.subqueryGuardrailHelper = subqueryGuardrailHelper;
this.subqueryStatsProvider = subqueryStatsProvider;
+ this.genericQueryMetricsFactory = genericQueryMetricsFactory;
}
@Inject
@@ -159,7 +163,8 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
Cache cache,
CacheConfig cacheConfig,
SubqueryGuardrailHelper subqueryGuardrailHelper,
- SubqueryCountStatsProvider subqueryStatsProvider
+ SubqueryCountStatsProvider subqueryStatsProvider,
+ GenericQueryMetricsFactory genericQueryMetricsFactory
)
{
this(
@@ -174,7 +179,8 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
cache,
cacheConfig,
subqueryGuardrailHelper,
- subqueryStatsProvider
+ subqueryStatsProvider,
+ genericQueryMetricsFactory
);
}
@@ -397,7 +403,10 @@ public class ClientQuerySegmentWalker implements
QuerySegmentWalker
return toInlineDataSource(
subQuery,
queryResults,
- (QueryToolChest) new
QueryLogicCompatToolChest(subQuery.getResultRowSignature()),
+ (QueryToolChest) new QueryLogicCompatToolChest(
+ subQuery.getResultRowSignature(),
+ genericQueryMetricsFactory
+ ),
subqueryRowLimitAccumulator,
subqueryMemoryLimitAccumulator,
cannotMaterializeToFrames,
diff --git a/server/src/test/java/org/apache/druid/server/QueryStackTests.java
b/server/src/test/java/org/apache/druid/server/QueryStackTests.java
index c9974a2ab0e..451bdd8d3d9 100644
--- a/server/src/test/java/org/apache/druid/server/QueryStackTests.java
+++ b/server/src/test/java/org/apache/druid/server/QueryStackTests.java
@@ -62,6 +62,7 @@ import
org.apache.druid.query.metadata.SegmentMetadataQueryRunnerFactory;
import org.apache.druid.query.metadata.metadata.SegmentMetadataQuery;
import org.apache.druid.query.operator.WindowOperatorQuery;
import org.apache.druid.query.operator.WindowOperatorQueryQueryRunnerFactory;
+import org.apache.druid.query.operator.WindowOperatorQueryQueryToolChest;
import org.apache.druid.query.policy.NoopPolicyEnforcer;
import org.apache.druid.query.scan.ScanQuery;
import org.apache.druid.query.scan.ScanQueryConfig;
@@ -106,7 +107,6 @@ import org.junit.Assert;
import org.junit.rules.ExternalResource;
import javax.annotation.Nullable;
-
import java.io.IOException;
import java.util.Collections;
import java.util.Map;
@@ -187,7 +187,8 @@ public class QueryStackTests
injector.getInstance(Cache.class),
injector.getInstance(CacheConfig.class),
new SubqueryGuardrailHelper(null,
JvmUtils.getRuntimeInfo().getMaxHeapSizeBytes(), 1),
- new SubqueryCountStatsProvider()
+ new SubqueryCountStatsProvider(),
+ new DefaultGenericQueryMetricsFactory()
);
}
@@ -430,7 +431,12 @@ public class QueryStackTests
)
.put(GroupByQuery.class, groupByQueryRunnerFactory)
.put(TimeBoundaryQuery.class, new
TimeBoundaryQueryRunnerFactory(QueryRunnerTestHelper.NOOP_QUERYWATCHER))
- .put(WindowOperatorQuery.class, new
WindowOperatorQueryQueryRunnerFactory())
+ .put(
+ WindowOperatorQuery.class,
+ new WindowOperatorQueryQueryRunnerFactory(
+ new
WindowOperatorQueryQueryToolChest(DefaultGenericQueryMetricsFactory.instance())
+ )
+ )
.build();
}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java
b/sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java
index 79918324765..4cd08014bb3 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java
@@ -57,6 +57,7 @@ import org.apache.druid.java.util.common.io.Closer;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.java.util.http.client.HttpClient;
import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DefaultGenericQueryMetricsFactory;
import org.apache.druid.query.DruidProcessingConfig;
import org.apache.druid.query.GlobalTableDataSource;
import org.apache.druid.query.QueryRunnerFactoryConglomerate;
@@ -1145,7 +1146,8 @@ public class SqlTestFramework
injector.getInstance(Cache.class),
injector.getInstance(CacheConfig.class),
new SubqueryGuardrailHelper(null,
JvmUtils.getRuntimeInfo().getMaxHeapSizeBytes(), 1),
- new SubqueryCountStatsProvider()
+ new SubqueryCountStatsProvider(),
+ new DefaultGenericQueryMetricsFactory()
);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
