[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3843: [WIP]Fix NullPointerException in case of multiple updates and clean files
CarbonDataQA1 commented on pull request #3843: URL: https://github.com/apache/carbondata/pull/3843#issuecomment-658321798 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1649/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3843: [WIP]Fix NullPointerException in case of multiple updates and clean files
CarbonDataQA1 commented on pull request #3843: URL: https://github.com/apache/carbondata/pull/3843#issuecomment-658321222 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3390/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] akashrn5 opened a new pull request #3843: [WIP]Fix NullPointerException in case of multiple updates and clean files
akashrn5 opened a new pull request #3843: URL: https://github.com/apache/carbondata/pull/3843 ### Why is this PR needed? ### What changes were proposed in this PR? ### Does this PR introduce any user interface change? - No - Yes. (please explain the change and update document) ### Is any new testcase added? - No - Yes This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3789: [CARBONDATA-3864] Store Size Optimization
CarbonDataQA1 commented on pull request #3789: URL: https://github.com/apache/carbondata/pull/3789#issuecomment-658245717 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1648/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3789: [CARBONDATA-3864] Store Size Optimization
CarbonDataQA1 commented on pull request #3789: URL: https://github.com/apache/carbondata/pull/3789#issuecomment-658244665 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3389/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (CARBONDATA-3898) Support Option 'carbon.enable.querywithmv'
[ https://issues.apache.org/jira/browse/CARBONDATA-3898?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kunal Kapoor resolved CARBONDATA-3898. -- Fix Version/s: 2.1.0 Resolution: Fixed > Support Option 'carbon.enable.querywithmv' > -- > > Key: CARBONDATA-3898 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3898 > Project: CarbonData > Issue Type: New Feature >Reporter: Xingjun Hao >Priority: Minor > Fix For: 2.1.0 > > Time Spent: 4h 40m > Remaining Estimate: 0h > > When MV enabled, SQL rewrite takes a lot of time, a new option > 'carbon.enable.querywithmv' shall be supported, which can turn off SQL > Rewrite when the configured value is false -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] asfgit closed pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
asfgit closed pull request #3839: URL: https://github.com/apache/carbondata/pull/3839 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] kunal642 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
kunal642 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658182073 LGTM This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658156072 LGTM This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Updated] (CARBONDATA-3831) Support write carbon files with presto.
[ https://issues.apache.org/jira/browse/CARBONDATA-3831?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Akash R Nilugal updated CARBONDATA-3831: Attachment: carbon_presto_write_transactional SUpport.pdf > Support write carbon files with presto. > --- > > Key: CARBONDATA-3831 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3831 > Project: CarbonData > Issue Type: New Feature >Reporter: Akash R Nilugal >Assignee: Akash R Nilugal >Priority: Major > Attachments: carbon_presto_write_transactional SUpport.pdf > > > As we know the CarbonDataisan indexed columnar data format for fast analytics > on big data platforms. So we have already integrated with the query engines > like spark and even presto. Currently with presto we only support the > querying of carbondata files. But we don’t yet support the writing of > carbondata files > through the presto engine. > Currentlypresto is integrated with carbondata for reading the > carbondata files via presto. For this, we should be having the store already > ready which may be written carbon in spark and the table > should be hive metastore. So using carbondata connector we are able to read > the carbondata files. But we cannot create table or load the data to table in > presto. So it will somewhat hectic job to read the carbonfiles , by writing > first with other engine. > So here i will be trying to support the transactional load support in presto > integration for carbon. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (CARBONDATA-3899) drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients.
[ https://issues.apache.org/jira/browse/CARBONDATA-3899?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] SHREELEKHYA GAMPA updated CARBONDATA-3899: -- Description: drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients from beeline. !screenshot-1.png! was:drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients. > drop materialized view when executed concurrently from 4 concurrent client > fails in all 4 clients. > -- > > Key: CARBONDATA-3899 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3899 > Project: CarbonData > Issue Type: Bug >Reporter: SHREELEKHYA GAMPA >Priority: Major > Attachments: screenshot-1.png > > > drop materialized view when executed concurrently from 4 concurrent client > fails in all 4 clients from beeline. > !screenshot-1.png! -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (CARBONDATA-3899) drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients.
[ https://issues.apache.org/jira/browse/CARBONDATA-3899?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] SHREELEKHYA GAMPA updated CARBONDATA-3899: -- Attachment: screenshot-1.png > drop materialized view when executed concurrently from 4 concurrent client > fails in all 4 clients. > -- > > Key: CARBONDATA-3899 > URL: https://issues.apache.org/jira/browse/CARBONDATA-3899 > Project: CarbonData > Issue Type: Bug >Reporter: SHREELEKHYA GAMPA >Priority: Major > Attachments: screenshot-1.png > > > drop materialized view when executed concurrently from 4 concurrent client > fails in all 4 clients. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (CARBONDATA-3899) drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients.
SHREELEKHYA GAMPA created CARBONDATA-3899: - Summary: drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients. Key: CARBONDATA-3899 URL: https://issues.apache.org/jira/browse/CARBONDATA-3899 Project: CarbonData Issue Type: Bug Reporter: SHREELEKHYA GAMPA drop materialized view when executed concurrently from 4 concurrent client fails in all 4 clients. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
CarbonDataQA1 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658137342 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1647/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
CarbonDataQA1 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658135809 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3388/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #3841: [WIP] : drop materialized view when executed concurrently from 4 concurrent c…
Indhumathi27 commented on pull request #3841: URL: https://github.com/apache/carbondata/pull/3841#issuecomment-658118242 @ShreelekhyaG Please create a jira and update the PR This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
CarbonDataQA1 commented on pull request #3842: URL: https://github.com/apache/carbondata/pull/3842#issuecomment-658101946 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1646/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
CarbonDataQA1 commented on pull request #3842: URL: https://github.com/apache/carbondata/pull/3842#issuecomment-658101120 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3387/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] QiangCai commented on a change in pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
QiangCai commented on a change in pull request #3842:
URL: https://github.com/apache/carbondata/pull/3842#discussion_r454233869
##
File path:
integration/spark/src/main/scala/org/apache/spark/rdd/CarbonMergeFilesRDD.scala
##
@@ -157,21 +157,21 @@ object CarbonMergeFilesRDD {
if (carbonTable.isHivePartitionTable &&
!StringUtils.isEmpty(tempFolderPath)) {
// remove all tmp folder of index files
val startDelete = System.currentTimeMillis()
- val numThreads = Math.min(Math.max(partitionInfo.size(), 1), 10)
- val executorService = Executors.newFixedThreadPool(numThreads)
- val carbonSessionInfo = ThreadLocalSessionInfo.getCarbonSessionInfo
- partitionInfo
-.asScala
-.map { partitionPath =>
- executorService.submit(new Runnable {
-override def run(): Unit = {
- ThreadLocalSessionInfo.setCarbonSessionInfo(carbonSessionInfo)
- FileFactory.deleteAllCarbonFilesOfDir(
-FileFactory.getCarbonFile(partitionPath + "/" +
tempFolderPath))
-}
- })
+ val allTmpDirs = partitionInfo
+.asScala.map { partitionPath =>
+ partitionPath + CarbonCommonConstants.FILE_SEPARATOR + tempFolderPath
}
-.map(_.get())
+ val allTmpFiles = allTmpDirs.map { partitionDir =>
+ FileFactory.getCarbonFile(partitionDir).listFiles()
+}.flatten.map(_.getAbsolutePath)
+ // delete tmp files in parallel
+ sparkSession.sparkContext.parallelize(allTmpFiles).map {
+FileFactory.getCarbonFile(_).delete()
+ }.collect()
+ // delete tmp dir in parallel
+ sparkSession.sparkContext.parallelize(allTmpDirs).map {
+FileFactory.getCarbonFile(_).delete()
+ }.collect()
Review comment:
You can just use job to replace multi-threads, one task delete one dir
with files.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [carbondata] marchpure commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
marchpure commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658078473 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
CarbonDataQA1 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658075553 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1645/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
CarbonDataQA1 commented on pull request #3839: URL: https://github.com/apache/carbondata/pull/3839#issuecomment-658073031 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3386/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3841: [WIP] : drop materialized view when executed concurrently from 4 concurrent c…
CarbonDataQA1 commented on pull request #3841: URL: https://github.com/apache/carbondata/pull/3841#issuecomment-658045347 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3385/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3841: [WIP] : drop materialized view when executed concurrently from 4 concurrent c…
CarbonDataQA1 commented on pull request #3841: URL: https://github.com/apache/carbondata/pull/3841#issuecomment-658045205 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1644/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] marchpure commented on pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
marchpure commented on pull request #3842: URL: https://github.com/apache/carbondata/pull/3842#issuecomment-658036209 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
CarbonDataQA1 commented on pull request #3842: URL: https://github.com/apache/carbondata/pull/3842#issuecomment-658033328 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1643/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3842: [CARBONDATA-3702] Clean temp index files in parallel in merge index flow
CarbonDataQA1 commented on pull request #3842: URL: https://github.com/apache/carbondata/pull/3842#issuecomment-658033201 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3384/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on a change in pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on a change in pull request #3839:
URL: https://github.com/apache/carbondata/pull/3839#discussion_r454137683
##
File path:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##
@@ -2433,12 +2441,6 @@ private CarbonCommonConstants() {
*/
public static final String INDEX_STATUS = "index_status";
- /**
- * Materialized view thread context properties
- */
- @CarbonProperty
- public static final String DISABLE_SQL_REWRITE = "disable_sql_rewrite";
Review comment:
Are there any user already use this configuration?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on a change in pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on a change in pull request #3839:
URL: https://github.com/apache/carbondata/pull/3839#discussion_r454137628
##
File path:
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##
@@ -2433,12 +2441,6 @@ private CarbonCommonConstants() {
*/
public static final String INDEX_STATUS = "index_status";
- /**
Review comment:
Are there any user already use this configuration?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on a change in pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on a change in pull request #3839:
URL: https://github.com/apache/carbondata/pull/3839#discussion_r454134453
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -501,6 +502,25 @@ private void validateCarbonTaskDistribution() {
}
}
+ private void validateEnableMV() {
+String isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+if (isMVEnabled == null) {
+ carbonProperties.setProperty(CarbonCommonConstants.CARBON_ENABLE_MV,
+ CarbonCommonConstants.CARBON_ENABLE_MV_DEFAULT);
+ isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+}
+boolean isValidBooleanValue = CarbonUtil.validateBoolean(isMVEnabled);
+if (!isValidBooleanValue) {
Review comment:
CarbonUtil.validateBoolean include judge null:
```
public static boolean validateBoolean(String value) {
if (null == value) {
return false;
} else if (!("false".equalsIgnoreCase(value) ||
"true".equalsIgnoreCase(value))) {
return false;
}
return true;
}
```
so line 507-511 is unnecessary
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on a change in pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on a change in pull request #3839:
URL: https://github.com/apache/carbondata/pull/3839#discussion_r454134453
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -501,6 +502,25 @@ private void validateCarbonTaskDistribution() {
}
}
+ private void validateEnableMV() {
+String isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+if (isMVEnabled == null) {
+ carbonProperties.setProperty(CarbonCommonConstants.CARBON_ENABLE_MV,
+ CarbonCommonConstants.CARBON_ENABLE_MV_DEFAULT);
+ isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+}
+boolean isValidBooleanValue = CarbonUtil.validateBoolean(isMVEnabled);
+if (!isValidBooleanValue) {
Review comment:
CarbonUtil.validateBoolean include judge null:
```
public static boolean validateBoolean(String value) {
if (null == value) {
return false;
} else if (!("false".equalsIgnoreCase(value) ||
"true".equalsIgnoreCase(value))) {
return false;
}
return true;
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [carbondata] xubo245 commented on a change in pull request #3839: [CARBONDATA-3898] Support Option 'carbon.enable.mv'
xubo245 commented on a change in pull request #3839:
URL: https://github.com/apache/carbondata/pull/3839#discussion_r454134062
##
File path:
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##
@@ -501,6 +502,25 @@ private void validateCarbonTaskDistribution() {
}
}
+ private void validateEnableMV() {
+String isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+if (isMVEnabled == null) {
+ carbonProperties.setProperty(CarbonCommonConstants.CARBON_ENABLE_MV,
+ CarbonCommonConstants.CARBON_ENABLE_MV_DEFAULT);
+ isMVEnabled =
carbonProperties.getProperty(CarbonCommonConstants.CARBON_ENABLE_MV);
+}
+boolean isValidBooleanValue = CarbonUtil.validateBoolean(isMVEnabled);
+if (!isValidBooleanValue) {
Review comment:
suggestion: merge it with line 507, null also is invalid value.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
