This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch fix/lal-numeric-eq-neq in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit cffb7f6f4f24e29bc112170035af8b9453702e6b Author: Wu Sheng <[email protected]> AuthorDate: Fri Mar 13 18:49:27 2026 +0800 Fix LAL == and != with numeric literals using object equality instead of numeric comparison When comparing with numeric literals (e.g., `!= 403`), the LAL compiler was generating `Objects.equals()` with boxed types (`Long.valueOf(403L)`), which fails when the left side is a different boxed type (e.g., Integer). Now EQ/NEQ with numeric right-hand side use `generateNumericComparison()` (primitive `== 403L` / `!= 403L`), matching the behavior of `>`, `<`, `>=`, `<=`. String/boolean/null comparisons still use `Objects.equals()`. Also ensures all LAL codegen unit tests both compile and assert generated source (previously some tests only asserted source without compiling). --- .../log/analyzer/v2/compiler/LALValueCodegen.java | 28 +++-- .../v2/compiler/LALClassGeneratorBasicTest.java | 7 +- .../compiler/LALClassGeneratorConditionTest.java | 132 +++++++++++++++++++-- .../v2/compiler/LALClassGeneratorDefTest.java | 66 ++++++----- .../compiler/LALClassGeneratorExtractorTest.java | 18 +-- 5 files changed, 187 insertions(+), 64 deletions(-) diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java index 63588f34b1..b59d6de257 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java @@ -95,18 +95,26 @@ final class LALValueCodegen { (LALScriptModel.ComparisonCondition) cond; switch (cc.getOp()) { case EQ: - sb.append("java.util.Objects.equals("); - generateValueAccessObj(sb, cc.getLeft(), cc.getLeftCast(), genCtx); - sb.append(", "); - generateConditionValue(sb, cc.getRight(), genCtx); - sb.append(")"); + if (cc.getRight() instanceof LALScriptModel.NumberConditionValue) { + generateNumericComparison(sb, cc, " == ", genCtx); + } else { + sb.append("java.util.Objects.equals("); + generateValueAccessObj(sb, cc.getLeft(), cc.getLeftCast(), genCtx); + sb.append(", "); + generateConditionValue(sb, cc.getRight(), genCtx); + sb.append(")"); + } break; case NEQ: - sb.append("!java.util.Objects.equals("); - generateValueAccessObj(sb, cc.getLeft(), cc.getLeftCast(), genCtx); - sb.append(", "); - generateConditionValue(sb, cc.getRight(), genCtx); - sb.append(")"); + if (cc.getRight() instanceof LALScriptModel.NumberConditionValue) { + generateNumericComparison(sb, cc, " != ", genCtx); + } else { + sb.append("!java.util.Objects.equals("); + generateValueAccessObj(sb, cc.getLeft(), cc.getLeftCast(), genCtx); + sb.append(", "); + generateConditionValue(sb, cc.getRight(), genCtx); + sb.append(")"); + } break; case GT: generateNumericComparison(sb, cc, " > ", genCtx); diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorBasicTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorBasicTest.java index 29f4103319..4e9c3310af 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorBasicTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorBasicTest.java @@ -75,9 +75,10 @@ class LALClassGeneratorBasicTest extends LALClassGeneratorTestBase { } @Test - void generateSourceReturnsJavaCode() { - final String source = generator.generateSource( - "filter { json {} sink {} }"); + void compileAndVerifySourceReturnsJavaCode() throws Exception { + final String dsl = "filter { json {} sink {} }"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertNotNull(source); assertTrue(source.contains("filterSpec.json(ctx)")); assertTrue(source.contains("filterSpec.sink(ctx)")); diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorConditionTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorConditionTest.java index b537a3e940..af4c8bc942 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorConditionTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorConditionTest.java @@ -40,13 +40,14 @@ class LALClassGeneratorConditionTest extends LALClassGeneratorTestBase { } @Test - void generateSourceTagFunctionEmitsTagValue() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyTagFunctionEmitsTagValue() throws Exception { + final String dsl = "filter {\n" + " if (tag(\"LOG_KIND\") == \"SLOW_SQL\") {\n" + " sink {}\n" + " }\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("h.tagValue(\"LOG_KIND\")"), "Expected tagValue call but got: " + source); assertTrue(source.contains("SLOW_SQL")); @@ -93,14 +94,15 @@ class LALClassGeneratorConditionTest extends LALClassGeneratorTestBase { } @Test - void generateSourceSafeNavMethodEmitsSpecificHelper() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifySafeNavMethodEmitsSpecificHelper() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " if (parsed?.flags?.toString()) {\n" + " sink {}\n" + " }\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("h.toString("), "Expected toString helper for safe nav method but got: " + source); assertTrue(source.contains("h.isNotEmpty("), @@ -139,6 +141,111 @@ class LALClassGeneratorConditionTest extends LALClassGeneratorTestBase { + "}"); } + // ==================== Numeric equality/inequality ==================== + + @Test + void compileNeqNumericEmitsNumericOp() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.code as Integer != 403) {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("!= 403L"), + "Expected numeric != but got: " + source); + } + + @Test + void compileEqNumericEmitsNumericOp() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.code as Integer == 200) {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("== 200L"), + "Expected numeric == but got: " + source); + } + + @Test + void compileEqStringUsesObjectsEquals() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.status == \"OK\") {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("java.util.Objects.equals("), + "Expected Objects.equals for string comparison but got: " + source); + } + + @Test + void compileNeqStringUsesObjectsEquals() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.status != \"ERROR\") {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("!java.util.Objects.equals("), + "Expected !Objects.equals for string != but got: " + source); + } + + @Test + void compileNeqNumericWithLogicalAnd() throws Exception { + // Matches the envoy-als pattern that triggered the bug + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.code as Integer != 401" + + " && parsed?.code as Integer != 403) {\n" + + " abort {}\n" + + " }\n" + + " sink {}\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("!= 401L"), + "Expected numeric != 401L but got: " + source); + assertTrue(source.contains("!= 403L"), + "Expected numeric != 403L but got: " + source); + } + + @Test + void compileEqNullUsesObjectsEquals() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.status == null) {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("java.util.Objects.equals("), + "Expected Objects.equals for null comparison but got: " + source); + } + + @Test + void compileNeqNullUsesObjectsEquals() throws Exception { + final String dsl = "filter {\n" + + " json {}\n" + + " if (parsed?.status != null) {\n" + + " sink {}\n" + + " }\n" + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); + assertTrue(source.contains("!java.util.Objects.equals("), + "Expected !Objects.equals for null != but got: " + source); + } + // ==================== Else-if chain ==================== @Test @@ -159,9 +266,8 @@ class LALClassGeneratorConditionTest extends LALClassGeneratorTestBase { } @Test - void generateSourceElseIfEmitsNestedBranches() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyElseIfEmitsNestedBranches() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " if (parsed.a) {\n" + " sink {}\n" @@ -170,7 +276,9 @@ class LALClassGeneratorConditionTest extends LALClassGeneratorTestBase { + " } else {\n" + " sink {}\n" + " }\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("else"), "Expected else branch but got: " + source); int ifCount = 0; diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorDefTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorDefTest.java index 9c867839f4..6cb833dff6 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorDefTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorDefTest.java @@ -33,16 +33,17 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { // ==================== Source generation ==================== @Test - void generateSourceDefWithToJson() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyDefWithToJson() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def config = toJson(parsed.metadata)\n" + " tag 'env': config?.get(\"env\")?.getAsString()\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("com.google.gson.JsonObject _def_config"), "Expected JsonObject declaration but got:\n" + source); assertTrue(source.contains("h.toJsonObject("), @@ -54,16 +55,17 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { } @Test - void generateSourceDefWithToJsonArray() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyDefWithToJsonArray() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def items = toJsonArray(parsed.tags)\n" + " tag 'first': items?.get(0)?.getAsString()\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("com.google.gson.JsonArray _def_items"), "Expected JsonArray declaration but got:\n" + source); assertTrue(source.contains("h.toJsonArray("), @@ -73,9 +75,8 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { } @Test - void generateSourceDefWithCondition() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyDefWithCondition() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def config = toJson(parsed.metadata)\n" @@ -84,7 +85,9 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { + " }\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("_def_config == null") || source.contains("_def_config != null"), "Expected null check on _def_config but got:\n" + source); assertTrue(source.contains(".has(\"env\")"), @@ -161,16 +164,17 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { // ==================== Type cast on def ==================== @Test - void generateSourceDefWithStringCast() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyDefWithStringCast() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def svc = parsed.service as String\n" + " tag 'svc': svc\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains("java.lang.String _def_svc"), "Expected String declaration but got:\n" + source); assertTrue(source.contains("(java.lang.String)"), @@ -198,18 +202,19 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { } @Test - void generateSourceDefWithQualifiedNameCast() { + void compileAndVerifyDefWithQualifiedNameCast() throws Exception { generator.setInputType( io.envoyproxy.envoy.data.accesslog.v3.HTTPAccessLogEntry.class); - final String source = generator.generateSource( - "filter {\n" + final String dsl = "filter {\n" + " extractor {\n" + " def common = parsed?.commonProperties" + " as io.envoyproxy.envoy.data.accesslog.v3.AccessLogCommon\n" + " tag 'cluster': common?.upstreamCluster\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains( "io.envoyproxy.envoy.data.accesslog.v3.AccessLogCommon _def_common"), "Expected AccessLogCommon declaration but got:\n" + source); @@ -221,9 +226,8 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { // ==================== Def variable as method argument ==================== @Test - void generateSourceDefVarAsMethodArg() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyDefVarAsMethodArg() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def key = parsed.fieldName as String\n" @@ -231,25 +235,25 @@ class LALClassGeneratorDefTest extends LALClassGeneratorTestBase { + " tag 'val': config?.get(key)?.getAsString()\n" + " }\n" + " sink {}\n" - + "}"); - // _def_key = key (String), _def_config = config (JsonObject) - // config?.get(key) should generate _def_config.get(_def_key), not _def_config.get(null) + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains(".get(_def_key)"), "Expected .get(_def_key) for def var arg but got:\n" + source); } @Test - void generateSourceBoolLiteralAsMethodArg() { - final String source = generator.generateSource( - "filter {\n" + void compileAndVerifyBoolLiteralAsMethodArg() throws Exception { + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " def config = toJson(parsed.metadata)\n" + " tag 'val': config?.get(\"key\")?.getAsBoolean()\n" + " }\n" + " sink {}\n" - + "}"); - // getAsBoolean() has no args, just verify it compiles + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains(".getAsBoolean()"), "Expected .getAsBoolean() call but got:\n" + source); } diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorExtractorTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorExtractorTest.java index 8fa8640ed8..179a6a0cdb 100644 --- a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorExtractorTest.java +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALClassGeneratorExtractorTest.java @@ -187,17 +187,18 @@ class LALClassGeneratorExtractorTest extends LALClassGeneratorTestBase { // ==================== Output field assignment ==================== @Test - void compileOutputFieldAssignment() { + void compileOutputFieldAssignment() throws Exception { generator.setOutputType(TestOutputType.class); - final String source = generator.generateSource( - "filter {\n" + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " statement parsed.statement as String\n" + " latency parsed.latency as Long\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains(".setStatement("), "Expected direct setStatement() call but got: " + source); assertTrue(source.contains(".setLatency("), @@ -213,16 +214,17 @@ class LALClassGeneratorExtractorTest extends LALClassGeneratorTestBase { } @Test - void compileOutputFieldWithOutputTypeValidation() { + void compileOutputFieldWithOutputTypeValidation() throws Exception { generator.setOutputType(TestOutputType.class); - final String source = generator.generateSource( - "filter {\n" + final String dsl = "filter {\n" + " json {}\n" + " extractor {\n" + " statement parsed.stmt as String\n" + " }\n" + " sink {}\n" - + "}"); + + "}"; + compileAndAssert(dsl); + final String source = generator.generateSource(dsl); assertTrue(source.contains(".setStatement("), "Expected direct setStatement() call but got: " + source); }
