You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/04/05 08:44:49 UTC

[GitHub] [flink] wuchong commented on a change in pull request #11186: [FLINK-16200][sql] Support JSON_EXISTS for blink planner

wuchong commented on a change in pull request #11186: [FLINK-16200][sql] Support JSON_EXISTS for blink planner
URL: https://github.com/apache/flink/pull/11186#discussion_r403669654
 
 

 ##########
 File path: flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/JsonFunctionsTest.scala
 ##########
 @@ -125,4 +127,53 @@ class JsonFunctionsTest extends ExpressionTestBase {
     }
   }
 
+  @Test
+  def testJsonExists(): Unit = {
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo' false on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo' true on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo' unknown on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo' false on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo' true on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo' unknown on error)", "true")
+    testSqlApi("json_exists('{}', "
+      + "'invalid $.foo' false on error)", "false")
+    testSqlApi("json_exists('{}', "
+      + "'invalid $.foo' true on error)", "true")
+    testSqlApi("json_exists('{}', "
+      + "'invalid $.foo' unknown on error)", "null")
+    testSqlApi("json_exists(cast('{\"foo\":\"bar\"}' as varchar), "
+      + "'strict $.foo1')", "false")
+
+    // not exists
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo1' false on error)", "false")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo1' true on error)", "true")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'strict $.foo1' unknown on error)", "null")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo1' true on error)", "false")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo1' false on error)", "false")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo1' error on error)", "false")
+    testSqlApi("json_exists('{\"foo\":\"bar\"}', "
+      + "'lax $.foo1' unknown on error)", "false")
+
+    // nulls
+    testSqlApi("json_exists(cast(null as varchar), 'lax $' unknown on error)", "null")
+  }
+
+  @Test
+  def testJsonFuncError(): Unit = {
+    expectedException.expect(classOf[CodeGenException])
+    expectedException.expectMessage(startsWith("Unsupported call: JSON_EXISTS"))
 
 Review comment:
   I got what you want to throw for the exception. Currently, Calcite JSON_EXISTS definition accepts any type for first argument, however, we only support string in the code generation (do you know why Calcite accept any type?)
   
   For this purpose, I would suggest to define JSON_EXISTS ourselves:
   
   ```
   	public static final SqlFunction JSON_EXISTS = new SqlFunction(
   		"JSON_EXISTS",
   		SqlKind.OTHER_FUNCTION,
   		ReturnTypes.cascade(ReturnTypes.BOOLEAN, SqlTypeTransforms.FORCE_NULLABLE), null,
   		OperandTypes.or(
   			OperandTypes.family(SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER),
   			OperandTypes.family(SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER, SqlTypeFamily.ANY)),
   		SqlFunctionCategory.SYSTEM);
   ```
   
   In this way, the exception will be more reable, and we don't need the "hack" in `StringCallGen.scala`. 
   
   ```
   ValidationException: SQL validation failed. From line 1, column 8 to line 1, column 48: Cannot apply 'JSON_EXISTS' to arguments of type 'JSON_EXISTS(<INTEGER>, <CHAR(5)>, <SYMBOL>)'. Supported form(s): 'JSON_EXISTS(<CHARACTER>, <CHARACTER>)'
   'JSON_EXISTS(<CHARACTER>, <CHARACTER>, <ANY>)'
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services