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

Fix logging for streaming and updateMany #2064

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

jatcwang
Copy link
Collaborator

@jatcwang jatcwang commented Jul 17, 2024

Introduces three new functions in the high level API for Connection (doobie.hi.HC).

Fixes #533, #1888

These helpers offer a few improvements:

  • Logging support
  • Handle and log exceptions throw during the PreparedStatement creation (Some JDBC drivers do this if the table doesn't exist)
  • A slighly simpler "flat" API for anyone that wants to customize their query

executeWithResultSet

This is a generalization for any query that executes a PreparedStatement and processes a ResultSet. This covers two cases:

  • A normal select query (JDBC's executeQuery)
  • An update which generates returned rows (i.e. RETURNING / .executeUpdate + .getGeneratedKeys)

All query methods like Query#to and Update#withUniqueGeneratedKeys have switched to use this internally.

executeWithoutResultSet

Similar to executeWithResultSet but for queries that doesn't need to process a ResultSet (i.e. simple updates).

Methods like Update's updateMany, update have switched to use this internally

stream

This will be replacing the existing stream method (which is currently deprecated because it doesn't support logging)

Query#stream and all the Update#withGeneratedKeys* have switched to this internally.

Further improvements can probably be made on top of these methods so users don't need to construct LoggingInfo themselves.

@jatcwang jatcwang merged commit 9e978b4 into main Jul 19, 2024
12 checks passed
@jatcwang jatcwang deleted the fix_logging_for_updateMany_and_streaming branch July 19, 2024 20:05
@henricook
Copy link

henricook commented Feb 10, 2025

Thanks for this improvement @jatcwang . I'm trying to convert some existing code when migrating from RC6 -> RC7 due to the deprecation and wonder if you have a minute to provide some guidance? 🙏🏻

Old code (RC6):

private def exec(sql: String): ConnectionIO[RawResultSet] = {

  HC.prepareStatement(sql)(HPS.executeQuery {
        for {
          md <- FRS.getMetaData
          rs <- row(md).whileM[List](HRS.next)
        } yield RawResultSet(header(md), rs)
      })

It's unclear to me how to convert this to the FC.prepareStatement / executeWithResultSet style, i've got something like this so far (RC7):

  private def exec(sql: String): ConnectionIO[RawResultSet] = {

    val preparedStatement = FC.prepareStatement(sql)

    doobie.hi.connection.executeWithResultSet(
      create = preparedStatement,
      prep = ???,
      exec = ???,
      process = ???,
      LoggingInfo(sql, NonBatch(List.empty), "fooLabel")
    )

It's unclear to me how to/what to put in prep, exec and process - might you have any pointers? Or a method I'm missing which just takes care of it for me?

My use case here is running (admin) user provided SQL without any bound parameters - just a straight string -> query mapping.

@jatcwang
Copy link
Collaborator Author

Hi @henricook sorry the documentation for custom JDBC operations isn't really there and I haven't gotten around to #2134 yet.

In your case,

  • prep: You don't need a prepare step, so you can use FPS.unit
  • exec: FPS.executeQuery
  • process:
for {
  md <- FRS.getMetaData
  rs <- row(md).whileM[List](HRS.next)
} yield RawResultSet(header(md), rs)

Hope that works!

@henricook
Copy link

henricook commented Feb 10, 2025

Thanks for a super speedy response @jatcwang. I was reading the MR but Doobie novice that I am it took me a while to realise that IFPS FPS etc are all remappings of things like doobie.free.preparedstatement.

Here's my compiling and working final item based on your comments, currently without the 'doobie idiomatic' remapping of doobie.free.preparedstatement to FPS and doobie.hi.connection to IHC - i'll do that next

Notes:

  • I have no params to bind, so this isn't suitable if you're reading and hoping to bind parameters.
  • mkLabel is a custom method I use just to prefix a repository name onto the label
private def exec(sql: String): ConnectionIO[RawResultSet] = {

    val preparedStatement = FC.prepareStatement(sql)

    doobie.hi.connection.executeWithResultSet(
      create = preparedStatement,
      prep = doobie.free.preparedstatement.unit,
      exec = doobie.free.preparedstatement.executeQuery,
      process = (for {
        md <- FRS.getMetaData
        rs <- row(md).whileM[List](HRS.next)
      } yield RawResultSet(header(md), rs)),
      LoggingInfo(sql, NonBatch(List.empty), mkLabel("executeQuery"))
    )

@jatcwang
Copy link
Collaborator Author

Thanks for posting an update @henricook glad it worked 🥳

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

Successfully merging this pull request may close these issues.

withGeneratedKeys & transaction statements missing from logs
2 participants