alex-plekhanov commented on code in PR #12723:
URL: https://github.com/apache/ignite/pull/12723#discussion_r2804340285
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlPlanHistoryIntegrationTest.java:
##########
@@ -551,6 +551,38 @@ public void testNoScanCountSuffixForUNION() throws
Exception {
checkMetrics(getSqlPlanHistory());
}
+ /**
+ * Ensures H2 auto-generated numeric aliases (e.g. "_1", "_4") are
normalized so that repeated executions of the same
+ * query produce a single unique plan in SQL plan history.
+ */
+ @Test
+ public void testH2AutoAliasInPlanHistory() throws Exception {
+
assumeTrue(IndexingQueryEngineConfiguration.ENGINE_NAME.equals(sqlEngine));
+
+ assumeFalse(isPerfStatsEnabled);
Review Comment:
Why perfstat check is skipped?
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlPlanHistoryIntegrationTest.java:
##########
@@ -551,6 +551,38 @@ public void testNoScanCountSuffixForUNION() throws
Exception {
checkMetrics(getSqlPlanHistory());
}
+ /**
+ * Ensures H2 auto-generated numeric aliases (e.g. "_1", "_4") are
normalized so that repeated executions of the same
+ * query produce a single unique plan in SQL plan history.
+ */
+ @Test
+ public void testH2AutoAliasInPlanHistory() throws Exception {
+
assumeTrue(IndexingQueryEngineConfiguration.ENGINE_NAME.equals(sqlEngine));
+
+ assumeFalse(isPerfStatsEnabled);
+
+ startTestGrid();
+
+ IgniteCache<Integer, String> cache = queryNode().cache("A");
+
+
queryNode().context().query().runningQueryManager().resetPlanHistoryMetrics();
+
+ cache.query(new SqlFieldsQuery("CREATE TABLE users(id INT PRIMARY KEY,
name VARCHAR)")).getAll();
+ cache.query(new SqlFieldsQuery("INSERT INTO users VALUES (1, 'a'), (2,
'b')")).getAll();
+
+ String qry = "SELECT id, name FROM (SELECT id, name FROM users) WHERE
id = 1";
+
+ for (int i = 0; i < planHistorySize; i++)
+ cache.query(new SqlFieldsQuery(qry).setLocal(loc)).getAll();
+
+ List<String> plans = getSqlPlanHistory().stream()
+ .filter(planView -> planView.sql().contains(qry))
+ .map(SqlPlanHistoryView::plan)
+ .collect(Collectors.toList());
+
+ assertEquals("Expected 1 unique plan, got " + plans.size() + ": " +
plans, plans.size(), 1);
Review Comment:
expected value - second parameter, actual value - third
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/SqlPlanHistoryIntegrationTest.java:
##########
@@ -551,6 +551,38 @@ public void testNoScanCountSuffixForUNION() throws
Exception {
checkMetrics(getSqlPlanHistory());
}
+ /**
+ * Ensures H2 auto-generated numeric aliases (e.g. "_1", "_4") are
normalized so that repeated executions of the same
+ * query produce a single unique plan in SQL plan history.
+ */
+ @Test
+ public void testH2AutoAliasInPlanHistory() throws Exception {
+
assumeTrue(IndexingQueryEngineConfiguration.ENGINE_NAME.equals(sqlEngine));
+
+ assumeFalse(isPerfStatsEnabled);
+
+ startTestGrid();
+
+ IgniteCache<Integer, String> cache = queryNode().cache("A");
+
+
queryNode().context().query().runningQueryManager().resetPlanHistoryMetrics();
+
+ cache.query(new SqlFieldsQuery("CREATE TABLE users(id INT PRIMARY KEY,
name VARCHAR)")).getAll();
+ cache.query(new SqlFieldsQuery("INSERT INTO users VALUES (1, 'a'), (2,
'b')")).getAll();
+
+ String qry = "SELECT id, name FROM (SELECT id, name FROM users) WHERE
id = 1";
+
+ for (int i = 0; i < planHistorySize; i++)
+ cache.query(new SqlFieldsQuery(qry).setLocal(loc)).getAll();
+
+ List<String> plans = getSqlPlanHistory().stream()
+ .filter(planView -> planView.sql().contains(qry))
+ .map(SqlPlanHistoryView::plan)
+ .collect(Collectors.toList());
Review Comment:
```
String qry = "SELECT _key, _val FROM (SELECT _key, _val FROM String)
WHERE _key = 0";
for (int i = 0; i < planHistorySize; i++)
cacheQuery(new SqlFieldsQuery(qry), "A");
List<SqlPlanHistoryView> plans = getSqlPlanHistory();
```
##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2QueryInfo.java:
##########
@@ -129,12 +131,22 @@ public synchronized String plan() {
if (plan == null) {
String plan0 = stmt.getPlanSQL();
- plan = (plan0 != null) ? planWithoutScanCount(plan0) : "";
+ plan = (plan0 != null) ? cleanPlan(plan0) : "";
}
return plan;
}
+ /**
+ * @param plan Plan.
+ */
+ private String cleanPlan(String plan) {
Review Comment:
normalizePlan?
##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2QueryInfo.java:
##########
@@ -258,19 +270,85 @@ public String planWithoutScanCount(String plan) {
* @param startPattern Start pattern.
* @param endMarker End marker.
*/
- private void removePattern(StringBuilder sb, String startPattern, String
endMarker) {
- int start = sb.lastIndexOf(startPattern);
+ private void removeLineWithPattern(StringBuilder sb, String startPattern,
String endMarker) {
+ int searchFrom = 0;
+ int start = sb.indexOf(startPattern, searchFrom);
while (start != -1) {
+ while (start > 0 && sb.charAt(start) != '\n')
+ --start;
Review Comment:
It can remove something unexpected, for example if user will add comment to
query, something like `SELECT * FROM tbl /* scanCount: unknown */` this code
will remove the whole query. Let's remove only spaces here, something like:
```
while (start > 0 && sb.charAt(start - 1) == ' ')
--start;
if (start > 0 && sb.charAt(start - 1) == '\n')
--start;
```
##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/H2QueryInfo.java:
##########
@@ -258,19 +270,85 @@ public String planWithoutScanCount(String plan) {
* @param startPattern Start pattern.
* @param endMarker End marker.
*/
- private void removePattern(StringBuilder sb, String startPattern, String
endMarker) {
- int start = sb.lastIndexOf(startPattern);
+ private void removeLineWithPattern(StringBuilder sb, String startPattern,
String endMarker) {
+ int searchFrom = 0;
Review Comment:
Looks like additional variable is redundant, it can be inlined as 0 in first
`indexOf` and inlined as `start` in second `indexOf`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
