[calcite] branch master updated: [CALCITE-3941] Add the default strict mode to the path in the Json functions
This is an automated email from the ASF dual-hosted git repository.
chunwei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new 057186d [CALCITE-3941] Add the default strict mode to the path in the
Json functions
057186d is described below
commit 057186d1c462ce06f72ed296d50e2836fe0d35df
Author: XuQianJin-Stars
AuthorDate: Fri May 1 15:04:02 2020 +0800
[CALCITE-3941] Add the default strict mode to the path in the Json functions
---
.../org/apache/calcite/runtime/JsonFunctions.java | 24 --
.../calcite/sql/test/SqlOperatorBaseTest.java | 15 ++
.../apache/calcite/test/SqlJsonFunctionsTest.java | 3 +++
3 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/JsonFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/JsonFunctions.java
index 75cce8a..21d42d7 100644
--- a/core/src/main/java/org/apache/calcite/runtime/JsonFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/JsonFunctions.java
@@ -29,6 +29,7 @@ import com.fasterxml.jackson.core.util.DefaultIndenter;
import com.fasterxml.jackson.core.util.DefaultPrettyPrinter;
import com.jayway.jsonpath.Configuration;
import com.jayway.jsonpath.DocumentContext;
+import com.jayway.jsonpath.InvalidPathException;
import com.jayway.jsonpath.JsonPath;
import com.jayway.jsonpath.Option;
import com.jayway.jsonpath.spi.json.JacksonJsonProvider;
@@ -112,18 +113,22 @@ public class JsonFunctions {
}
public static JsonPathContext jsonApiCommonSyntax(JsonValueContext input,
String pathSpec) {
+PathMode mode;
+String pathStr;
try {
Matcher matcher = JSON_PATH_BASE.matcher(pathSpec);
if (!matcher.matches()) {
-throw RESOURCE.illegalJsonPathSpec(pathSpec).ex();
+mode = PathMode.STRICT;
+pathStr = pathSpec;
+ } else {
+mode = PathMode.valueOf(matcher.group(1).toUpperCase(Locale.ROOT));
+pathStr = matcher.group(2);
}
- PathMode mode =
PathMode.valueOf(matcher.group(1).toUpperCase(Locale.ROOT));
- String pathWff = matcher.group(2);
DocumentContext ctx;
switch (mode) {
case STRICT:
if (input.hasException()) {
- return JsonPathContext.withStrictException(input.exc);
+ return JsonPathContext.withStrictException(pathSpec, input.exc);
}
ctx = JsonPath.parse(input.obj,
Configuration
@@ -148,9 +153,9 @@ public class JsonFunctions {
throw RESOURCE.illegalJsonPathModeInPathSpec(mode.toString(),
pathSpec).ex();
}
try {
-return JsonPathContext.withJavaObj(mode, ctx.read(pathWff));
+return JsonPathContext.withJavaObj(mode, ctx.read(pathStr));
} catch (Exception e) {
-return JsonPathContext.withStrictException(e);
+return JsonPathContext.withStrictException(pathSpec, e);
}
} catch (Exception e) {
return JsonPathContext.withUnknownException(e);
@@ -715,6 +720,13 @@ public class JsonFunctions {
return new JsonPathContext(PathMode.STRICT, null, exc);
}
+public static JsonPathContext withStrictException(String pathSpec,
Exception exc) {
+ if (exc.getClass() == InvalidPathException.class) {
+exc = RESOURCE.illegalJsonPathSpec(pathSpec).ex();
+ }
+ return withStrictException(exc);
+}
+
public static JsonPathContext withJavaObj(PathMode mode, Object obj) {
if (mode == PathMode.UNKNOWN) {
throw RESOURCE.illegalJsonPathMode(mode.toString()).ex();
diff --git
a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 18a3313..f4e055a 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -4655,6 +4655,10 @@ public abstract class SqlOperatorBaseTest {
}
@Test void testJsonExists() {
+// default pathmode the default is: strict mode
+tester.checkBoolean("json_exists('{\"foo\":\"bar\"}', "
++ "'$.foo')", Boolean.TRUE);
+
tester.checkBoolean("json_exists('{\"foo\":\"bar\"}', "
+ "'strict $.foo' false on error)", Boolean.TRUE);
tester.checkBoolean("json_exists('{\"foo\":\"bar\"}', "
@@ -4707,6 +4711,9 @@ public abstract class SqlOperatorBaseTest {
true);
}
+// default pathmode the default is: strict mode
+tester.checkString("json_value('{\"foo\":100}', '$.foo')",
+"100", "VARCHAR(2000)");
// type casting test
tester.checkString("json_value('{\"foo\":100}', 'strict $.foo')",
"100", "VARCHAR(2000)");
@@ -4786,6 +4793,10 @@ public abstract class SqlOperatorBaseTest {
}
@Test void testJsonQuery() {
+// default pathmode the default i
[GitHub] [calcite] chunweilei merged pull request #1955: [CALCITE-3941] Add the default strict mode to the path in the Json functions
chunweilei merged pull request #1955: URL: https://github.com/apache/calcite/pull/1955 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: us...@infra.apache.org
[GitHub] [calcite] yanlin-Lynn commented on pull request #2021: [CALCITE-3929] When deserialize UDAF aggregate call from json string,…
yanlin-Lynn commented on pull request #2021: URL: https://github.com/apache/calcite/pull/2021#issuecomment-650744525 LGTM 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: us...@infra.apache.org
[GitHub] [calcite] michaelmior merged pull request #2021: [CALCITE-3929] When deserialize UDAF aggregate call from json string,…
michaelmior merged pull request #2021: URL: https://github.com/apache/calcite/pull/2021 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: us...@infra.apache.org
[calcite] branch master updated: [CALCITE-3929] When deserialize UDAF aggregate call from json string, throws NPE (Xu Zhaohui)
This is an automated email from the ASF dual-hosted git repository.
mmior pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new da71139 [CALCITE-3929] When deserialize UDAF aggregate call from json
string, throws NPE (Xu Zhaohui)
da71139 is described below
commit da7113949d6e22979cbb6981c1992a44db70cd4e
Author: xzh <953396...@qq.com>
AuthorDate: Thu Jun 11 20:56:32 2020 +0800
[CALCITE-3929] When deserialize UDAF aggregate call from json string,
throws NPE (Xu Zhaohui)
---
.../org/apache/calcite/rel/externalize/RelJson.java | 6 +-
.../java/org/apache/calcite/plan/RelWriterTest.java | 21 +
.../apache/calcite/test/MockSqlOperatorTable.java | 10 ++
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
index 2c9a0ca..7ee8519 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
@@ -273,7 +273,11 @@ public class RelJson {
public Object toJson(AggregateCall node) {
final Map map = jsonBuilder.map();
-map.put("agg", toJson(node.getAggregation()));
+final Map aggMap = toJson(node.getAggregation());
+if (node.getAggregation().getFunctionType().isUserDefined()) {
+ aggMap.put("class", node.getAggregation().getClass().getName());
+}
+map.put("agg", aggMap);
map.put("type", toJson(node.getType()));
map.put("distinct", node.isDistinct());
map.put("operands", node.getArgList());
diff --git a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
index 55a3d58..f64793e 100644
--- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
+++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
@@ -797,6 +797,27 @@ class RelWriterTest {
assertThat(s, isLinux(expected));
}
+ @Test void testUDAF() {
+final FrameworkConfig config = RelBuilderTest.config().build();
+final RelBuilder builder = RelBuilder.create(config);
+final RelNode rel = builder
+.scan("EMP")
+.project(builder.field("ENAME"), builder.field("DEPTNO"))
+.aggregate(
+builder.groupKey("ENAME"),
+builder.aggregateCall(new MockSqlOperatorTable.MyAggFunc(),
+builder.field("DEPTNO")))
+.build();
+final String relJson = RelOptUtil.dumpPlan("", rel,
+SqlExplainFormat.JSON, SqlExplainLevel.EXPPLAN_ATTRIBUTES);
+final String result = deserializeAndDumpToTextFormat(getSchema(rel),
relJson);
+final String expected = ""
++ "LogicalAggregate(group=[{0}], agg#0=[myAggFunc($1)])\n"
++ " LogicalProject(ENAME=[$1], DEPTNO=[$7])\n"
++ "LogicalTableScan(table=[[scott, EMP]])\n";
+assertThat(result, isLinux(expected));
+ }
+
@Test void testArrayType() {
final FrameworkConfig config = RelBuilderTest.config().build();
final RelBuilder builder = RelBuilder.create(config);
diff --git
a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
index 0c574a5..b1ac440 100644
--- a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
+++ b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
@@ -125,6 +125,16 @@ public class MockSqlOperatorTable extends
ChainedSqlOperatorTable {
}
}
+ /** "MYAGGFUNC" user-defined aggregate function. This agg function accept
one or more arguments
+ * in order to reproduce the throws of CALCITE-3929. */
+ public static class MyAggFunc extends SqlAggFunction {
+public MyAggFunc() {
+ super("myAggFunc", null, SqlKind.OTHER_FUNCTION, ReturnTypes.BIGINT,
null,
+ OperandTypes.ONE_OR_MORE, SqlFunctionCategory.USER_DEFINED_FUNCTION,
false, false,
+ Optionality.FORBIDDEN);
+}
+ }
+
/**
* "SPLIT" user-defined function. This function return array type
* in order to reproduce the throws of CALCITE-4062.
[GitHub] [calcite] zabetak commented on pull request #2009: Site: Add instructions for managing Calcite repos through GitHub
zabetak commented on pull request #2009: URL: https://github.com/apache/calcite/pull/2009#issuecomment-650835726 > Forgive me for being rather pedantic. This looks great. Just a few small suggested changes. Thanks for the second pair of eyes @michaelmior and @chunweilei , much appreciated. I addressed the comments, if nothing comes up in the following days I will push it through ;) 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: us...@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #2019: [CALCITE-4059] SqlTypeUtil#equalSansNullability consider Array/Map type.
danny0405 commented on a change in pull request #2019:
URL: https://github.com/apache/calcite/pull/2019#discussion_r446743935
##
File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
##
@@ -1160,6 +1161,49 @@ public static boolean equalSansNullability(
factory.createTypeWithNullability(type2, type1.isNullable()));
}
+ /**
+ * Returns whether two array/multiset types are equal, ignoring nullability.
+ *
+ * They need not come from the same factory.
+ *
+ * @param factory Type factory
+ * @param type1 First type
+ * @param type2 Second type
+ * @return Whether types are equal, ignoring nullability
+ */
+ public static boolean equalAsCollectionSansNullability(
+ RelDataTypeFactory factory,
+ RelDataType type1,
+ RelDataType type2) {
+Preconditions.checkArgument(isCollection(type1));
Review comment:
Add `type1 == type2` check to see if they are equals for fast check.
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:
us...@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #2019: [CALCITE-4059] SqlTypeUtil#equalSansNullability consider Array/Map type.
danny0405 commented on a change in pull request #2019:
URL: https://github.com/apache/calcite/pull/2019#discussion_r446743740
##
File path: core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java
##
@@ -117,6 +120,47 @@
SqlTypeCoercionRule.THREAD_PROVIDERS.set(defaultRules);
}
+ @Test void testEqualAsCollectionSansNullability() {
+final SqlTypeFactoryImpl factory =
+new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RelDataType bigint = factory.createSqlType(SqlTypeName.BIGINT);
+
+// case array
+final RelDataType arrayNullable =
+new ArraySqlType(factory.createTypeWithNullability(bigint, true),
false);
+final RelDataType arrayNotNull =
+new ArraySqlType(factory.createTypeWithNullability(bigint, false),
false);
+assertThat(equalAsCollectionSansNullability(factory, arrayNullable,
arrayNotNull),
+is(true));
+
+// case multiset
+final RelDataType setNullable =
+new MultisetSqlType(factory.createTypeWithNullability(bigint, true),
false);
+final RelDataType setNotNull =
+new MultisetSqlType(factory.createTypeWithNullability(bigint, false),
false);
+assertThat(equalAsCollectionSansNullability(factory, setNullable,
setNotNull),
+is(true));
+
+// multiset and array are not equal.
+assertThat(equalAsCollectionSansNullability(factory, arrayNotNull,
setNotNull),
+is(false));
+ }
+
+ @Test void testEqualAsMapSansNullability() {
+final SqlTypeFactoryImpl factory =
+new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+final RelDataType bigint = factory.createSqlType(SqlTypeName.BIGINT);
Review comment:
Use type from `SqlTypeFixture` if possible.
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:
us...@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #2019: [CALCITE-4059] SqlTypeUtil#equalSansNullability consider Array/Map type.
danny0405 commented on a change in pull request #2019:
URL: https://github.com/apache/calcite/pull/2019#discussion_r446743961
##
File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
##
@@ -1160,6 +1161,49 @@ public static boolean equalSansNullability(
factory.createTypeWithNullability(type2, type1.isNullable()));
}
+ /**
+ * Returns whether two array/multiset types are equal, ignoring nullability.
+ *
+ * They need not come from the same factory.
+ *
+ * @param factory Type factory
+ * @param type1 First type
+ * @param type2 Second type
+ * @return Whether types are equal, ignoring nullability
+ */
+ public static boolean equalAsCollectionSansNullability(
+ RelDataTypeFactory factory,
+ RelDataType type1,
+ RelDataType type2) {
+Preconditions.checkArgument(isCollection(type1));
+Preconditions.checkArgument(isCollection(type2));
+return type1.getSqlTypeName() == type2.getSqlTypeName()
+&& equalSansNullability(factory, type1.getComponentType(),
type2.getComponentType());
+ }
+
+ /**
+ * Returns whether two map types are equal, ignoring nullability.
+ *
+ * They need not come from the same factory.
+ *
+ * @param factory Type factory
+ * @param type1 First type
+ * @param type2 Second type
+ * @return Whether types are equal, ignoring nullability
+ */
+ public static boolean equalAsMapSansNullability(
+ RelDataTypeFactory factory,
+ RelDataType type1,
+ RelDataType type2) {
+Preconditions.checkArgument(isMap(type1));
Review comment:
Add `type1 == type2` check to see if they are equals for fast check.
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:
us...@infra.apache.org
[GitHub] [calcite] danny0405 commented on a change in pull request #2029: [CALCITE-4066] SqlTypeUtil#convertTypeToSpec cover Array/Multiset/Row types.
danny0405 commented on a change in pull request #2029:
URL: https://github.com/apache/calcite/pull/2029#discussion_r446744526
##
File path: core/src/test/java/org/apache/calcite/sql/type/SqlTypeUtilTest.java
##
@@ -117,6 +126,37 @@
SqlTypeCoercionRule.THREAD_PROVIDERS.set(defaultRules);
}
+ @Test void testConvertTypeToSpec() {
+SqlBasicTypeNameSpec basicSpec =
+(SqlBasicTypeNameSpec)
convertTypeToSpec(f.sqlBigInt).getTypeNameSpec();
+assertThat(basicSpec.getTypeName().getSimple(), is("BIGINT"));
+
+SqlCollectionTypeNameSpec arraySpec =
+(SqlCollectionTypeNameSpec)
convertTypeToSpec(f.arrayBigInt).getTypeNameSpec();
+assertThat(arraySpec.getTypeName().getSimple(), is("ARRAY"));
+assertThat(arraySpec.getElementTypeName().getTypeName().getSimple(),
is("BIGINT"));
+
+SqlCollectionTypeNameSpec multisetSpec =
+(SqlCollectionTypeNameSpec)
convertTypeToSpec(f.multisetBigInt).getTypeNameSpec();
+assertThat(multisetSpec.getTypeName().getSimple(), is("MULTISET"));
+assertThat(multisetSpec.getElementTypeName().getTypeName().getSimple(),
is("BIGINT"));
+
+SqlRowTypeNameSpec rowSpec =
+(SqlRowTypeNameSpec)
convertTypeToSpec(f.structOfInt).getTypeNameSpec();
+List fieldNames = rowSpec.getFieldNames()
+.stream()
+.map(SqlIdentifier::getSimple)
+.collect(Collectors.toList());
+List fieldTypeNames = rowSpec.getFieldTypes()
+.stream()
+.map(f -> f.getTypeName().getSimple())
+.collect(Collectors.toList());
+assertThat(rowSpec.getTypeName().getSimple(), is("ROW"));
+assertThat(fieldNames, is(Arrays.asList("i", "j")));
+assertThat(fieldTypeNames, is(Arrays.asList("INTEGER", "INTEGER")));
+System.out.println();
+ }
Review comment:
Remove `System.out.println();`.
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:
us...@infra.apache.org
[GitHub] [calcite] julianhyde opened a new pull request #2047: [CALCITE-3936] JDBC adapter, when generating SQL, changes target of ambiguous HAVING clause with a Project on Filter on Aggregate
julianhyde opened a new pull request #2047: URL: https://github.com/apache/calcite/pull/2047 In order to fix https://issues.apache.org/jira/browse/CALCITE-3936, we needed significant refactoring of `SqlImplementor` and `RelToSqlConverter`. There are various changes to the signatures of protected methods; I don't consider them public APIs: Renamed `SqlImplementor.visitChild` to `visitInput`, replaced `Sqlmplementor.Result.builder(RelNode, Clause...)` with two methods, `builder(RelNode)` and `builder(RelNode, Clause, Clause...)`, the latter of which is deprecated. The changes have not yet been squashed, and incorrectly reference CALCITE-3896; should be CALCITE-3936. 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: us...@infra.apache.org
