Log SQL query results if supported by DB (rows affected, last insert ID)

[obaltian]

Hi! Recently we have started using goose to run all SQL migrations in a company I work for, so first of all thank you for this great project.

Our developers came to us with a feature request - is it possible to log results of UPDATE/DELETE SQL queries? We run migrations as a part of our CI process, so there are 50 devs looking at CI logs wanting to understand how data has changed immediately. There are cases when devs do not have any access to production databases, so they can't get this info anywhere except logs.

I have prepared a basic implementation. This was my first experience with go so please pay attention for any rookie mistakes I could make.

(BTW #1, am I right that there are SQL-execution code duplication in migration_sql.go and provider_run.go? And that migration.go could in theory be rewritten to use Provider object? Probably it's already planned, just trying to understand the project.)

(BTW #2 I have noticed a typo here, may be reverted if you want to integrate it separately.)

Thanks in advance.

[obaltian]

Hello @mfridman, do you have an opportunity to take a look at this feature? We are starting to plan to build our own binaries with this included – I think it's a good thing to have in goose itself.

(BTW let me know if I should edit commit messages if you don't squash commits before merge – they are very short, my bad)

Thank you!

[mfridman]

To answer some of your questions,

And that migration.go could in theory be rewritten to use Provider object? Probably it's already planned, just trying to understand the project.)

Yes, hehe. This is absolutely something I want to get towards. Most likely in a /v4 version out of precaution to avoid any behavioral changes. (I tried to keep it the same, but you never know what users depend on). And we take breaking changes outside major versions extremly seriously.

Fixing a type is fine

---

Having said all that, I'm not sure this is the direction we should go in. I.e., I don't want to log too much from the provider code. Instead, I think we should allow callers to hook into various parts of the provider and log as needed.

Unrelated to this PR, but in a similar way I'd like to allow users to tap into the errors as well and handle them similar to item 3 in this #962 (comment)

[obaltian]

Ok, I understand, but that moment I tried to implement it at least for the code used for CLI calls. And as I understood provider is not used for CLI so there are no ways for this to be implemented other way at the moment. This PR just looked to me as an easy and valuable improvement for CLI users just like us.

PS Sorry, I could misunderstood you, but are you still reviewing? Or I should continue working on something? Thanks

[mfridman]

PR looks fine as a whole, could be merged in.

Trying to decide if we want this behavior in general and whether to modify any of the log output formatting.

[obaltian]

@mfridman I'm hoping everything's fixed now, didn't know which linters are used before creating the PR.