-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix: add traces to updates with generared keys #112
fix: add traces to updates with generared keys #112
Conversation
@@ -14,7 +14,8 @@ import scala.annotation.nowarn | |||
@nowarn | |||
private[doobie] case class TracedStatement( | |||
p: PreparedStatement, | |||
queryString: String | |||
queryString: String, | |||
c: Any* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does c
stand for? Could you please use a more descriptive name?
I see that we didn't set a good example with p
for preparedStatement
on line 16, but we should do better in the future.
override def prepareStatement( a: String, b: Array[String]): Kleisli[F, Connection, PreparedStatement] = | ||
super.prepareStatement(a, b).map(TracedStatement(_, a, b): PreparedStatement) | ||
override def prepareStatement(a: String, b: Array[Int]): Kleisli[F, Connection, PreparedStatement] = | ||
super.prepareStatement(a, b).map(TracedStatement(_, a, b): PreparedStatement) | ||
override def prepareStatement(a: String, b: Int) = | ||
super.prepareStatement(a, b).map(TracedStatement(_, a, b): PreparedStatement) | ||
override def prepareStatement(a: String, b: Int, c: Int) = | ||
super.prepareStatement(a, b, c).map(TracedStatement(_, a, b, c): PreparedStatement) | ||
override def prepareStatement(a: String, b: Int, c: Int, d: Int) = | ||
super.prepareStatement(a, b, c, d).map(TracedStatement(_, a, b, c, d): PreparedStatement) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the variadic parameter is not used anywhere, why is it needed to be set?
@ajkaanbal Could you please describe the issue / symptom you were experiencing? |
Hi @voidcontext, I believe we ran into the same issue; queries using |
HI @ajkaanbal and @richard-dennehy-ovo, I had some time today so I added the suggested changes in #114 and merged it, I'll tag a fix release soon, it should be available in maven later today. |
I was having trouble tracing
updates
usingwithUniqueGeneratedKeys
, these changes appear to have solved the problem so far.