Skip to content
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.

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

[SPARK-10946][SQL]JDBC - Use Statement.executeUpdate instead of PreparedStatement.executeUpdate for DDLs #9733

Closed
wants to merge 2 commits into from

Conversation

somideshmukh
Copy link
Contributor

New changes with JDBCRDD

@srowen
Copy link
Member

srowen commented Nov 16, 2015

I agree with this, and am OK to merge this.
There is still an issue with not closing the Statements, although it's a separate issue.

@SparkQA
Copy link

SparkQA commented Nov 16, 2015

Test build #2063 has finished for PR 9733 at commit 9db069d.

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

@@ -120,7 +120,7 @@ private[sql] object JDBCRDD extends Logging {
val dialect = JdbcDialects.get(url)
val conn: Connection = getConnector(properties.getProperty("driver"), url, properties)()
try {
val rs = conn.prepareStatement(s"SELECT * FROM $table WHERE 1=0").executeQuery()
val rs = conn.createStatement.executeQuery(s"SELECT * FROM $table WHERE 1=0")
Copy link
Member

Choose a reason for hiding this comment

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

Ah hang on a sec, I see you've changed some non-DDL statements in this version of the PR. The change is just about avoiding PreparedStatement for DDL right? My earlier comments were about closing resources for a bunch of queries including non-DDL. But if you're only addressing DDL, then your last PR was fine (before it was corrupted).

@somideshmukh
Copy link
Contributor Author

OK,I will remove non-DDL statements and create new pull request.

@srowen
Copy link
Member

srowen commented Nov 17, 2015

You don't need a new PR, just push a new commit here. It's better, as it preserves the history of the discussion.

@somideshmukh
Copy link
Contributor Author

I have push new commit to SomilBranch-1.1

@asfgit asfgit closed this in b8f4379 Nov 18, 2015
asfgit pushed a commit that referenced this pull request Nov 18, 2015
…aredStatement.executeUpdate for DDLs

New changes with JDBCRDD

Author: somideshmukh <somilde@us.ibm.com>

Closes #9733 from somideshmukh/SomilBranch-1.1.

(cherry picked from commit b8f4379)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@srowen
Copy link
Member

srowen commented Nov 18, 2015

Merged to master/1.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants