You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2023/11/24 07:12:08 UTC
(ignite) branch master updated: IGNITE-20950 SQL Calcite: Fix NPE on write query plan to performance statistics - Fixes #11067.
This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 141cc3699d3 IGNITE-20950 SQL Calcite: Fix NPE on write query plan to performance statistics - Fixes #11067.
141cc3699d3 is described below
commit 141cc3699d3ba04c99fd074f85efc067fa5e11f8
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Fri Nov 24 10:10:29 2023 +0300
IGNITE-20950 SQL Calcite: Fix NPE on write query plan to performance statistics - Fixes #11067.
Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
---
.../query/calcite/prepare/PlanExtractor.java | 4 --
.../integration/SqlDiagnosticIntegrationTest.java | 60 ++++++++++++++++++++++
.../FilePerformanceStatisticsWriter.java | 3 ++
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanExtractor.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanExtractor.java
index 4c1273222a6..e350895ac34 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanExtractor.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanExtractor.java
@@ -50,10 +50,6 @@ public class PlanExtractor {
/** */
public String extract(IgniteRel rel) {
- // Currently, plan required only for preformance statistics, skip it if performance statistics disabled.
- if (!perfStatProc.enabled())
- return null;
-
if (QueryUtils.INCLUDE_SENSITIVE)
return RelOptUtil.toString(rel, SqlExplainLevel.ALL_ATTRIBUTES);
else {
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java
index f005d69c479..2274741bbc9 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlDiagnosticIntegrationTest.java
@@ -32,6 +32,7 @@ import java.util.Set;
import java.util.UUID;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicIntegerArray;
import java.util.concurrent.atomic.AtomicLong;
@@ -147,6 +148,8 @@ public class SqlDiagnosticIntegrationTest extends AbstractBasicIntegrationTest {
super.afterTest();
stopAllGrids();
+
+ cleanPerformanceStatisticsDir();
}
/** */
@@ -430,6 +433,63 @@ public class SqlDiagnosticIntegrationTest extends AbstractBasicIntegrationTest {
assertEquals(5L, rowsScanned.get());
}
+ /** */
+ @Test
+ public void testPerformanceStatisticsEnableAfterQuery() throws Exception {
+ cleanPerformanceStatisticsDir();
+
+ String qry = "SELECT * FROM table(system_range(1, 1000))";
+
+ sql(grid(0), qry);
+
+ startCollectStatistics();
+
+ AtomicInteger finishQryCnt = new AtomicInteger();
+ grid(0).context().query().runningQueryManager().registerQueryFinishedListener(q -> finishQryCnt.incrementAndGet());
+
+ sql(grid(0), qry);
+
+ assertTrue(GridTestUtils.waitForCondition(() -> finishQryCnt.get() == 1, 1_000L));
+
+ AtomicInteger qryCnt = new AtomicInteger();
+ AtomicBoolean hasPlan = new AtomicBoolean();
+
+ stopCollectStatisticsAndRead(new AbstractPerformanceStatisticsTest.TestHandler() {
+ @Override public void query(
+ UUID nodeId,
+ GridCacheQueryType type,
+ String text,
+ long id,
+ long qryStartTime,
+ long duration,
+ boolean success
+ ) {
+ qryCnt.incrementAndGet();
+
+ assertEquals(grid(0).localNode().id(), nodeId);
+ assertEquals(SQL_FIELDS, type);
+ assertTrue(success);
+ }
+
+ @Override public void queryProperty(
+ UUID nodeId,
+ GridCacheQueryType type,
+ UUID qryNodeId,
+ long id,
+ String name,
+ String val
+ ) {
+ if ("Query plan".equals(name)) {
+ assertFalse(F.isEmpty(val));
+ hasPlan.set(true);
+ }
+ }
+ });
+
+ assertEquals(1, qryCnt.get());
+ assertTrue(hasPlan.get());
+ }
+
/** */
@Test
public void testSqlEvents() {
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
index 293b675738e..f1ae19b9684 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
@@ -311,6 +311,9 @@ public class FilePerformanceStatisticsWriter {
* @param val Query property value.
*/
public void queryProperty(GridCacheQueryType type, UUID qryNodeId, long id, String name, String val) {
+ if (val == null)
+ return;
+
boolean cachedName = cacheIfPossible(name);
boolean cachedVal = cacheIfPossible(val);