[GitHub] carbondata pull request #1447: [CARBONDATA-1611] Reject Update and Delete op...

[qiuchenjian-2]

GitHub user jackylk opened a pull request:

    https://github.com/apache/carbondata/pull/1447

    [CARBONDATA-1611] Reject Update and Delete operation for streaming table

    Streaming table is not mutable, data update and delete operation should be rejected.
    An exception will be thrown in the scenario.
   
     - [X] Any interfaces changed?
        No
   
     - [X] Any backward compatibility impacted?
        No
   
     - [X] Document update required?
        Yes
   
     - [ ] Testing done
       Yes, one testcase added
         
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
        NA      


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jackylk/incubator-carbondata block_iud

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1447.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1447
   
----
commit f050774de36e0fe8fb16495f3e8c5cf941e9690d
Author: Jacky Li <[hidden email]>
Date:   2017-10-29T16:01:21Z

    block iud

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/730/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1367/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/733/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1370/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/744/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1381/



---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148073749
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/TestStreamingTableOperation.scala ---
    @@ -0,0 +1,63 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.carbondata
    +
    +import org.apache.spark.sql.test.util.QueryTest
    +import org.scalatest.BeforeAndAfterAll
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +class TestStreamingTableOperation extends QueryTest with BeforeAndAfterAll {
    +  override def beforeAll {
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    sql("CREATE DATABASE streaming")
    +    sql("USE streaming")
    +    sql(
    +      """
    +        | create table source(
    +        |    c1 string,
    +        |    c2 int,
    +        |    c3 string,
    +        |    c5 string
    +        | ) STORED BY 'org.apache.carbondata.format'
    +        | TBLPROPERTIES ('streaming' = 'true')
    +      """.stripMargin)
    +    sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE source""")
    +  }
    +
    +
    +  test("test blocking update and delete operation on streaming table") {
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""UPDATE source d SET (d.c2) = (d.c2 + 1) WHERE d.c1 = 'a'""").show()
    +    }
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""DELETE FROM source WHERE d.c1 = 'a'""").show()
    +    }
    +  }
    +
    +  override def afterAll {
    +    sql("USE default")
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    CarbonProperties.getInstance()
    --- End diff --
   
    These code no need to add.
    -------------------------
     CarbonProperties.getInstance()
        .addProperty(CarbonCommonConstants.isHorizontalCompactionEnabled , "true")
       CarbonProperties.getInstance()
        .addProperty(CarbonCommonConstants.ENABLE_VECTOR_READER , "true")


---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148073914
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/TestStreamingTableOperation.scala ---
    @@ -0,0 +1,63 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.carbondata
    +
    +import org.apache.spark.sql.test.util.QueryTest
    +import org.scalatest.BeforeAndAfterAll
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +class TestStreamingTableOperation extends QueryTest with BeforeAndAfterAll {
    +  override def beforeAll {
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    sql("CREATE DATABASE streaming")
    +    sql("USE streaming")
    +    sql(
    +      """
    +        | create table source(
    +        |    c1 string,
    +        |    c2 int,
    +        |    c3 string,
    +        |    c5 string
    +        | ) STORED BY 'org.apache.carbondata.format'
    +        | TBLPROPERTIES ('streaming' = 'true')
    +      """.stripMargin)
    +    sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE source""")
    +  }
    +
    +
    +  test("test blocking update and delete operation on streaming table") {
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""UPDATE source d SET (d.c2) = (d.c2 + 1) WHERE d.c1 = 'a'""").show()
    +    }
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""DELETE FROM source WHERE d.c1 = 'a'""").show()
    +    }
    +  }
    +
    +  override def afterAll {
    --- End diff --
   
    in afterAll {}, please drop all created tables.
   



---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148074211
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/StreamingTableStrategy.scala ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.strategy
    +
    +import org.apache.spark.sql.{CarbonEnv, SparkSession}
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.execution.{SparkPlan, SparkStrategy}
    +import org.apache.spark.sql.execution.command.ExecutedCommandExec
    +import org.apache.spark.sql.execution.command.mutation.{DeleteExecution, ProjectForDeleteCommand, ProjectForUpdateCommand}
    +import org.apache.spark.sql.hive.CarbonRelation
    +
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +/**
    + * Strategy for streaming table, like blocking unsupported operation
    --- End diff --
   
    align the comment.


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148075998
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/TestStreamingTableOperation.scala ---
    @@ -0,0 +1,63 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.carbondata
    +
    +import org.apache.spark.sql.test.util.QueryTest
    +import org.scalatest.BeforeAndAfterAll
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +class TestStreamingTableOperation extends QueryTest with BeforeAndAfterAll {
    +  override def beforeAll {
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    sql("CREATE DATABASE streaming")
    +    sql("USE streaming")
    +    sql(
    +      """
    +        | create table source(
    +        |    c1 string,
    +        |    c2 int,
    +        |    c3 string,
    +        |    c5 string
    +        | ) STORED BY 'org.apache.carbondata.format'
    +        | TBLPROPERTIES ('streaming' = 'true')
    +      """.stripMargin)
    +    sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE source""")
    +  }
    +
    +
    +  test("test blocking update and delete operation on streaming table") {
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""UPDATE source d SET (d.c2) = (d.c2 + 1) WHERE d.c1 = 'a'""").show()
    +    }
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""DELETE FROM source WHERE d.c1 = 'a'""").show()
    +    }
    +  }
    +
    +  override def afterAll {
    +    sql("USE default")
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    CarbonProperties.getInstance()
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148076106
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/TestStreamingTableOperation.scala ---
    @@ -0,0 +1,63 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.carbondata
    +
    +import org.apache.spark.sql.test.util.QueryTest
    +import org.scalatest.BeforeAndAfterAll
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants
    +import org.apache.carbondata.core.util.CarbonProperties
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +class TestStreamingTableOperation extends QueryTest with BeforeAndAfterAll {
    +  override def beforeAll {
    +    sql("DROP DATABASE IF EXISTS streaming CASCADE")
    +    sql("CREATE DATABASE streaming")
    +    sql("USE streaming")
    +    sql(
    +      """
    +        | create table source(
    +        |    c1 string,
    +        |    c2 int,
    +        |    c3 string,
    +        |    c5 string
    +        | ) STORED BY 'org.apache.carbondata.format'
    +        | TBLPROPERTIES ('streaming' = 'true')
    +      """.stripMargin)
    +    sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE source""")
    +  }
    +
    +
    +  test("test blocking update and delete operation on streaming table") {
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""UPDATE source d SET (d.c2) = (d.c2 + 1) WHERE d.c1 = 'a'""").show()
    +    }
    +    intercept[MalformedCarbonCommandException] {
    +      sql("""DELETE FROM source WHERE d.c1 = 'a'""").show()
    +    }
    +  }
    +
    +  override def afterAll {
    --- End diff --
   
    DROP DATABASE IF EXISTS streaming CASCADE will drop all tables


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1447#discussion_r148076563
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/StreamingTableStrategy.scala ---
    @@ -0,0 +1,62 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.strategy
    +
    +import org.apache.spark.sql.{CarbonEnv, SparkSession}
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.execution.{SparkPlan, SparkStrategy}
    +import org.apache.spark.sql.execution.command.ExecutedCommandExec
    +import org.apache.spark.sql.execution.command.mutation.{DeleteExecution, ProjectForDeleteCommand, ProjectForUpdateCommand}
    +import org.apache.spark.sql.hive.CarbonRelation
    +
    +import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
    +
    +/**
    + * Strategy for streaming table, like blocking unsupported operation
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/782/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1415/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/797/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1431/



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/815/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1447
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1447/



---