Skip to content

[SPARK-14460] [SQL] properly handling of column name contains space #12252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
bomeng wants to merge 11 commits into from
Closed

[SPARK-14460] [SQL] properly handling of column name contains space #12252

bomeng wants to merge 11 commits into from

Conversation

@bomeng
Copy link
Contributor

@bomeng bomeng commented Apr 8, 2016 •
edited
Loading

What changes were proposed in this pull request?

Although it is not recommended, table can be created with column name containing space. For example,

create table test.people1 (name TEXT(32) NOT NULL,the id INTEGER NOT NULL)

When we query the table via JDBC, we also need to quote the column name in the same way if the column name contains space.

I am leveraging the quote method in the dialect, it will add the proper quote to the column name based on its data source.

How was this patch tested?

I've updated the test case to test this scenario.

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55312 has finished for PR 12252 at commit 51b7f86.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55309 has finished for PR 12252 at commit 88935c5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55330 has finished for PR 12252 at commit d0c86ea.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55328 has finished for PR 12252 at commit 9ffb462.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@bomeng
Copy link
Contributor Author

bomeng commented Apr 11, 2016

@andrewor14 Could you please review my codes?

@SparkQA
Copy link

SparkQA commented Apr 14, 2016

Test build #55851 has finished for PR 12252 at commit ecd52bf.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 14, 2016

Test build #55854 has finished for PR 12252 at commit b70f21f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

ash211
ash211 reviewed Apr 15, 2016
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
*/
def schemaString(df: DataFrame, url: String): String = {
def schemaString(dialect: JdbcDialect, df: DataFrame, url: String): String = {
val sb = new StringBuilder()
Copy link
Contributor

@ash211 ash211 Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the new dialect you're passing in different from this one in some way?

Copy link
Contributor Author

@bomeng bomeng Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose to pass in dialect is to get proper quote for columns based on its data source. Any suggestion?

Copy link
Contributor

@ash211 ash211 Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You pass in a parameter named dialect to the schemaString method, but there's also the dialect that comes from JdbcDialects.get(url) --- that's the duplicate I was trying to point out

Copy link
Contributor Author

@bomeng bomeng Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out. I've modified the codes. Please check it out.

@SparkQA
Copy link

SparkQA commented Apr 15, 2016

Test build #55889 has finished for PR 12252 at commit 715777a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 15, 2016

Test build #55891 has finished for PR 12252 at commit 675700f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 20, 2016

Test build #56384 has finished for PR 12252 at commit b151065.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

cloud-fan commented May 25, 2017

seems it's fixed in #15662 ?

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Jun 2, 2017

ping @bomeng for ^.

@HyukjinKwon HyukjinKwon mentioned this pull request Jun 7, 2017
Closed
@asfgit asfgit closed this in b771fed Jun 8, 2017
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
@HyukjinKwon @zifeif2
[INFRA] Close stale PRs
ce94f63 
# What changes were proposed in this pull request?

This PR proposes to close stale PRs, mostly the same instances with apache#18017

Closes apache#11459
Closes apache#13833
Closes apache#13720
Closes apache#12506
Closes apache#12456
Closes apache#12252
Closes apache#17689
Closes apache#17791
Closes apache#18163
Closes apache#17640
Closes apache#17926
Closes apache#18163
Closes apache#12506
Closes apache#18044
Closes apache#14036
Closes apache#15831
Closes apache#14461
Closes apache#17638
Closes apache#18222

Added:
Closes apache#18045
Closes apache#18061
Closes apache#18010
Closes apache#18041
Closes apache#18124
Closes apache#18130
Closes apache#12217

Added:
Closes apache#16291
Closes apache#17480
Closes apache#14995

Added:
Closes apache#12835
Closes apache#17141

## How was this patch tested?

N/A

Author: hyukjinkwon <gurwls223@gmail.com>

Closes apache#18223 from HyukjinKwon/close-stale-prs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bomeng @SparkQA @cloud-fan @HyukjinKwon @ash211