Skip to content

Commit 26b2d69

Browse files
committed
style: trim verbose comments per review
Per @penghuo: drop the verbose multi-line explanatory comments and tighten the class/method javadoc on the new dashboard IT. Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent c42fb7c commit 26b2d69

4 files changed

Lines changed: 7 additions & 50 deletions

File tree

api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,6 @@ public static class Builder {
157157
PPL_JOIN_SUBSEARCH_MAXOUT, SysLimit.UNLIMITED_SUBSEARCH.joinSubsearchLimit()),
158158
Map.entry(CALCITE_ENGINE_ENABLED, true),
159159
Map.entry(PPL_REX_MAX_MATCH_LIMIT, 10),
160-
// PPL `patterns` command defaults — mirror the cluster-side defaults registered in
161-
// OpenSearchSettings (DEFAULT_PATTERN_METHOD_SETTING etc.). Without these the
162-
// analytics-engine path's AstBuilder.visitPatternsCommand reads null from
163-
// `settings.getSettingValue(Key.PATTERN_METHOD)`, fails with
164-
// `PatternMethod.valueOf("NULL")` IllegalArgumentException, and every query that
165-
// omits an explicit `method=` / `mode=` argument is rejected.
166160
Map.entry(PATTERN_METHOD, "SIMPLE_PATTERN"),
167161
Map.entry(PATTERN_MODE, "LABEL"),
168162
Map.entry(PATTERN_MAX_SAMPLE_COUNT, 10),

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4303,12 +4303,8 @@ private void buildParseRelNode(Parse node, CalcitePlanContext context) {
43034303
for (String groupCandidate : groupCandidates) {
43044304
RexNode innerRex;
43054305
if (ParseMethod.PATTERNS.equals(parseMethod)) {
4306-
// Emit `regexp_replace(field, pattern, replacement, "g")` directly so the replacement
4307-
// is global (every match replaced). DataFusion's `regexp_replace` defaults to FIRST
4308-
// match only without the "g" flag — using the 3-arg form via the REPLACE handler
4309-
// produces `<*>@pyrami.com` instead of `<*>@<*>.<*>` on the analytics-engine route.
4310-
// Calcite's REGEXP_REPLACE_PG_4 with "g" matches what `replaceAll` does, so V2 /
4311-
// Calcite-path semantics are preserved.
4306+
// Emit 4-arg REGEXP_REPLACE_PG_4 with "g" so DataFusion's regexp_replace
4307+
// (first-match-only by default) replaces every match.
43124308
RexNode globalFlag =
43134309
context.rexBuilder.makeLiteral(
43144310
"g", context.rexBuilder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR), true);

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLDashboardPatternsIT.java

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,7 @@
1414
import org.junit.Test;
1515
import org.opensearch.sql.ppl.PPLIntegTestCase;
1616

17-
/**
18-
* Pins the exact PPL query shape OpenSearch Dashboards uses to render BRAIN-pattern panels: {@code
19-
* patterns ... method=BRAIN mode=label} followed by {@code stats count(), take(message, 1) by
20-
* patterns_field | sort -pattern_count | fields patterns_field, pattern_count, sample_logs}.
21-
*
22-
* <p>The combination exercises three pieces that all have to be wired through the analytics-engine
23-
* route together:
24-
*
25-
* <ul>
26-
* <li>{@code INTERNAL_PATTERN} as a window function (label mode emits one matched wildcard
27-
* pattern per row, broadcast across the partition).
28-
* <li>{@code take(field, n)} aggregate to capture a representative sample log per discovered
29-
* pattern group.
30-
* <li>{@code count()} aggregate + {@code sort} on the result.
31-
* </ul>
32-
*
33-
* <p>Schema-only assertions: BRAIN's clustering depends on a corpus large enough that the default
34-
* heuristics fire, so per-row pattern strings are sensitive to dataset version. The schema check
35-
* guarantees the query plans, executes, and returns the dashboard's three expected columns in the
36-
* right order.
37-
*
38-
* @opensearch.internal
39-
*/
17+
/** Pins the BRAIN-label pattern panel query shape used by OpenSearch Dashboards. */
4018
public class CalcitePPLDashboardPatternsIT extends PPLIntegTestCase {
4119
@Override
4220
public void init() throws Exception {
@@ -45,11 +23,6 @@ public void init() throws Exception {
4523
loadIndex(Index.HDFS_LOGS);
4624
}
4725

48-
/**
49-
* Mirrors the canonical Dashboards BRAIN-label pattern panel query. Unfiltered variant — pins the
50-
* end-to-end plan compiles and returns the dashboard's three-column shape (matched wildcard
51-
* pattern + occurrence count + sample log).
52-
*/
5326
@Test
5427
public void testDashboardBrainLabelStatsByPatternsField() throws IOException {
5528
JSONObject result =

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RexStandardizer.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,10 @@ public RexNode visitCall(final RexCall call, ScriptParameterHelper helper) {
7474
// We can downgrade to still use `Sarg` literal instead of replacing it with parameter.
7575
}
7676
}
77-
// PPL's `patterns` lowering emits 4-arg `REGEXP_REPLACE_PG_4(field, pattern, replacement,
78-
// 'g')` so the analytics-engine/DataFusion path gets global replacement (DataFusion's
79-
// `regexp_replace` defaults to first-match-only without an explicit flag). Calcite's
80-
// 3-arg `REGEXP_REPLACE_3` is already replace-all in its enumerable runtime, but the PG_4
81-
// form has no matching `SqlFunctions.regexpReplace(String, String, String, String)` impl
82-
// — Calcite's runtime only ships the `(String, String, String, int[, ...])` shapes (the
83-
// 4-arg variant treats the 4th arg as start-position, not a flags string). The script
84-
// pushdown path codegen would fail with `No applicable constructor/method found`. Collapse
85-
// the 4-arg call to the 3-arg form whenever the flags literal is exactly `"g"`, which
86-
// preserves replace-all semantics on the V2 / Calcite-pushdown side.
77+
// Calcite's enumerable runtime has no regexpReplace(String, String, String, String) impl,
78+
// so 4-arg REGEXP_REPLACE_PG_4 fails Janino codegen on the script-pushdown path. Collapse
79+
// to 3-arg REGEXP_REPLACE_3 when the flags literal is "g" — the 3-arg form is already
80+
// replace-all, so semantics are preserved.
8781
if (call.op == SqlLibraryOperators.REGEXP_REPLACE_PG_4
8882
&& call.operands.size() == 4
8983
&& call.operands.get(3) instanceof RexLiteral flagsLit

0 commit comments

Comments
 (0)