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

CalciteSignature's ColumnMetaData for DELETE should be same as INSERT

    Details

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

      Description

      DELETE, as one type of TableModify operation, has the same RelDataType as INSERT, which is RelRecordType(ROWCOUNT INTEGER). But during "prepare" stage, the corresponding ColumnMetaData info becomes inconsistent, due to:

            preparedResult = preparingStmt.prepareSql(
                sqlNode, Object.class, validator, true);
            switch (sqlNode.getKind()) {
            case INSERT:
            case EXPLAIN:
              // FIXME: getValidatedNodeType is wrong for DML
              x = RelOptUtil.createDmlRowType(sqlNode.getKind(), typeFactory);
              break;
            default:
              x = validator.getValidatedNodeType(sqlNode);
            }
      

      I've noticed that there is a "FIXME: getValidatedNodeType is wrong for DML". Guess that's the root cause, and RelOptUtil.createDmlRowType() is probably a workaround? For now, we can simply include DELETE and other TableModify Operation in the first switch case.

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        Yes, I think DELETE (and UPDATE and MERGE) should be handled in exactly the same way as INSERT. That is, it returns a single count column.

        Show
        julianhyde Julian Hyde added a comment - Yes, I think DELETE (and UPDATE and MERGE) should be handled in exactly the same way as INSERT. That is, it returns a single count column.
        Hide
        maryannxue Maryann Xue added a comment -

        The fix is straightforward and easy, but I had a problem when I was trying to add a test into JdbcFrontLinqBackTest. It hangs till it ends with an OOM error.

          @Test public void testDelete() {
            final List<JdbcTest.Employee> employees = new ArrayList<>();
            CalciteAssert.AssertThat with = mutable(employees);
            with.query("select * from \"foo\".\"bar\"")
                .returns(
                    "empid=0; deptno=0; name=first; salary=0.0; commission=null\n");
            with.query("insert into \"foo\".\"bar\" select * from \"hr\".\"emps\"")
                .updates(4);
            with.query("select count(*) as c from \"foo\".\"bar\"")
                .returns("C=5\n");
            with.query("delete from \"foo\".\"bar\" "
                + "where \"deptno\" = 10")
                .typeIs("");
            with.query("select \"name\", count(*) as c from \"foo\".\"bar\" "
                + "group by \"name\"")
                .returnsUnordered(
                    "name=Eric; C=1",
                    "name=first; C=1");
          }
        
        Show
        maryannxue Maryann Xue added a comment - The fix is straightforward and easy, but I had a problem when I was trying to add a test into JdbcFrontLinqBackTest. It hangs till it ends with an OOM error. @Test public void testDelete() { final List<JdbcTest.Employee> employees = new ArrayList<>(); CalciteAssert.AssertThat with = mutable(employees); with.query( "select * from \" foo\ ".\" bar\"") .returns( "empid=0; deptno=0; name=first; salary=0.0; commission= null \n" ); with.query( "insert into \" foo\ ".\" bar\ " select * from \" hr\ ".\" emps\"") .updates(4); with.query( "select count(*) as c from \" foo\ ".\" bar\"") .returns( "C=5\n" ); with.query( "delete from \" foo\ ".\" bar\ " " + "where \" deptno\ " = 10" ) .typeIs(""); with.query( "select \" name\ ", count(*) as c from \" foo\ ".\" bar\ " " + "group by \" name\"") .returnsUnordered( "name=Eric; C=1" , "name=first; C=1" ); }
        Hide
        maryannxue Maryann Xue added a comment - - edited

        Julian Hyde, Could you please take a look at this patch? I'm going to file a couple of other JIRAs for the rest of DML ops. The way I implement "equals()" JdbcTest data object classes would probably make UPDATE easier.

        Show
        maryannxue Maryann Xue added a comment - - edited Julian Hyde , Could you please take a look at this patch? I'm going to file a couple of other JIRAs for the rest of DML ops. The way I implement "equals()" JdbcTest data object classes would probably make UPDATE easier.
        Hide
        julianhyde Julian Hyde added a comment -

        Will do - after the release closes.

        Show
        julianhyde Julian Hyde added a comment - Will do - after the release closes.
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/76616379 . Maryann Xue , thanks for the patch!
        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:
            maryannxue Maryann Xue
            Reporter:
            maryannxue Maryann Xue
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development