Customizable key-value separator and quoting to be machine-readable
This pull request surfaces a new optional configuration to format the Marginalia comments in a machine-readable manner. See the changes to the README in this PR for a summary of the new feature.
Note that this PR is backwards-compatible with existing code. The new setting is optional, and if unset, the default values will retain all existing behavior. Unit tests have also be added to ensure there are no regressions.
Further notes:
- The sqlcommenter project references this fork of Marginalia
- Cloning and installing this fork of Marginalia is one of the steps in the sqlcommenter Rails docs. Merging this pull request will simplify the instructions by removing that extra step.
- The formatting introduced in this pull request enables the Marginalia comments to be parsed more easily. For example, Google CloudSQL uses this formatting to add tags and metadata to queries: https://cloud.google.com/sql/docs/postgres/using-query-insights#adding-tags-to-sql-queries
- Here is a previous pull request that attempted to add this same feature to Marginalia: https://github.com/basecamp/marginalia/pull/89/commits
- That pull request had favorable reviews, but seemed to fizzle out due to merge conflicts.
- The only difference is that it resolves some merge conflicts and adds some testing.
- My company has been running this fork of Marginalia in production since March 3rd, and we haven't had any issues.
- I'm more than happy to give anyone a walkthrough of the changes, or buy a coffee, or make a donation for your time :)
TODO:
- [x] verify that the unit tests are working, following these steps: https://github.com/basecamp/marginalia#contributing
- [x] conduct an end-to-end test to verify this feature works correctly with the sqlcommenter_rails gem
- [x] update this pull request's description to describe this feature
Note that some changes were made to the API of this feature over the course of this PR. Here is the original description, for posterity:
This pull request provides 2 new optional configs which are backwards compatible with all existing code by default. The 2 optional configs will adjust the formatting of Marginalia's SQL comments:
key_value_separator: inject the key-value separator, which is a colon:by defaultquote_values: configure whether the values will be wrapped in a single quoted string. By default, this isfalse, which will not wrap the values at all.For example, if a client uses the following configuration:
Marginalia::Comment.key_value_separator = '=' Marginalia::Comment.quote_values = :singleit will adjust the formatting from this:
"select id from posts /*application:Joe's app,controller:my_controller*/"to this:
"select id from posts /*application='Joe\\'s app',controller='my_controller*/"Note that this PR is backwards-compatible with existing code. If neither
quote_valuesnorkey_value_separatorare set, their default values will retain all existing behavior. Unit tests have also be added to ensure there are no regressions.
@sj26 I would love to get a review on this PR when you have a chance. Or let me know if there's anything I can do to help get this reviewed.
Kindly pinging the marginalia authors (@arthurnn @jeremy) - is there anything I can do to help get this PR reviewed?
Also pinging @balachandr and @odeke-em, as you both had comments in the original PR: https://github.com/basecamp/marginalia/pull/89
I did my best to write up a concise description of the new behavior, and demonstrate that it's all backwards compatible.