UDF Runner V2: Interface skeleton by kratz00 · Pull Request #65 · exasol/udf-runner-cpp

kratz00 · 2026-09-25T09:34:28Z

No description provided.

Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>

tomuben · 2026-10-08T15:25:10Z

+class CallMessageView
+{
+public:
+    explicit CallMessageView(const CallMessage* value = nullptr) : value_(value)


Is it really a good idea to pass CallMessage as raw pointer? Would a shared_ptr or unique_ptr be better?

tomuben · 2026-10-08T15:34:47Z

+    /// Returns whether call-opening metadata is present.
+    [[nodiscard]] bool has_open_call() const noexcept;
+    /// Returns call-opening metadata, or null when absent.
+    [[nodiscard]] const OpenCall* open_call() const noexcept;


I'am not sure if it's a good idea to return raw pointers here. Wouldn't it be better to return the std::optional?

tomuben · 2026-10-08T15:35:52Z

In many places you use the -> operator to access the value of a std::optional. This is against our coding style guidelines:

For std::optional, use has_value() when testing presence and value() when explicitly retrieving the contained value. Name the variable after its value, not after the fact that it is optional.

Signed-off-by: Steffen Pankratz <steffen.pankratz@exasol.com>

sonarqubecloud · 2026-10-09T11:21:17Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

tkilias · 2026-10-11T11:04:50Z

+    TransportError
+};
+/// Machine-readable and human-readable operation error details.
+struct ErrorInfo


Maybe actually use a class because the members should be constant after creation

tkilias · 2026-10-11T11:06:43Z

+using Timeout = std::optional<std::chrono::steady_clock::duration>;
+
+/// Named string or binary payload.
+struct Payload


We should split this file, we have general types for the interface and then types for the content

tkilias · 2026-10-11T11:09:51Z

+    [[nodiscard]] bool has_server_capabilities() const noexcept;
+    /// Returns server capabilities, or an empty optional when absent.
+    [[nodiscard]]
+    std::optional<std::reference_wrapper<const ServerCapabilities>> server_capabilities()


Can we use the Flatbuffer object here instead of defining a new new type which we need to synchronize

tkilias · 2026-10-11T11:10:51Z

+    /// Returns whether payloads are present.
+    [[nodiscard]] bool has_payloads() const noexcept;
+    /// Returns payloads, or an empty optional when absent.
+    [[nodiscard]] std::optional<std::reference_wrapper<const Payloads>> payloads() const noexcept;


Can we use the Flatbuffer object here instead of defining a new new type which we need to synchronize

tkilias · 2026-10-11T11:11:31Z

+    /// Returns whether an error is present.
+    [[nodiscard]] bool has_error() const noexcept;
+    /// Returns the error, or an empty optional when absent.
+    [[nodiscard]] std::optional<std::reference_wrapper<const ErrorInfo>> error() const noexcept;


Can we use the Flatbuffer object here instead of defining a new new type which we need to synchronize

tkilias · 2026-10-11T11:12:58Z

+    /// Returns whether connection shutdown is being started or acknowledged.
+    [[nodiscard]] bool has_close_connection() const noexcept;
+    /// Returns whether this message starts or acknowledges connection shutdown.
+    [[nodiscard]] bool close_connection() const noexcept;


has_close_connection() and close_connection() are duplicated

tkilias · 2026-10-11T11:14:11Z

+    /// Returns whether a keep-alive is present.
+    [[nodiscard]] bool has_keep_alive() const noexcept;
+    /// Returns whether this message is a keep-alive message.
+    [[nodiscard]] bool keep_alive() const noexcept;


KeepAlive is a table for a reason in the Flatbuffer we should return the Flatbuffer object

kratz00 requested a deployment to v2-fuzzing-pr-approval September 25, 2026 09:34 — with GitHub Actions Waiting