feat: Simple query interface #2201
Conversation
| /// Get an existing session for a database, or creates a new one using the | ||
| /// provided configuration. | ||
| async fn get_or_initialize_session( | ||
| &self, | ||
| db_id: Uuid, | ||
| storage_conf: SessionStorageConfig, | ||
| ) -> Result<RemoteSession> { | ||
| let sess = match self.sessions.get(&db_id) { | ||
| Some(sess) => sess.clone(), | ||
| None => { | ||
| info!(session_id=%db_id, "initializing remote session"); | ||
| let context = self | ||
| .engine | ||
| .new_remote_session_context(db_id, storage_conf) | ||
| .await?; | ||
|
|
||
| let sess = RemoteSession::new(context); | ||
| self.sessions.insert(db_id, sess.clone()); | ||
| sess | ||
| } | ||
| }; | ||
|
|
||
| Ok(sess) | ||
| } |
There was a problem hiding this comment.
I factored this out since I was originally going to use the remote sessions for this, but the remote session doesn't having all the capabilities we need to actually execute sql queries.
There was a problem hiding this comment.
The definitions Cloud cares about.
Note that this does import from common.proto, so it's not enough to just copy this file, we'll want to make sure protoc has access to the entire directory.
| // TODO: We may want to extend this to actually stream/return the record | ||
| // batches. |
There was a problem hiding this comment.
Question(if-minor): Is this something we should hold off until we change to Flight?
There was a problem hiding this comment.
We don't need to. Depends on the requirements of cloud. It would be straightforward to add the batches to this if cloud needs the results (we already have all the types needed). It would just be a cloud to know how to deserialize those batches (which it'd also need for flightsql anyways).
| @@ -7,6 +7,9 @@ syntax = "proto3"; | |||
|
|
|||
| package rpcsrv.common; | |||
|
|
|||
| // Storage config for the session. | |||
| message SessionStorageConfig { optional string gcs_bucket = 1; } | |||
There was a problem hiding this comment.
I might have let this be any kind of bucket?
(are we talking about session state or default location for storage for anything?
There was a problem hiding this comment.
This is for database table data.
Currently this is the equivalent to this: https://github.com/glaredb/glaredb/blob/425681572f04b796739c5f1b761caddc96eae59a/crates/sqlexec/src/engine.rs#L37-L44
We should (at some point) make it more generic to allow things other than gcs buckets, but I think this is fine for now as its very specific to where we currently deploy cloud (gcp).
| if rpc_bind.is_none() && enable_simple_query_rpc { | ||
| return Err(anyhow!( | ||
| "An rpc bind address needs to be provided to enable the simple query interface" | ||
| )); | ||
| } |
There was a problem hiding this comment.
given the impending use of this RPC interface for flightsql (e.g. attaching different grpc services to this RPC endpoint, we might just know that we'll need to
Adds a simple query interface for use with Cloud.