lidavidm commented on code in PR #34817:
URL: https://github.com/apache/arrow/pull/34817#discussion_r1394748940
##########
format/Flight.proto:
##########
@@ -525,3 +525,108 @@ message FlightData {
message PutResult {
bytes app_metadata = 1;
}
+
+/*
+ * EXPERIMENTAL: Union of possible value types for a Session Option to be set
to.
+ */
+message SessionOptionValue {
Review Comment:
Just wondering; was the standard
[google.protobuf.Value](https://protobuf.dev/reference/protobuf/google.protobuf/#value)
considered?
##########
cpp/src/arrow/flight/types.h:
##########
@@ -750,6 +754,190 @@ struct ARROW_FLIGHT_EXPORT CancelFlightInfoRequest {
static arrow::Result<CancelFlightInfoRequest> Deserialize(std::string_view
serialized);
};
+/// \brief Variant supporting all possible value types for
{Set,Get}SessionOptions
+using SessionOptionValue = std::variant<std::string, bool, int32_t, int64_t,
float,
+ double, std::vector<std::string>>;
+
+/// \brief The result of setting a session option.
+enum class SetSessionOptionStatus : int8_t {
+ /// \brief The status of setting the option is unknown.
+ ///
+ /// Servers should avoid using this value (send a NOT_FOUND error if the
requested
+ /// query is not known). Clients can retry the request.
+ kUnspecified,
+ // The session option setting completed successfully.
+ kOk,
+ /// \brief The given session option name was an alias for another option
name.
+ kOkMapped,
+ /// \brief The given session option name is invalid.
+ kInvalidKey,
+ /// \brief The session option value is invalid.
+ kInvalidValue,
+ /// \brief The session option cannot be set.
+ kError
+};
+std::ostream& operator<<(std::ostream& os, const SetSessionOptionStatus& r);
+
+/// \brief The result of closing a session.
+enum class CloseSessionStatus : int8_t { kUnspecified, kClosed, kClosing,
kNotClosable };
+std::ostream& operator<<(std::ostream& os, const CloseSessionStatus& r);
+
+static const char* const SetSessionOptionStatusNames[] = {
Review Comment:
nit: move this to the .cc file?
##########
cpp/src/arrow/flight/sql/server_session_middleware.h:
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Middleware for handling Flight SQL Sessions including session cookie
handling.
+// Currently experimental.
+
+#pragma once
+
+#include <functional>
+#include <optional>
+#include <shared_mutex>
+#include <string_view>
+
+#include "arrow/flight/server_middleware.h"
+#include "arrow/flight/sql/types.h"
+
+namespace arrow {
+namespace flight {
+namespace sql {
+
+static constexpr char const kSessionCookieName[] = "arrow_flight_session_id";
+
+class FlightSqlSession {
+ protected:
+ std::map<std::string, SessionOptionValue> map_;
+ std::shared_mutex map_lock_;
+
+ public:
+ /// \brief Get session option by key
+ ::arrow::Result<std::optional<SessionOptionValue>>
Review Comment:
Does this need to be `Result<optional>` or just `optional`?
##########
cpp/src/arrow/flight/sql/server_session_middleware.h:
##########
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Middleware for handling Flight SQL Sessions including session cookie
handling.
+// Currently experimental.
+
+#pragma once
+
+#include <functional>
+#include <optional>
+#include <shared_mutex>
+#include <string_view>
+
+#include "arrow/flight/server_middleware.h"
+#include "arrow/flight/sql/types.h"
+
+namespace arrow {
+namespace flight {
+namespace sql {
+
+static constexpr char const kSessionCookieName[] = "arrow_flight_session_id";
+
+class FlightSqlSession {
Review Comment:
```suggestion
class ARROW_FLIGHT_SQL_EXPORT FlightSqlSession {
```
##########
cpp/src/arrow/flight/client.h:
##########
@@ -383,6 +383,27 @@ class ARROW_FLIGHT_EXPORT FlightClient {
return DoExchange({}, descriptor);
}
+ /// \\brief Set server session option(s) by key/value. Sessions are generally
+ /// persisted via HTTP cookies.
+ /// \param[in] options Per-RPC options
+ /// \param[in] request The server session options to set
+ ::arrow::Result<SetSessionOptionsResult> SetSessionOptions(
+ const FlightCallOptions& options, const SetSessionOptionsRequest&
request);
+
+ /// \\brief Get the current server session options. The session is generally
+ /// accessed via an HTTP cookie.
+ /// \param[in] options Per-RPC options
+ /// \param[in] request The (empty) GetSessionOptions request object.
+ ::arrow::Result<GetSessionOptionsResult> GetSessionOptions(
+ const FlightCallOptions& options, const GetSessionOptionsRequest&
request);
+
+ /// \\brief Close/invalidate the current server session. The session is
generally
Review Comment:
```suggestion
/// \brief Close/invalidate the current server session. The session is
generally
```
nit (here and elsewhere)
##########
format/Flight.proto:
##########
Review Comment:
Would it be useful to also have an action to list supported session options?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
