Getting rid of probably redundant params indexing while querying#10
Getting rid of probably redundant params indexing while querying#10wiluite wants to merge 1 commit into
Conversation
|
Thank you so much for taking the time to submit this PR and for the kind words about the library! I really appreciate you diving into the code and identifying these improvements. |
|
Yes, all tests of the project are still working/passed, including QueryParamsWriteTest in module db_io_tests.cpp. I noticed that param format builder specialization for params that are only in text format seems not engaged in QueryParamsWriteTest (this specialization seems picked up for some other tests but not for that). If need be, I probably could add yet another test (say, QueryTextParamsWriteTest) to db_io_test.cpp to check against say: ...make_test_data(std::string("a"), std::string("b")...). |
|
There's no need to test text parsing extensively, the hard part is binary protocol. I'll merge the pr as soon as the power outage is over. Just curious, are you using the library in some project or just for studying purposes? For production purposes I'd recommend you the https://userver.tech C++ framework, it has a postgres driver also written by me. I It's binary protocol only and template magic is more elegant there |
|
Yes, just for studying, for learning techniques, approaches, and so on in high-professional code. Im just an old programmer (with young soul;-), retired from previous occupation (not IT) and having time to look around. Such things like metapushkin/afsm/pg_async were, for years, unclosed gestalt for me (especially afsm), and this year sometime I worked hard to close it in key aspects. |
Hi.
This PR is about more concise code for the purpose of its clear understanding by those interested.
I'm just learning template metaprogramming from your projects.
I noticed that when forming a query within the params_format_builder, format_selector, and nth_param template classes, numeric indexing isn't actually used, and apparently doesn't matter, since the data types themselves are sufficient to generate all the necessary data for output.
And perhaps the numeric indexing was left in the code for the convenience of quick debugging at the right moment.
If it does make sense in the release version, could you please clarify?
Otherwise, you could decide to accept this.
Thanks!