diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonInsert.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonInsert.java index a99df63983c2a2..06bedc8d522729 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonInsert.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonInsert.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; @@ -64,6 +65,15 @@ public FunctionSignature customSignature() { return FunctionSignature.of(JsonType.INSTANCE, arguments); } + @Override + public void checkLegalityBeforeTypeCoercion() { + // arguments are a JSON document followed by (path, value) pairs, so arity must be odd + if ((arity() & 1) == 0) { + throw new AnalysisException(getName() + " requires a JSON document followed by path/value pairs," + + " so the number of arguments must be odd, but got " + arity() + ": " + this.toSql()); + } + } + /** * withChildren. */ diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonReplace.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonReplace.java index a4d06ddbaff6e7..c626d3ed5a8855 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonReplace.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonReplace.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; @@ -64,6 +65,15 @@ public FunctionSignature customSignature() { return FunctionSignature.of(JsonType.INSTANCE, arguments); } + @Override + public void checkLegalityBeforeTypeCoercion() { + // arguments are a JSON document followed by (path, value) pairs, so arity must be odd + if ((arity() & 1) == 0) { + throw new AnalysisException(getName() + " requires a JSON document followed by path/value pairs," + + " so the number of arguments must be odd, but got " + arity() + ": " + this.toSql()); + } + } + /** * withChildren. */ diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonSet.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonSet.java index cdc9ed82b4f778..0f017d3d0d0ade 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonSet.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/JsonSet.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.CustomSignature; @@ -64,6 +65,15 @@ public FunctionSignature customSignature() { return FunctionSignature.of(JsonType.INSTANCE, arguments); } + @Override + public void checkLegalityBeforeTypeCoercion() { + // arguments are a JSON document followed by (path, value) pairs, so arity must be odd + if ((arity() & 1) == 0) { + throw new AnalysisException(getName() + " requires a JSON document followed by path/value pairs," + + " so the number of arguments must be odd, but got " + arity() + ": " + this.toSql()); + } + } + /** * withChildren. */ diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/CheckExpressionLegalityTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/CheckExpressionLegalityTest.java index 447156afb38c95..21c11e6797e52d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/CheckExpressionLegalityTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/CheckExpressionLegalityTest.java @@ -70,6 +70,24 @@ public void testArraySortLambdaArgumentCount() { }); } + @Test + public void testJsonModifyFunctionsRejectEvenArity() { + ConnectContext connectContext = MemoTestUtils.createConnectContext(); + for (String function : new String[] {"json_set", "jsonb_set", "json_insert", "jsonb_insert", + "json_replace", "jsonb_replace"}) { + ExceptionChecker.expectThrowsWithMsg(AnalysisException.class, + "number of arguments must be odd, but got 4", () -> + PlanChecker.from(connectContext) + .analyze("select " + function + "('{}', '$.a', 1, '$.b')")); + ExceptionChecker.expectThrowsWithMsg(AnalysisException.class, + "number of arguments must be odd, but got 6", () -> + PlanChecker.from(connectContext) + .analyze("select " + function + "('{}', '$.a', 1, '$.b', 2, '$.c')")); + PlanChecker.from(connectContext).analyze("select " + function + "('{}', '$.a', 1)"); + PlanChecker.from(connectContext).analyze("select " + function + "('{}', '$.a', 1, '$.b', 2)"); + } + } + @Test public void testCountDistinctBitmap() { ConnectContext connectContext = MemoTestUtils.createConnectContext(); diff --git a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_insert.out b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_insert.out index 9c6126266f91eb..e43d2018d7ae8e 100644 --- a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_insert.out +++ b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_insert.out @@ -93,3 +93,6 @@ -- !insert7 -- {"a":200} +-- !insert_odd_arity_ok -- +{"a":1} {"a":1,"b":2} {"a":1,"b":2} + diff --git a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_replace.out b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_replace.out index ac8cfb4501008f..3a67a270a042e8 100644 --- a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_replace.out +++ b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_replace.out @@ -80,3 +80,6 @@ null -- !replace8 -- {"a":100} +-- !replace_odd_arity_ok -- +{} {} {} + diff --git a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_set.out b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_set.out index b57a50e314bb5d..e198f86db0b5c4 100644 --- a/regression-test/data/query_p0/sql_functions/json_function/test_query_json_set.out +++ b/regression-test/data/query_p0/sql_functions/json_function/test_query_json_set.out @@ -85,3 +85,6 @@ null -- !set8 -- {"a":100} +-- !set_odd_arity_ok -- +{"a":1} {"a":1,"b":2} {"a":1,"b":2} + diff --git a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_insert.groovy b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_insert.groovy index 13893795fc1297..25d91e8c03c4f0 100644 --- a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_insert.groovy +++ b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_insert.groovy @@ -128,4 +128,26 @@ suite("test_query_json_insert", "query,arrow_flight_sql") { sql "select json_insert('1', '\$.', 4);" exception "Json path error: Invalid Json Path for value" } + + // arguments must be a JSON document followed by path/value pairs, so an even count is rejected by FE + test { + sql "select json_insert('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "select json_insert('{}', '\$.a', 1, '\$.b', 2, '\$.c');" + exception "number of arguments must be odd, but got 6" + } + + test { + sql "select jsonb_insert('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "explain select json_insert('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + qt_insert_odd_arity_ok """select json_insert('{}', '\$.a', 1), json_insert('{}', '\$.a', 1, '\$.b', 2), jsonb_insert('{}', '\$.a', 1, '\$.b', 2);""" } diff --git a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_replace.groovy b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_replace.groovy index 69f7142eff75b4..0b91ca39dbbddc 100644 --- a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_replace.groovy +++ b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_replace.groovy @@ -133,4 +133,26 @@ suite("test_query_json_replace", "query") { sql "select json_replace('1', '\$.', 4);" exception "Json path error: Invalid Json Path for value" } + + // arguments must be a JSON document followed by path/value pairs, so an even count is rejected by FE + test { + sql "select json_replace('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "select json_replace('{}', '\$.a', 1, '\$.b', 2, '\$.c');" + exception "number of arguments must be odd, but got 6" + } + + test { + sql "select jsonb_replace('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "explain select json_replace('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + qt_replace_odd_arity_ok """select json_replace('{}', '\$.a', 1), json_replace('{}', '\$.a', 1, '\$.b', 2), jsonb_replace('{}', '\$.a', 1, '\$.b', 2);""" } diff --git a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_set.groovy b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_set.groovy index 705e6046705e54..91174fa88271e9 100644 --- a/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_set.groovy +++ b/regression-test/suites/query_p0/sql_functions/json_function/test_query_json_set.groovy @@ -120,4 +120,26 @@ suite("test_query_json_set", "query") { sql "select json_set('1', '\$.', 4);" exception "Json path error: Invalid Json Path for value" } + + // arguments must be a JSON document followed by path/value pairs, so an even count is rejected by FE + test { + sql "select json_set('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "select json_set('{}', '\$.a', 1, '\$.b', 2, '\$.c');" + exception "number of arguments must be odd, but got 6" + } + + test { + sql "select jsonb_set('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + + test { + sql "explain select json_set('{}', '\$.a', 1, '\$.b');" + exception "number of arguments must be odd, but got 4" + } + qt_set_odd_arity_ok """select json_set('{}', '\$.a', 1), json_set('{}', '\$.a', 1, '\$.b', 2), jsonb_set('{}', '\$.a', 1, '\$.b', 2);""" }