Encapsulate format logic within Statement and helper classes
Summary
Handling different I/O formats added a lot of complexity to the codebase: whenever a specific format is needed or requested, that format has to be passed around to whatever handles the response so that it can be parsed correctly. Various methods around parsing responses according to different formats have been growing and collecting within the SchemaStatements module, where they don't really seem to belong. process_response also has nested case statements that are hard to grok.
This PR extracts three helper classes:
FormatManagerto determine when to automatically apply aFORMATclause to a SQL statement,ResponseProcessorto parse the response according to the requested format and handle errors, andStatementto couple them together and maintain the format state.
The first two are internal to Statement and have no reason to be used directly outside of it.
Breaking Changes
- Move
DEFAULT_RESPONSE_FORMATconstant into ClickhouseAdapter, since it's now used by helper classes instead of only within the SchemaStatements module.
Deprecations
- Passing
format:keyword toexecuteis deprecated (with no specific horizon yet) in favor of the newwith_response_formatwrapper method. The deprecation warning points users to the new method. executeanddo_executehad effectively identical signatures, sodo_executehas been deprecated.
Next Steps
The skip_format? check in the FormatManager really just includes checks that were already being run plus a few more we've found. They are not comprehensive. Ideally, we should go through the documentation to identify exactly which statements do or don't accept a FORMAT clause.
The chain of conditionals is also less than ideal. It's ugly but manageable for now. Whenever the complete list of inclusions/exclusions materializes, it may be better to apply a filter pattern to clean up the checks.
@leboshi need resolve conflicts
Sorry that took so long! It's been a busy couple months.
OK, all the above should be resolved! Thanks to both of you for the review!
🎉 We rebased, got tests passing, and have this branch working against our code, except for one other PR we're about to submit. The last requested change about exec_insert has also been made. Should be good to go!