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

Support postgresql kafka connector #23

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

atiqsayyed
Copy link

Ref #4

  • Added support for PostgreSQL database

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2017

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

✅ atiqsayyed
❌ Atiq
❌ Atiq Sayyed


Atiq, Atiq Sayyed seem not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link

codecov bot commented Oct 1, 2017

Codecov Report

Merging #23 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   96.15%   96.19%   +0.04%     
==========================================
  Files           7        7              
  Lines         260      263       +3     
  Branches       29       32       +3     
==========================================
+ Hits          250      253       +3     
  Misses         10       10
Impacted Files Coverage Δ
...connector/jdbc/services/TimeBasedDataService.scala 100% <100%> (ø) ⬆️
...nnector/jdbc/services/TimeIdBasedDataService.scala 92.85% <100%> (+0.17%) ⬆️
...a/connector/jdbc/services/IdBasedDataService.scala 91.42% <100%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 280ae24...1fac870. Read the comment docs.

Copy link
Member

@arpanchaudhury arpanchaudhury left a comment

Choose a reason for hiding this comment

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

apart from the named argument, everything looks good to me.

@@ -12,7 +12,7 @@ import scala.util.{Failure, Success, Try}

class JdbcSourceConnector extends SourceConnector {
private val logger = LoggerFactory.getLogger(classOf[JdbcSourceConnector])

Class.forName("org.postgresql.Driver")
Copy link
Member

Choose a reason for hiding this comment

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

The driver should be loaded automatically, is it required to explicitly do this?

case MySQL => connection.prepareStatement(s"CALL $storedProcedureName (@$incrementingVariableName := ?, @$batchSizeVariableName := ?)")
case MsSQL => connection.prepareStatement(s"EXECUTE $storedProcedureName @$incrementingVariableName = ?, @$batchSizeVariableName = ?")
case MySQL => connection.prepareStatement(s"CALL $storedProcedureName (@$incrementingVariableName := ?, @$batchSizeVariableName := ?)")
case PostgreSQL => connection.prepareStatement(s"SELECT * from $storedProcedureName (?, ?)")
Copy link
Member

Choose a reason for hiding this comment

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

Can we pass arguments by name?

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

Successfully merging this pull request may close these issues.

3 participants