[jira] [Commented] (CALCITE-2266) Implement SQL 2016 JSON functions

[Vladimir Sitnikov (JIRA)]

    [ 
https://issues.apache.org/jira/browse/CALCITE-2266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16675271#comment-16675271
 ] 

Vladimir Sitnikov commented on CALCITE-2266:
--------------------------------------------

The feature looks great, and it definitely took tremendous resources to build, 
however I wonder if tests could print human-readable (and undstandable) 
messages in case of failures.

For instance, I've taken a sample from 
{code:java}
  @Test public void testJsonApiCommonSyntax() {
    assertJsonApiCommonSyntaxPathReturned(
        SqlFunctions.
            jsonApiCommonSyntax(ImmutableMap.of("foo", "bar"),
                "strict $.foor"), // <-- Note "r" here
        is("bar"));
  }
{code}

The exception is as follows:
{noformat}
Expected: a instance of PathContext, and the property [pathReturned] is "bar"
     but: was <PathContext{mode=STRICT, pathReturned=null, 
exc=com.jayway.jsonpath.PathNotFoundException: No results for path: $['foor']}>
Expected :a instance of PathContext, and the property [pathReturned] is "bar"
Actual   :<PathContext{mode=STRICT, pathReturned=null, 
exc=com.jayway.jsonpath.PathNotFoundException: No results for path: $['foor']}>
 <Click to see difference>


        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.junit.Assert.assertThat(Assert.java:956)
        at org.junit.Assert.assertThat(Assert.java:923)
        at 
org.apache.calcite.test.SqlFunctionsTest.assertObjectProperty(SqlFunctionsTest.java:848)
        at 
org.apache.calcite.test.SqlFunctionsTest.assertJsonApiCommonSyntaxPathReturned(SqlFunctionsTest.java:871)
        at 
org.apache.calcite.test.SqlFunctionsTest.testJsonApiCommonSyntax(SqlFunctionsTest.java:900)
{noformat}

Frankly speaking, the exception is very moot.

Here's how the same test could be corrected to produce much cleaner exceptions:

{code:java}
  @Test public void testJsonApiCommonSyntax() {
    assertJsonApiCommonSyntaxPathReturned2(
        ImmutableMap.of("foo", "bar"),
        "strict $.foor",
        is("bar"));
  }

  private void assertJsonApiCommonSyntaxPathReturned2(Map<String, Object> 
context,
      String jsonPath, Matcher<Object> matcher) {
    SqlFunctions.PathContext pathContext = 
SqlFunctions.jsonApiCommonSyntax(context, jsonPath);
    assertThat("jsonApiCommonSyntax(" + context + ", " + jsonPath + ")",
        pathContext.pathReturned, matcher);
  }
{code}

Now it produces
{noformat}
java.lang.AssertionError: jsonApiCommonSyntax({foo=bar}, strict $.foor)
Expected: is "bar"
     but: was null
Expected :is "bar"
Actual   :null
 <Click to see difference>


        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.junit.Assert.assertThat(Assert.java:956)
        at 
org.apache.calcite.test.SqlFunctionsTest.assertJsonApiCommonSyntaxPathReturned2(SqlFunctionsTest.java:876)
        at 
org.apache.calcite.test.SqlFunctionsTest.testJsonApiCommonSyntax(SqlFunctionsTest.java:906)
{noformat}


[~zhztheplayer], would you please update test code accordingly?

PS. I think it is worth putting the newly added tests to its own file instead 
of abusing the existing {{SqlFunctionsTest}}


> Implement SQL 2016 JSON functions
> ---------------------------------
>
>                 Key: CALCITE-2266
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2266
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Michael Mior
>            Priority: Major
>             Fix For: 1.18.0
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)