Skip to content

Getting rid of probably redundant params indexing while querying#10

Open
wiluite wants to merge 1 commit into
zmij:developfrom
wiluite:probably-redundant-params-indexing
Open

Getting rid of probably redundant params indexing while querying#10
wiluite wants to merge 1 commit into
zmij:developfrom
wiluite:probably-redundant-params-indexing

Conversation

@wiluite

@wiluite wiluite commented Oct 1, 2025

Copy link
Copy Markdown

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!

@zmij

zmij commented Oct 6, 2025

Copy link
Copy Markdown
Owner

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.
I have to admit, it's been about 7 years since I last worked on this project, so I don't recall the intricate details of why those particular type index choices were made originally. Your analysis looks thorough and well-reasoned.
You're absolutely right that in compiled code, these type index optimisations don't affect runtime performance - the compiler sorts it all out regardless. The improvements you've made seem to be more about code clarity and following better practices, which is always valuable.
Have you had a chance to run the tests to make sure everything still builds and works correctly? I never set up proper CI for this library (something I probably should have done!), so manual testing is still the way to go.
I'm grateful that people like you are still finding the library useful and taking the time to contribute back. Thank you for keeping the project alive with your contribution!

@wiluite

wiluite commented Oct 9, 2025

Copy link
Copy Markdown
Author

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")...).

@zmij

zmij commented Oct 11, 2025

Copy link
Copy Markdown
Owner

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

@wiluite

wiluite commented Oct 18, 2025

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants