Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1208

Improve two-level column structure handling

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.0
    • Fix Version/s: 1.9.0
    • Component/s: core
    • Labels:

      Description

      Calcite now has support for nested column structure in parsing and validation, by representing the inner-level columns as a RexFieldAccess based on a RexInputRef. Meanwhile it does not flatten the inner level structure in wildcard expansion, which would then cause an UnsupportedOperationException in Avatica.

      The idea is to take into account this nested structure in column resolving, but to flatten the structure when translating to RelNode/RexNode.
      For example, if the table structure is defined as

      VARCHAR K0,
      VARCHAR C1,
      RecordType(INTEGER C0, INTEGER C1) F0,
      RecordType(INTEGER C0, INTEGER C2) F1

      , it should be viewed as a flat type like

      VARCHAR K0,
      VARCHAR C1,
      INTEGER F0.C0,
      INTEGER F0.C1,
      INTEGER F1.C0,
      INTEGER F1.C2

      , so that:
      1) Column reference "K0" is translated as $0
      2) Column reference "F0.C1" is translated as $3
      3) Wildcard "*" is translated as: $0, $1, $2, $3, $4, $5
      4) Complex-column wildcard "F1.*", which is translated as $2, $3
      And we would like to resolve columns based on the following rules (here we only consider the "suffix" part of the qualified names, which means the table resolving is already done by this time):
      a) A two-part column name is matched with its first-level column name and its second-level column name. For example, "F1.C0" corresponds to $4; "F1,X" will throw a column not found error.
      b) A single-part column name is matched against non-nested columns first, and if no matches, it is then matched against those second-level column names. For example, "C1" will be matched as "$1" instead of "$3", since non-nested columns have a higher priority; "C2" will be matched as "$5"; "C0" will lead to an ambiguous column error, since it exists under both "F0" and "F1".
      c) We would also like to have a way for defining "default first-level column" so that it has a precedence in column resolving over other first-level columns. For example, if "F0" is defined as default, "C0" will not cause an ambiguous column error, but instead be matched as "$2".
      d) Reference to first-level column only without wildcard is not allowed, e.g., "F1".

        Issue Links

          Activity

          Hide
          maryannxue Maryann Xue added a comment -

          Julian Hyde, James Taylor, would you mind reviewing the description and see whether I've missed anything or made any mistakes there?

          Show
          maryannxue Maryann Xue added a comment - Julian Hyde , James Taylor , would you mind reviewing the description and see whether I've missed anything or made any mistakes there?
          Hide
          jamestaylor James Taylor added a comment -

          +1 Maryann Xue. Looks right to me.

          Show
          jamestaylor James Taylor added a comment - +1 Maryann Xue . Looks right to me.
          Hide
          julianhyde Julian Hyde added a comment -

          I'm swamped for the next day or two but will review as soon as I can.

          Show
          julianhyde Julian Hyde added a comment - I'm swamped for the next day or two but will review as soon as I can.
          Hide
          maryannxue Maryann Xue added a comment -

          Added validator test cases in https://github.com/maryannxue/calcite/tree/calcite-1208.
          Found out that my previous statement was wrong, since Calcite did flatten the structure for "select star" and "select column" at a later stage by adding an extra Project rel", although the biggest con would be insufficient field trimming. Basically it always selects all families regardless of which columns are used.
          I was still struggling with what interface we'd use for specifying the column family structure and the default column family. Say, if we specify a nested column structure just as I did for the test table "T" and let calcite flatten it "secretly", would it sound inconsistent if "select * from T" appears a different type from table "T"?
          I was also thinking if we could achieve this by having a special implementation for RelDataType.

          Show
          maryannxue Maryann Xue added a comment - Added validator test cases in https://github.com/maryannxue/calcite/tree/calcite-1208 . Found out that my previous statement was wrong, since Calcite did flatten the structure for "select star" and "select column" at a later stage by adding an extra Project rel", although the biggest con would be insufficient field trimming. Basically it always selects all families regardless of which columns are used. I was still struggling with what interface we'd use for specifying the column family structure and the default column family. Say, if we specify a nested column structure just as I did for the test table "T" and let calcite flatten it "secretly", would it sound inconsistent if "select * from T" appears a different type from table "T"? I was also thinking if we could achieve this by having a special implementation for RelDataType.
          Hide
          julianhyde Julian Hyde added a comment -

          Is CALCITE-1150 useful for that "special implementation for RelDataType"?

          Show
          julianhyde Julian Hyde added a comment - Is CALCITE-1150 useful for that "special implementation for RelDataType"?
          Hide
          jni Jinfeng Ni added a comment -

          @maryannxue, can you give some examples of sql query that you want to support from CALCITE-1208? I guess this feature might be useful for Drill as well. thanks.

          Show
          jni Jinfeng Ni added a comment - @maryannxue, can you give some examples of sql query that you want to support from CALCITE-1208 ? I guess this feature might be useful for Drill as well. thanks.
          Hide
          maryannxue Maryann Xue added a comment -

          Hi Jinfeng Ni, please check this file for a (hopefully complete) list of query examples https://github.com/apache/calcite/compare/master...maryannxue:calcite-1208#diff-0a6b96592dd053cb0697ea9ebfa3eea4.
          Let me know if it helps. Phoenix follows the hierarchical column naming convention of HBase, which is ColumnFamilyName.ColumnName, but this is only for name resolving. We would like to have the a flat structure as table type.

          Show
          maryannxue Maryann Xue added a comment - Hi Jinfeng Ni , please check this file for a (hopefully complete) list of query examples https://github.com/apache/calcite/compare/master...maryannxue:calcite-1208#diff-0a6b96592dd053cb0697ea9ebfa3eea4 . Let me know if it helps. Phoenix follows the hierarchical column naming convention of HBase, which is ColumnFamilyName.ColumnName, but this is only for name resolving. We would like to have the a flat structure as table type.
          Hide
          julianhyde Julian Hyde added a comment -

          Assigned to myself. I'm working on this now.

          Show
          julianhyde Julian Hyde added a comment - Assigned to myself. I'm working on this now.
          Hide
          jni Jinfeng Ni added a comment -

          Couple of comments.

          • Can we add comment / example to explain the three new StructKind? What are the implication for each StructKind?
          • For the 3rd case in testStructType() "select c2 from struct.t", I'm not sure if it makes sense to use "c2" to refer a sub-field under field "F1". Are we aware of any other system that would allow such behavior?
          Show
          jni Jinfeng Ni added a comment - Couple of comments. Can we add comment / example to explain the three new StructKind? What are the implication for each StructKind? For the 3rd case in testStructType() "select c2 from struct.t", I'm not sure if it makes sense to use "c2" to refer a sub-field under field "F1". Are we aware of any other system that would allow such behavior?
          Hide
          julianhyde Julian Hyde added a comment -

          Can we add comment / example to explain the three new StructKind? What are the implication for each StructKind?

          Yes, we will improve the description of each StructKind.

          For the 3rd case in testStructType() "select c2 from struct.t", I'm not sure if it makes sense to use "c2" to refer a sub-field under field "F1". Are we aware of any other system that would allow such behavior?

          It isn't the "normal" behavior of SQL but it absolutely makes sense in Phoenix/HBase because HBase organizes columns into column families and you want to be able to optionally qualify a column with its family name. (Qualifying the column becomes mandatory if the same name occurs in more than one column family.) If your database doesn't need those semantics, don't use that particular StructKind.

          Show
          julianhyde Julian Hyde added a comment - Can we add comment / example to explain the three new StructKind? What are the implication for each StructKind? Yes, we will improve the description of each StructKind. For the 3rd case in testStructType() "select c2 from struct.t", I'm not sure if it makes sense to use "c2" to refer a sub-field under field "F1". Are we aware of any other system that would allow such behavior? It isn't the "normal" behavior of SQL but it absolutely makes sense in Phoenix/HBase because HBase organizes columns into column families and you want to be able to optionally qualify a column with its family name. (Qualifying the column becomes mandatory if the same name occurs in more than one column family.) If your database doesn't need those semantics, don't use that particular StructKind.
          Hide
          julianhyde Julian Hyde added a comment -

          I finally have a draft change that can be reviewed (see https://github.com/julianhyde/calcite/tree/1208-column-family commit e65a664). SqlValidatorTest and SqlToRelConverterTest pass 100%, including the tests for column families that Maryann Xue wrote, and previous tests for CALCITE-1150 and CALCITE-1305. But I have quite a bit of clean up still to do.

          Show
          julianhyde Julian Hyde added a comment - I finally have a draft change that can be reviewed (see https://github.com/julianhyde/calcite/tree/1208-column-family commit e65a664). SqlValidatorTest and SqlToRelConverterTest pass 100%, including the tests for column families that Maryann Xue wrote, and previous tests for CALCITE-1150 and CALCITE-1305 . But I have quite a bit of clean up still to do.
          Hide
          julianhyde Julian Hyde added a comment -

          The latest commit to julianhyde/master is good to go. Maryann Xue, Jinfeng Ni, can you please review before I push it to Apache?

          Show
          julianhyde Julian Hyde added a comment - The latest commit to julianhyde/master is good to go. Maryann Xue , Jinfeng Ni , can you please review before I push it to Apache?
          Hide
          julianhyde Julian Hyde added a comment - - edited

          Thanks for your review comments, Jinfeng Ni. I have made fixes in commit 50e176b on https://github.com/julianhyde/calcite/tree/1208-column-family, and my responses to your questions are inline.

          Within one struct, do we allow multiple fields to have PEEK_FIELD_DEFAULT? Seems the current code does not enforce the requirement of only one field with PEEK_FIELD_DEFAULT when fields are added to struct.

          In Phoenix there would be only one field PEEK_FIELD_DEFAULT (i.e. the default column group). But in principle there could be more. If two record fields in table t were labeled PEEK_FIELD_DEFAULT, and each had a column called c, then t.c would be ambiguous because both have equal rank.

          this query is same as the one on line 7823?

          Oops, yes; fixed.

          Probably we had better add StructType (peek_fields) to this comment. Otherwise, people have to go to MockCatalogReader to see each record type's struct-type to understand these test case.

          Agreed.

          I understand the tests cover second-level columns and phenix may only need two-level columns. But looking at the definition of RelRecordType, the code will allow any-level columns, right? Is this desirable?

          You are correct that Phoenix has a maximum depth of 2, but it would work at greater depths, and it would be useful. Suppose you have a table with an XML (or JSON) schema that (unlike Drill) you know when validating the query. In the Orders table, you could allow customer.address.zipcode to be abbreviated as zipcode.

           T1 has
              F0 - peek_fields_default
                   C0
                   C1
                   F0  -- peek_fields
                       C0
                       C1
          

          Select F0.C0 from T1 ; Will it resolve to C0 at second level, or third level?

          If there are two matches, we choose the shorter one. It will resolve to C0 at the second level, i.e. F0.C0.

           T1 has
              F0 - peek_fields_default
                   C0
                   C1
                   F0  -- peek_fields_default
                       C0
                       C1
          

          Select C0 from t1; Will this resolve to second level C0, or third level C0?

          Again, the shorter path, so F0.C0, the second level.

          Show
          julianhyde Julian Hyde added a comment - - edited Thanks for your review comments, Jinfeng Ni . I have made fixes in commit 50e176b on https://github.com/julianhyde/calcite/tree/1208-column-family , and my responses to your questions are inline. Within one struct, do we allow multiple fields to have PEEK_FIELD_DEFAULT? Seems the current code does not enforce the requirement of only one field with PEEK_FIELD_DEFAULT when fields are added to struct. In Phoenix there would be only one field PEEK_FIELD_DEFAULT (i.e. the default column group). But in principle there could be more. If two record fields in table t were labeled PEEK_FIELD_DEFAULT, and each had a column called c, then t.c would be ambiguous because both have equal rank. this query is same as the one on line 7823? Oops, yes; fixed. Probably we had better add StructType (peek_fields) to this comment. Otherwise, people have to go to MockCatalogReader to see each record type's struct-type to understand these test case. Agreed. I understand the tests cover second-level columns and phenix may only need two-level columns. But looking at the definition of RelRecordType, the code will allow any-level columns, right? Is this desirable? You are correct that Phoenix has a maximum depth of 2, but it would work at greater depths, and it would be useful. Suppose you have a table with an XML (or JSON) schema that (unlike Drill) you know when validating the query. In the Orders table, you could allow customer.address.zipcode to be abbreviated as zipcode. T1 has F0 - peek_fields_default C0 C1 F0 -- peek_fields C0 C1 Select F0.C0 from T1 ; Will it resolve to C0 at second level, or third level? If there are two matches, we choose the shorter one. It will resolve to C0 at the second level, i.e. F0.C0. T1 has F0 - peek_fields_default C0 C1 F0 -- peek_fields_default C0 C1 Select C0 from t1 ; Will this resolve to second level C0, or third level C0? Again, the shorter path, so F0.C0, the second level.
          Hide
          maryannxue Maryann Xue added a comment -

          Thank you, Julian Hyde, for the fix! I was trying to test it out with Phoenix's existing multi-column-family tests and realized that there was another issue in two-level column resolving: the DML parsing for compound identifiers. Right now DMLs only accept simple identifiers as column names.

          {
              (
                  <INSERT>
              |
                  <UPSERT> { keywords.add(SqlInsertKeyword.UPSERT.symbol(getPos())); }
              )
              SqlInsertKeywords(keywords)
              <INTO> table = CompoundIdentifier()
              {
                  pos = getPos();
              }
              [
                  LOOKAHEAD(2)
                  columnList = ParenthesizedSimpleIdentifierList()
              ]
              source = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY)
              {
                  return new SqlInsert(pos, new SqlNodeList(keywords, pos), table, source,
                      columnList);
              }
          }
          
          Show
          maryannxue Maryann Xue added a comment - Thank you, Julian Hyde , for the fix! I was trying to test it out with Phoenix's existing multi-column-family tests and realized that there was another issue in two-level column resolving: the DML parsing for compound identifiers. Right now DMLs only accept simple identifiers as column names. { ( <INSERT> | <UPSERT> { keywords.add(SqlInsertKeyword.UPSERT.symbol(getPos())); } ) SqlInsertKeywords(keywords) <INTO> table = CompoundIdentifier() { pos = getPos(); } [ LOOKAHEAD(2) columnList = ParenthesizedSimpleIdentifierList() ] source = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY) { return new SqlInsert(pos, new SqlNodeList(keywords, pos), table, source, columnList); } }
          Hide
          julianhyde Julian Hyde added a comment -

          Yeah, I had forgotten about that. I guess it should be a new issue (since it deals with parsing rather than validation). I think I'd like to parameterize the parser, because I think Phoenix is the only client that needs family.column in DML.

          Show
          julianhyde Julian Hyde added a comment - Yeah, I had forgotten about that. I guess it should be a new issue (since it deals with parsing rather than validation). I think I'd like to parameterize the parser, because I think Phoenix is the only client that needs family.column in DML.
          Hide
          maryannxue Maryann Xue added a comment -

          Yes, totally agree, Julian Hyde

          Show
          maryannxue Maryann Xue added a comment - Yes, totally agree, Julian Hyde
          Hide
          maryannxue Maryann Xue added a comment -

          Julian Hyde, I just managed to try out the Phoenix test case with this branch. I hit an IndexOutOfBoundException though. I will go further to find out the cause and let you know.

          Show
          maryannxue Maryann Xue added a comment - Julian Hyde , I just managed to try out the Phoenix test case with this branch. I hit an IndexOutOfBoundException though. I will go further to find out the cause and let you know.
          Hide
          maryannxue Maryann Xue added a comment -

          I can repo the issue with a Calcite test case below. The validation is good, but the exception occurred in SqlToRelConverter.

          --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
          +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
          @@ -1709,6 +1709,9 @@ protected final void check(
           
             @Test public void testStructType() {
               sql("select * from struct.t").convertsTo("${plan}");
          +
          +    // Resolve F1.C2 as fully qualified column F1.C2.
          +    sql("select f1.c2 from struct.t").convertsTo("${plan}");;
             }
           
             /**
          

          The IndexOutOfBoundException I got:

          java.lang.IndexOutOfBoundsException: index (-1) must not be negative
          	at com.google.common.base.Preconditions.checkElementIndex(Preconditions.java:310)
          	at com.google.common.base.Preconditions.checkElementIndex(Preconditions.java:292)
          	at com.google.common.collect.RegularImmutableList.get(RegularImmutableList.java:65)
          	at org.apache.calcite.util.Util.last(Util.java:2022)
          	at org.apache.calcite.sql.validate.ListScope.findChild(ListScope.java:74)
          	at org.apache.calcite.sql.validate.ListScope.resolve(ListScope.java:166)
          	at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.lookupExp(SqlToRelConverter.java:3879)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertIdentifier(SqlToRelConverter.java:3248)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.access$4(SqlToRelConverter.java:3233)
          	at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4271)
          	at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:1)
          	at org.apache.calcite.sql.SqlIdentifier.accept(SqlIdentifier.java:324)
          	at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4152)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectList(SqlToRelConverter.java:3465)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:665)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:622)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:2841)
          	at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556)
          	at org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:541)
          	at org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:649)
          	at org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:1932)
          	at org.apache.calcite.test.SqlToRelConverterTest.testStructType(SqlToRelConverterTest.java:1714)
          	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
          	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          	at java.lang.reflect.Method.invoke(Method.java:497)
          	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
          	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
          	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
          	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
          	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
          	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
          	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
          	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
          	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
          	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
          	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
          	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
          	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
          	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
          	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
          	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
          	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
          	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
          	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
          
          Show
          maryannxue Maryann Xue added a comment - I can repo the issue with a Calcite test case below. The validation is good, but the exception occurred in SqlToRelConverter. --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -1709,6 +1709,9 @@ protected final void check( @Test public void testStructType() { sql( "select * from struct.t" ).convertsTo( "${plan}" ); + + // Resolve F1.C2 as fully qualified column F1.C2. + sql( "select f1.c2 from struct.t" ).convertsTo( "${plan}" );; } /** The IndexOutOfBoundException I got: java.lang.IndexOutOfBoundsException: index (-1) must not be negative at com.google.common.base.Preconditions.checkElementIndex(Preconditions.java:310) at com.google.common.base.Preconditions.checkElementIndex(Preconditions.java:292) at com.google.common.collect.RegularImmutableList.get(RegularImmutableList.java:65) at org.apache.calcite.util.Util.last(Util.java:2022) at org.apache.calcite.sql.validate.ListScope.findChild(ListScope.java:74) at org.apache.calcite.sql.validate.ListScope.resolve(ListScope.java:166) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.lookupExp(SqlToRelConverter.java:3879) at org.apache.calcite.sql2rel.SqlToRelConverter.convertIdentifier(SqlToRelConverter.java:3248) at org.apache.calcite.sql2rel.SqlToRelConverter.access$4(SqlToRelConverter.java:3233) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:4271) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.visit(SqlToRelConverter.java:1) at org.apache.calcite.sql.SqlIdentifier.accept(SqlIdentifier.java:324) at org.apache.calcite.sql2rel.SqlToRelConverter$Blackboard.convertExpression(SqlToRelConverter.java:4152) at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectList(SqlToRelConverter.java:3465) at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelectImpl(SqlToRelConverter.java:665) at org.apache.calcite.sql2rel.SqlToRelConverter.convertSelect(SqlToRelConverter.java:622) at org.apache.calcite.sql2rel.SqlToRelConverter.convertQueryRecursive(SqlToRelConverter.java:2841) at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:556) at org.apache.calcite.test.SqlToRelTestBase$TesterImpl.convertSqlToRel(SqlToRelTestBase.java:541) at org.apache.calcite.test.SqlToRelTestBase$TesterImpl.assertConvertsTo(SqlToRelTestBase.java:649) at org.apache.calcite.test.SqlToRelConverterTest$Sql.convertsTo(SqlToRelConverterTest.java:1932) at org.apache.calcite.test.SqlToRelConverterTest.testStructType(SqlToRelConverterTest.java:1714) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) at org.junit.runners.ParentRunner.run(ParentRunner.java:363) at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          It seems this needs more work; I am deferring it to 1.10.0.

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - It seems this needs more work; I am deferring it to 1.10.0.
          Hide
          julianhyde Julian Hyde added a comment -

          What is done already is good enough to go into 1.9. What Maryann has found is a bug, not an architectural problem. If we keep it on a separate branch it will be difficult to merge.

          Show
          julianhyde Julian Hyde added a comment - What is done already is good enough to go into 1.9. What Maryann has found is a bug, not an architectural problem. If we keep it on a separate branch it will be difficult to merge.
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/a859f93 and http://git-wip-us.apache.org/repos/asf/calcite/commit/0938c7b .

          Please, leave a comment if there are any issues left.

          Thanks for the PR Maryann Xue and Julian Hyde, and Jinfeng Ni for the feedback.

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/a859f93 and http://git-wip-us.apache.org/repos/asf/calcite/commit/0938c7b . Please, leave a comment if there are any issues left. Thanks for the PR Maryann Xue and Julian Hyde , and Jinfeng Ni for the feedback.
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Resolved in release 1.9.0 (2016-09-22)

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.9.0 (2016-09-22)

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              maryannxue Maryann Xue
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development