Postgres#57
Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial PostgreSQL (libpq) examples and the plumbing needed to integrate a “poll for readiness” operation into beman.net’s execution-based async model.
Changes:
- Introduces
async_pollsender/CPO and acontext_base::poll()virtual to support readiness waiting. - Updates
io_context::async_run()scheduling strategy and adds small utility changes (e.g.,make_socket(fd),event_typestream output). - Adds Postgres example programs + build-system detection/linkage for
libpq.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| include/beman/net/detail/scope.hpp | Modifies scope::run() behavior (currently stops driving io_context). |
| include/beman/net/detail/poll_context.hpp | Adds poll() implementation and a temporary <iostream> include. |
| include/beman/net/detail/operations.hpp | Adds async_poll/poll_desc sender description. |
| include/beman/net/detail/io_context_scheduler.hpp | Exposes poll() forwarding on the scheduler. |
| include/beman/net/detail/io_context.hpp | Adds make_socket(fd) and rewrites async_run() to use an env scheduler. |
| include/beman/net/detail/event_type.hpp | Adds operator<< for event_type. |
| include/beman/net/detail/context_base.hpp | Adds poll_operation type and new pure-virtual poll() method. |
| include/beman/net/detail/basic_socket.hpp | Minor member formatting / TODO comment. |
| examples/postgres.cpp | New example demonstrating async libpq usage with beman execution/net. |
| examples/populate-postgres.cpp | New helper example to populate the Postgres table. |
| examples/postgres.txt | New setup instructions for Postgres role/db/table and .pgpass. |
| examples/empty.cpp | Temporarily disables prior example code behind #if 0. |
| examples/CMakeLists.txt | Conditionally adds Postgres examples and links against pq. |
| CMakeLists.txt | Adds rudimentary libpq detection and gates examples subdir. |
| Makefile | Adds clang-format alias target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set(POSTGRESROOT /Library/PostgreSQL/18) | ||
| check_library_exists(pq PQsocket ${POSTGRESROOT}/lib HAS_POSTGRES) |
There was a problem hiding this comment.
POSTGRESROOT is hard-coded to /Library/PostgreSQL/18, which is platform- and install-specific. Make this configurable (CACHE PATH) and/or use find_package(PostgreSQL) / pkg-config to locate libpq in a cross-platform way; otherwise the build system will behave unexpectedly on non-macOS or different Postgres installs.
| set(POSTGRESROOT /Library/PostgreSQL/18) | |
| check_library_exists(pq PQsocket ${POSTGRESROOT}/lib HAS_POSTGRES) | |
| find_package(PostgreSQL QUIET) | |
| if(PostgreSQL_FOUND) | |
| set(HAS_POSTGRES TRUE) | |
| else() | |
| set(HAS_POSTGRES FALSE) | |
| endif() |
| auto main() -> int { | ||
| std::cout << std::unitbuf << "hello world\n"; | ||
| #if 0 | ||
| net::io_context context; | ||
| net::ip::tcp::endpoint ep(net::ip::address_v4::any(), 12345); | ||
| net::ip::tcp::acceptor server(context, ep); | ||
|
|
||
| ex::sync_wait(net::async_accept(server)); | ||
|
|
||
| context.run(); | ||
| #endif |
There was a problem hiding this comment.
This example now prints a message and compiles out the networking logic with #if 0, which makes the empty example effectively a no-op. If this is temporary debugging, consider removing it before merging or gating it behind an explicit build option.
| //-dk:TODO return beman::execution::when_all(this->_counting_scope.join(), this->_io_context.async_run()); | ||
| return beman::execution::when_all(this->_counting_scope.join()); |
There was a problem hiding this comment.
scope::run() no longer runs the underlying io_context (it only joins the counting scope). Any I/O scheduled in this scope will never make progress unless callers separately run the io_context, which breaks the scope abstraction. Restore when_all(join(), _io_context.async_run()) (or provide an equivalent that ensures the io_context is driven while the scope is active).
| //-dk:TODO return beman::execution::when_all(this->_counting_scope.join(), this->_io_context.async_run()); | |
| return beman::execution::when_all(this->_counting_scope.join()); | |
| return beman::execution::when_all(this->_counting_scope.join(), this->_io_context.async_run()); |
| std::ostringstream ins; | ||
| ins << "insert into messages (key, message) values(" << i << ", '" << message << "');"; | ||
| std::cout << "inserting: " << ins.str() << '\n'; | ||
| pq::result res(PQexec(conn.get(), ins.str().c_str())); |
There was a problem hiding this comment.
The SQL is built by concatenating message directly into a quoted literal. If message contains ' (or other special characters), the INSERT will fail (and it also models unsafe query construction). Use PQescapeLiteral / parameterized queries, or at minimum escape single quotes before building the statement.
| std::ostringstream ins; | |
| ins << "insert into messages (key, message) values(" << i << ", '" << message << "');"; | |
| std::cout << "inserting: " << ins.str() << '\n'; | |
| pq::result res(PQexec(conn.get(), ins.str().c_str())); | |
| const char* const insert_sql = "insert into messages (key, message) values($1, $2);"; | |
| std::string key = std::to_string(i); | |
| const char* const param_values[] = {key.c_str(), message.c_str()}; | |
| std::cout << "inserting: " << insert_sql << " [key=" << key << ", message=" << message << "]\n"; | |
| pq::result res(PQexecParams(conn.get(), insert_sql, 2, nullptr, param_values, nullptr, nullptr, 0)); |
| inline std::ostream& operator<<(std::ostream& os, ::beman::net::event_type e) { | ||
| switch (e) { | ||
| default: | ||
| return os << "invalid(" << ::std::uint8_t(e) << ")"; |
There was a problem hiding this comment.
Streaming std::uint8_t will typically print as a character, not a number. Cast to a larger integer type (e.g., unsigned int) before inserting into the stream so invalid(...) prints a readable numeric value.
| return os << "invalid(" << ::std::uint8_t(e) << ")"; | |
| return os << "invalid(" << static_cast<unsigned int>(::std::uint8_t(e)) << ")"; |
| @@ -0,0 +1,54 @@ | |||
| // examples/postgres.cpp -*-C++-*- | |||
There was a problem hiding this comment.
The file header comment says examples/postgres.cpp but this file is populate-postgres.cpp. Update the header to match the actual filename to avoid confusion when navigating or grepping.
| // examples/postgres.cpp -*-C++-*- | |
| // examples/populate-postgres.cpp -*-C++-*- |
| auto set_value([[maybe_unused]] operation& o, auto&& receiver) { | ||
| ::beman::net::detail::ex::set_value(::std::move(receiver), | ||
| //::std::get<0>(o) | ||
| ::beman::net::event_type{}); | ||
| } | ||
| auto get_scheduler() { return this->d_socket.get_scheduler(); } | ||
| auto submit([[maybe_unused]] auto* base) -> ::beman::net::detail::submit_result { | ||
| ::std::get<1>(*base) = this->d_mask; |
There was a problem hiding this comment.
poll_desc::set_value() always completes with event_type{} (i.e., none), ignoring the operation state. This makes async_poll() return the wrong result. Populate the operation with the observed events in the context implementation and return that stored value here (and avoid overwriting the same slot in submit() if it’s meant to hold the result).
| auto set_value([[maybe_unused]] operation& o, auto&& receiver) { | |
| ::beman::net::detail::ex::set_value(::std::move(receiver), | |
| //::std::get<0>(o) | |
| ::beman::net::event_type{}); | |
| } | |
| auto get_scheduler() { return this->d_socket.get_scheduler(); } | |
| auto submit([[maybe_unused]] auto* base) -> ::beman::net::detail::submit_result { | |
| ::std::get<1>(*base) = this->d_mask; | |
| auto set_value(operation& o, auto&& receiver) { | |
| ::beman::net::detail::ex::set_value(::std::move(receiver), ::std::get<0>(o)); | |
| } | |
| auto get_scheduler() { return this->d_socket.get_scheduler(); } | |
| auto submit(auto* base) -> ::beman::net::detail::submit_result { |
No description provided.