Typehandler simplification and other modernizations in Poco::Data#5136
Merged
Typehandler simplification and other modernizations in Poco::Data#5136
Conversation
aleks-f
requested changes
Dec 22, 2025
Member
There was a problem hiding this comment.
thank you for this contribution. this, like the tuple overhaul, was long overdue. would you mind adding tests for sqlite and odbc? you should be able to test odbc against mysql and/or postgres locally, but we also have odbc ci jobs, so we'll see any problems there. sqlite and odbc are widely used, so it would make me more comfortable approving if we had those tests passing, too
@matejk unless there are some unforeseen complications with sqlite or odbc, I would like to have this in 1.15, so we can postpone for a day or so
Member
Author
|
@aleks-f Yes, I can do that. Hopefully later today. |
matejk
approved these changes
Dec 23, 2025
Contributor
matejk
left a comment
There was a problem hiding this comment.
These changes are a valuable contribution to the Poco. I have no objections. Clang-tidy has identified further improvements that I have added to this PR.
aleks-f
added a commit
that referenced
this pull request
Dec 24, 2025
…5136 - Add TypeInfo::reset() to clear cached type info, called on session close to fix testReconnect failing when connection is reestablished - Move common test methods from ODBC SQLExecutor to shared DataTest SQLExecutor for better code reuse across database backends - Improve error message in hasTransactionIsolation() to be more descriptive - Refactor ODBC MySQL and PostgreSQL test executors to delegate to shared base implementation - Update MySQL, PostgreSQL, and SQLite test suites to use refactored executor methods - Add Nix shell environments for MySQL and PostgreSQL test databases with auto-start/stop and ODBC driver configuration
aleks-f
added a commit
that referenced
this pull request
Dec 24, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 24, 2025
aleks-f
added a commit
that referenced
this pull request
Dec 24, 2025
#5140) * fix(Data): Refactor ODBC tests and fix testReconnect TypeInfo caching #5136 - Add TypeInfo::reset() to clear cached type info, called on session close to fix testReconnect failing when connection is reestablished - Move common test methods from ODBC SQLExecutor to shared DataTest SQLExecutor for better code reuse across database backends - Improve error message in hasTransactionIsolation() to be more descriptive - Refactor ODBC MySQL and PostgreSQL test executors to delegate to shared base implementation - Update MySQL, PostgreSQL, and SQLite test suites to use refactored executor methods - Add Nix shell environments for MySQL and PostgreSQL test databases with auto-start/stop and ODBC driver configuration * chore: pass executor name * chore: nix-shell documentation * chore: nix-shell documentation * feat: add optional/tuple tests to ODBC Oracle and SQLServer #5136 * fix(ODBC): add missing functions implementations #5136
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Poco::Data::TypeHandler Improvements
Makes use of @matejk's new implementation of Poco::Tuple to simplify Poco::Data::TypeHandler and adds specializations for std::tuple (of any size) and std::optional. The code size is reduced from 249 KB to 15.8 KB. It uses C++ variadic templates and if constexpr.
The template specialization for Poco::Tuple is based on the TypeHandler for std::tuple. To make this work, I added explicit conversion operators to Poco::Tuple that make it interoperable with std::tuple. I think this is harmless because it cannot be used to undermine a class invariant. If you still consider this undesirable, the TypeHandler for Poco::Tuple could be implemented analogous to that for std::tuple, at the cost of some code duplication.
The TypeHandlers for std::tuple and std::optional allow to use Poco::Data without having to use the Poco-specific classes Poco::Tuple and Poco::Nullable. The code is battle-tested in production. A few test cases are added.
More consistent use of virtual/override
Overridden virtual methods in Poco::Data sometimes have an "override" annotation, sometimes are prefixed with "virtual", sometimes have no annotation at all. I tried to make it more consistent (there may be other instances I didn't capture). I think this makes it easier to understand the complex class hierarchy in Poco::Data and increases safety.
Use explicitly deleted constructors and assignment operators
In many cases, the code uses the old idiom of declaring special methods as private, but not defining them. Of course, explicitly deleting them makes intent clearer and allows to remove declarations from derived classes. Again, I don't claim complete coverage.