LiShuMing opened a new pull request #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631 ### Why is this PR needed? I found a lot of `listTables` of the db to check if table exists like this: ``` sparkSession.sessionState.catalog.listTables(databaseName) .exists(_.table.equalsIgnoreCase(tableName) ``` This may affect performances if there are lots of tables in one database. In `spark`, we can check this by `tableExists` function. ### What changes were proposed in this PR? use `exits(db, table)` to replace `iterate tables in a db`. ### Does this PR introduce any user interface change? - No ### Is any new testcase added? - No ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-589625045 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/391/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-589642319 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2092/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-589667457 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/392/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-589702089 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2093/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
kunal642 commented on a change in pull request #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#discussion_r383255402 ########## File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonCreateTableAsSelectCommand.scala ########## @@ -54,8 +54,8 @@ case class CarbonCreateTableAsSelectCommand( setAuditTable(dbName, tableName) setAuditInfo(Map("query" -> query.simpleString)) // check if table already exists - if (sparkSession.sessionState.catalog.listTables(dbName) - .exists(_.table.equalsIgnoreCase(tableName))) { + if (!sparkSession.sessionState.catalog + .tableExists(TableIdentifier(tableName, Some(dbName)))) { Review comment: We need to check if the table exists, currently because of the ! check the condition is reversed. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
ajantha-bhat commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593747546 @LiShuMing : please rebase and handle the comment ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
ajantha-bhat commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593747600 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593752488 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/567/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593770634 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2271/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
LiShuMing commented on a change in pull request #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#discussion_r386956578 ########## File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonCreateTableAsSelectCommand.scala ########## @@ -54,8 +54,8 @@ case class CarbonCreateTableAsSelectCommand( setAuditTable(dbName, tableName) setAuditInfo(Map("query" -> query.simpleString)) // check if table already exists - if (sparkSession.sessionState.catalog.listTables(dbName) - .exists(_.table.equalsIgnoreCase(tableName))) { + if (!sparkSession.sessionState.catalog + .tableExists(TableIdentifier(tableName, Some(dbName)))) { Review comment: I'm sorry. It's my mistake. What I mean is It's very bad to use `listTables` to check table's existence instead of `tableExists`. I've rebased and fixed this. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
LiShuMing commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593903051 > @LiShuMing : please rebase and handle the comment I've rebased and fixed this. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
ajantha-bhat commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593904298 @LiShuMing : Ok, once the build is passed it can be merged ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593911959 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/582/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-593937254 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2287/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on issue #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631#issuecomment-594371040 LGTM Thanks for working on this ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
asfgit closed pull request #3631: [CARBONDATA-3696] Avoid list db's all Tables to check table exists in the db
URL: https://github.com/apache/carbondata/pull/3631 ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
Free forum by Nabble | Edit this page |