You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cammachusa <gi...@git.apache.org> on 2017/06/26 18:24:04 UTC
[GitHub] spark pull request #18423: SPARK-21158: Fix case sensitive of temp table/vie...
GitHub user cammachusa opened a pull request:
https://github.com/apache/spark/pull/18423
SPARK-21158: Fix case sensitive of temp table/view name
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/InspurUSA/spark SPARK-21158
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18423.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 #18423
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18423
is this a real issue? Ideally you should call `Catalog.tableExists` to check table/view existence. I think it's reasonable for `Catalog.listTables` to return lower-cased table names if case sensitive is off.
BTW, I also don't think this minor "improvement" worth such a big change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa commented on the issue:
https://github.com/apache/spark/pull/18423
@cloud-fan ,
1. catalog.tableExists is not always visible to callers (pls refer to the ticket's desc)
2. catalog.listTables: it depends on idea. Reporter, has a good reasons, and use case (pls refer to his ticket), and so I'm on her page.
3. Yes, it's a minor "improvement", but I don't see those are big change.
I'd like to have second reviewer, see what he think.
@gatorsmile , @andrewor14 , @rxin can you help to review this patch as well?
Thanks,
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa commented on the issue:
https://github.com/apache/spark/pull/18423
Sorry, It closed by mistake. Please help me to review this patch
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18423
I can see the issue, but adding all these APIs might not be worthy.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18423: SPARK-21158: Fix case sensitive of temp table/vie...
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa closed the pull request at:
https://github.com/apache/spark/pull/18423
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18423
can you give a test case that fails without this patch?
BTW the `GlobalViewManager` documents explicitly:
```
Note that, the view name is always case-sensitive here, callers are responsible to format the
view name w.r.t. case-sensitive config.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa commented on the issue:
https://github.com/apache/spark/pull/18423
OK to test
@cloud-fan , @gatorsmile , @andrewor14 can you help to review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa commented on the issue:
https://github.com/apache/spark/pull/18423
Hi @cloud-fan , thanks for you quick response.
The failure test cases is very straight forward, and can be found in the ticket's description: https://issues.apache.org/jira/browse/SPARK-21158.
To summary:
1. Set your spark sql to case insensitive: spark.sql("SET spark.sql.caseSensitive=false");
2. Create and temp table/view e.g. CameraAndModels
3. Then, check if your CameraAndModes exists in spark.catalog.listTables(). It will return false, while you know for sure that your table does exist.
=> The reason is the formatTable() in SessionCatalog.scala convert your table name to lowercase (btw, by default case sensitive is set to false)
To your second point:
The GlobalViewManager behaves just like the "LocalView", it also call the formatTable() before creating a view. Below is the code in SessionCatalog.scala for your references:
def createGlobalTempView(
name: String,
viewDefinition: LogicalPlan,
overrideIfExists: Boolean): Unit = {
globalTempViewManager.create(formatTableName(name), viewDefinition, overrideIfExists)
}
I fixed this issue by simply return the table name itself from the formatTable(), without convert to lowercase. By doing this way, I don't have to clean up the callers which is erroneous (more than 20 references to this function). And provide a convenience way to check if a table exists, and do care about case sensitive at this point.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by cammachusa <gi...@git.apache.org>.
Github user cammachusa commented on the issue:
https://github.com/apache/spark/pull/18423
It's such a pain when you register your table's name as "MyTempTable", and later in your code you want to check if your table still exists by the same string "MyTempTable", but spark says "No". Now, you start figuring out what's going wrong with your code, and may spend a ton of time for it. And at the end, you realize it's Spark's bug, and that you need to convert it to lower case.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18423: SPARK-21158: Fix case sensitive of temp table/vie...
Posted by cammachusa <gi...@git.apache.org>.
GitHub user cammachusa reopened a pull request:
https://github.com/apache/spark/pull/18423
SPARK-21158: Fix case sensitive of temp table/view name
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/InspurUSA/spark SPARK-21158
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18423.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 #18423
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18423: SPARK-21158: Fix case sensitive of temp table/view name
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18423
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org