You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@carbondata.apache.org by GitBox <gi...@apache.org> on 2021/09/14 08:18:43 UTC
[GitHub] [carbondata] ShreelekhyaG opened a new pull request #4216: [CARBONDATA-4277] geo instance compatability fix
ShreelekhyaG opened a new pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216
### Why is this PR needed?
The `CustomIndex `interface extends Serializable and for different version store, if the serialization id doesn't match, it throws `java.io.InvalidClassException` during load/update/query operations.
### What changes were proposed in this PR?
As the instance is stored in table properties, made changes to initialize and update instance while refresh table. Also added static serialId for the CustomIndex interface.
### Does this PR introduce any user interface change?
- No
### Is any new testcase added?
- No, tested in cluster
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] ShreelekhyaG commented on a change in pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
ShreelekhyaG commented on a change in pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#discussion_r709182359
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
Review comment:
Verified for direct upgrade scenario and update document.
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
+ val indexName = tableProperties.get(CarbonCommonConstants.SPATIAL_INDEX)
+ if (indexName != null) {
+ val SPATIAL_INDEX_CLASS = s"${ CarbonCommonConstants.SPATIAL_INDEX }.$indexName.class"
+ val SPATIAL_INDEX_INSTANCE = s"${
+ CarbonCommonConstants.SPATIAL_INDEX
+ }.$indexName.instance"
+ // For spatial table, To make the instance compatible with previous versions,
+ // initialise and update the index instance in table properties.
+ tableProperties.remove(SPATIAL_INDEX_INSTANCE)
+ val spatialIndexClass: Class[_] = java.lang.Class.forName(tableProperties
Review comment:
Done
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
+ val indexName = tableProperties.get(CarbonCommonConstants.SPATIAL_INDEX)
+ if (indexName != null) {
+ val SPATIAL_INDEX_CLASS = s"${ CarbonCommonConstants.SPATIAL_INDEX }.$indexName.class"
Review comment:
Done
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-920141715
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-920124068
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5915/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#discussion_r708157522
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
+ val indexName = tableProperties.get(CarbonCommonConstants.SPATIAL_INDEX)
+ if (indexName != null) {
+ val SPATIAL_INDEX_CLASS = s"${ CarbonCommonConstants.SPATIAL_INDEX }.$indexName.class"
+ val SPATIAL_INDEX_INSTANCE = s"${
+ CarbonCommonConstants.SPATIAL_INDEX
+ }.$indexName.instance"
+ // For spatial table, To make the instance compatible with previous versions,
+ // initialise and update the index instance in table properties.
+ tableProperties.remove(SPATIAL_INDEX_INSTANCE)
+ val spatialIndexClass: Class[_] = java.lang.Class.forName(tableProperties
Review comment:
Same code is present in CarbonParserUtil.processSpatialIndexProperty. Please move to common place and reuse
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#discussion_r708160667
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
+ val indexName = tableProperties.get(CarbonCommonConstants.SPATIAL_INDEX)
+ if (indexName != null) {
+ val SPATIAL_INDEX_CLASS = s"${ CarbonCommonConstants.SPATIAL_INDEX }.$indexName.class"
Review comment:
can create new variable for CarbonCommonConstants.SPATIAL_INDEX and format the code
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-919021491
Build Success with Spark 3.1, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_3.1/315/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-920125471
Build Success with Spark 3.1, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_3.1/317/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-919029378
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/4169/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#discussion_r708156034
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/RefreshCarbonTableCommand.scala
##########
@@ -83,6 +84,23 @@ case class RefreshCarbonTableCommand(
if (FileFactory.isFileExist(schemaFilePath)) {
// read TableInfo
val tableInfo = SchemaReader.getTableInfo(identifier)
+ val tableProperties = tableInfo.getFactTable.getTableProperties
Review comment:
Please verify direct upgrade scenarios in this case and handle if it doesnt work. Cam make changes in Refresh table to support this only for spatial table and update the document as well
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-919025667
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5913/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216#issuecomment-920105106
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/4171/
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4216: [CARBONDATA-4277] geo instance compatability fix
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4216:
URL: https://github.com/apache/carbondata/pull/4216
--
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.
To unsubscribe, e-mail: dev-unsubscribe@carbondata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org