[CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string #2961

zygitup · 2018-11-28T12:54:27Z

Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:

Any interfaces changed?
Any backward compatibility impacted?
Document update required?
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

…ng causes the select table data to be empty

CarbonDataQA · 2018-11-28T12:55:06Z

Can one of the admins verify this patch?

qiuchenjian · 2018-11-28T14:03:44Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -248,7 +248,7 @@ object CarbonSession {

        session = new CarbonSession(sparkContext, None, !enableInMemCatlog)
        val carbonProperties = CarbonProperties.getInstance()
-        if (storePath != null) {
+        if (storePath != null && !storePath.trim.isEmpty) {


why not use StringUtils or other String tools

Which method? Can you give details? @qiuchenjian

such as StringUtils.isNotEmpty

StringUtils.isNotEmpty shouldn't support " "

Use StringUtils.isNotBlank.

chenliang613 · 2018-11-28T15:51:54Z

add to whitelist

chenliang613 · 2018-11-28T15:52:39Z

the pr title is not correct, the format should be : [JIRA NUMBER] PR description

CarbonDataQA · 2018-11-28T16:04:23Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1574/

CarbonDataQA · 2018-11-28T17:03:53Z

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1786/

CarbonDataQA · 2018-11-28T17:16:27Z

Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9833/

xubo245 · 2018-11-29T02:16:14Z

Please optimize the title, for example: [CARBONDATA-3119]Fix the error of getOrCreateCarbonSession method parameter is an empty string
Please optimize the PR content, keep the checklist format and finish the checklist

xubo245 · 2018-11-30T01:44:09Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -180,7 +180,7 @@ object CarbonSession {
      val userSuppliedContext: Option[SparkContext] =
        getValue("userSuppliedContext", builder).asInstanceOf[Option[SparkContext]]

-      if (metaStorePath != null) {
+      if (metaStorePath != null && !metaStorePath.trim.isEmpty) {


Can you add some test case to test it?

Use StringUtils.isNotBlank.

xubo245 · 2018-11-30T01:44:13Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -248,7 +248,7 @@ object CarbonSession {

        session = new CarbonSession(sparkContext, None, !enableInMemCatlog)
        val carbonProperties = CarbonProperties.getInstance()
-        if (storePath != null) {
+        if (storePath != null && !storePath.trim.isEmpty) {


Can you add some test case to test it?

xuchuanyin · 2018-11-30T02:20:59Z

+1 for @zzcclp 's comments

problem: When create carbonSession in this way: [val carbon = SparkSession.builder().config(sc.getConf).getOrCreateCarbonSession("")],The prompt can be created successfully, with no exceptions thrown during the process, but ultimately [carbon.sql("SELECT * FROM TABLE").show()] is empty. cause: [carbon.sql("SELECT * FROM TABLE").show()] is empty. solution: Increases the null or empty judgment of getOrCreateCarbonSession method parameters[storePath,metaStorePath]

CarbonDataQA · 2018-11-30T04:39:20Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1595/

CarbonDataQA · 2018-11-30T05:37:01Z

Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9855/

CarbonDataQA · 2018-11-30T05:39:10Z

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1807/

problem: When create carbonSession in this way: [val carbon = SparkSession.builder().config(sc.getConf).getOrCreateCarbonSession("")],The prompt can be created successfully, with no exceptions thrown during the process, but ultimately [carbon.sql("SELECT * FROM TABLE").show()] is empty. cause: [carbon.sql("SELECT * FROM TABLE").show()] is empty. solution: Increases the null or empty judgment of getOrCreateCarbonSession method parameters[storePath,metaStorePath]

CarbonDataQA · 2018-11-30T08:44:12Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1601/

xubo245 · 2018-11-30T08:45:25Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -180,7 +183,7 @@ object CarbonSession {
      val userSuppliedContext: Option[SparkContext] =
        getValue("userSuppliedContext", builder).asInstanceOf[Option[SparkContext]]

-      if (metaStorePath != null) {
+      if (metaStorePath != null && StringUtils.isNotBlank(metaStorePath)) {


StringUtils.isNotBlank will judge null

zzcclp · 2018-11-30T08:47:11Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -37,6 +38,8 @@ import org.apache.carbondata.core.constants.CarbonCommonConstants
 import org.apache.carbondata.core.util.{CarbonProperties, CarbonSessionInfo, ThreadLocalSessionInfo}
 import org.apache.carbondata.streaming.CarbonStreamingQueryListener

+
+


remove these two lines.

ok.Has been removed

zzcclp · 2018-11-30T08:50:25Z

integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala

@@ -248,7 +251,7 @@ object CarbonSession {

        session = new CarbonSession(sparkContext, None, !enableInMemCatlog)
        val carbonProperties = CarbonProperties.getInstance()
-        if (storePath != null) {
+        if (storePath != null && StringUtils.isNotBlank(storePath)) {


remove 'storePath != null', StringUtils.isNotBlank already includes this judgement.

zzcclp · 2018-11-30T08:51:13Z

If possible, it better add a test case for this.

CarbonDataQA · 2018-11-30T09:42:00Z

Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9861/

CarbonDataQA · 2018-11-30T09:44:34Z

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1812/

problem: When create carbonSession in this way: [val carbon = SparkSession.builder().config(sc.getConf).getOrCreateCarbonSession("")],The prompt can be created successfully, with no exceptions thrown during the process, but ultimately [carbon.sql("SELECT * FROM TABLE").show()] is empty. cause: [carbon.sql("SELECT * FROM TABLE").show()] is empty. solution: Increases the null or empty judgment of getOrCreateCarbonSession method parameters[storePath,metaStorePath]

CarbonDataQA · 2018-11-30T10:19:28Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1603/

CarbonDataQA · 2018-11-30T11:17:33Z

Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9863/

CarbonDataQA · 2018-11-30T11:19:47Z

Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1814/

xuchuanyin · 2018-11-30T13:45:24Z

LGTM

zzcclp · 2018-11-30T14:52:02Z

LGTM

…r to an empty string Fixing the getOrCreateCarbonSession method parameter to an empty string causes the select table data to be empty problem: When create carbonSession in this way: [val carbon = SparkSession.builder().config(sc.getConf).getOrCreateCarbonSession("")],The prompt can be created successfully, with no exceptions thrown during the process, but ultimately [carbon.sql("SELECT * FROM TABLE").show()] is empty. cause: [carbon.sql("SELECT * FROM TABLE").show()] is empty. solution: Increases the null or empty judgment of getOrCreateCarbonSession method parameters[storePath,metaStorePath] This closes #2961

[CARBONDATA-2961] Simplify SDK API interfaces problem: current SDK API interfaces are not simpler and don't follow builder pattern. If new features are added, it will become more complex. Solution: Simplify the SDK interfaces as per builder pattern. Refer the latest sdk-guide. Added: changes in Carbon Writer: public CarbonWriterBuilder withThreadSafe(short numOfThreads) public CarbonWriterBuilder withHadoopConf(Configuration conf) public CarbonWriterBuilder withCsvInput(Schema schema) public CarbonWriterBuilder withAvroInput(org.apache.avro.Schema avroSchema) public CarbonWriterBuilder withJsonInput(Schema carbonSchema) public CarbonWriter build() throws IOException, InvalidLoadOptionException Changes in carbon Reader public CarbonReaderBuilder withHadoopConf(Configuration conf) public CarbonWriter build() throws IOException, InvalidLoadOptionException Removed: changes in Carbon Writer: public CarbonWriterBuilder isTransactionalTable(boolean isTransactionalTable) public CarbonWriterBuilder persistSchemaFile(boolean persist) setAccessKey setAccessKey setSecretKey setSecretKey setEndPoint setEndPoint public CarbonWriter buildWriterForCSVInput(Schema schema, Configuration configuration) public CarbonWriter buildThreadSafeWriterForCSVInput(Schema schema, short numOfThreads,Configuration configuration) public CarbonWriter buildWriterForAvroInput(org.apache.avro.Schema avroSchema,Configuration configuration) public CarbonWriter buildThreadSafeWriterForAvroInput(org.apache.avro.Schema avroSchema,short numOfThreads, Configuration configuration) public JsonCarbonWriter buildWriterForJsonInput(Schema carbonSchema, Configuration configuration) public JsonCarbonWriter buildThreadSafeWriterForJsonInput(Schema carbonSchema, short numOfThreads,Configuration configuration) Changes in carbon Reader public CarbonReaderBuilder isTransactionalTable(boolean isTransactionalTable) public CarbonWriter build(Configuration conf) throws IOException, InvalidLoadOptionException This closes apache#2961

…r to an empty string Fixing the getOrCreateCarbonSession method parameter to an empty string causes the select table data to be empty problem: When create carbonSession in this way: [val carbon = SparkSession.builder().config(sc.getConf).getOrCreateCarbonSession("")],The prompt can be created successfully, with no exceptions thrown during the process, but ultimately [carbon.sql("SELECT * FROM TABLE").show()] is empty. cause: [carbon.sql("SELECT * FROM TABLE").show()] is empty. solution: Increases the null or empty judgment of getOrCreateCarbonSession method parameters[storePath,metaStorePath] This closes apache#2961

Fixing the getOrCreateCarbonSession method parameter to an empty stri…

f934d79

…ng causes the select table data to be empty

qiuchenjian reviewed Nov 28, 2018

View reviewed changes

zygitup changed the title ~~Fixing the getOrCreateCarbonSession method parameter to an empty stri…[[JIRA CARBONDATA-3119]]~~ [[JIRA CARBONDATA-3119]]Fixing the getOrCreateCarbonSession method parameter to an empty string Nov 29, 2018

zygitup changed the title ~~[[JIRA CARBONDATA-3119]]Fixing the getOrCreateCarbonSession method parameter to an empty string~~ [JIRA CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string Nov 29, 2018

zygitup changed the title ~~[JIRA CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string~~ [CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string Nov 29, 2018

xubo245 reviewed Nov 30, 2018

View reviewed changes

zygitup added 2 commits November 30, 2018 11:14

Merge branch 'new1'

f7f942e

xubo245 reviewed Nov 30, 2018

View reviewed changes

zzcclp reviewed Nov 30, 2018

View reviewed changes

asfgit closed this in 382ce43 Nov 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string #2961

[CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string #2961

zygitup commented Nov 28, 2018 •

edited

CarbonDataQA commented Nov 28, 2018

qiuchenjian Nov 28, 2018

xubo245 Nov 29, 2018 •

edited

qiuchenjian Nov 29, 2018

xubo245 Nov 29, 2018

zzcclp Nov 30, 2018

chenliang613 commented Nov 28, 2018

chenliang613 commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

xubo245 commented Nov 29, 2018

xubo245 Nov 30, 2018

zzcclp Nov 30, 2018

xubo245 Nov 30, 2018

xuchuanyin commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

xubo245 Nov 30, 2018

zzcclp Nov 30, 2018

zygitup Nov 30, 2018

zzcclp Nov 30, 2018

zzcclp commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

xuchuanyin commented Nov 30, 2018

zzcclp commented Nov 30, 2018

		@@ -37,6 +38,8 @@ import org.apache.carbondata.core.constants.CarbonCommonConstants
		import org.apache.carbondata.core.util.{CarbonProperties, CarbonSessionInfo, ThreadLocalSessionInfo}
		import org.apache.carbondata.streaming.CarbonStreamingQueryListener

[CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string #2961

[CARBONDATA-3119] Fixing the getOrCreateCarbonSession method parameter to an empty string #2961

Conversation

zygitup commented Nov 28, 2018 • edited

CarbonDataQA commented Nov 28, 2018

Choose a reason for hiding this comment

xubo245 Nov 29, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenliang613 commented Nov 28, 2018

chenliang613 commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

CarbonDataQA commented Nov 28, 2018

xubo245 commented Nov 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xuchuanyin commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzcclp commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

CarbonDataQA commented Nov 30, 2018

xuchuanyin commented Nov 30, 2018

zzcclp commented Nov 30, 2018

zygitup commented Nov 28, 2018 •

edited

xubo245 Nov 29, 2018 •

edited