Skip to content

How to design the WIT layer for pg host component #576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
miketang84 opened this issue Jun 16, 2022 · 8 comments
Closed

How to design the WIT layer for pg host component #576

miketang84 opened this issue Jun 16, 2022 · 8 comments

Comments

@miketang84
Copy link
Contributor

miketang84 commented Jun 16, 2022

Hi, I'm designing the feature of supporting the postgres as a host component, the first problem is:

The client.query() returns Vec<Row> which is defined in https://docs.rs/tokio-postgres/0.7.6/tokio_postgres/row/struct.Row.html.

And the outbound-redis sample just uses a plain string as parameters.

I wonder that if I need to map the definition of Row and its inner types into the wit types one by one (maybe it's difficult, and it has two generics in its' method parameters), or is there some simple way to just use the string as input and output parameters.

Thank you.

@miketang84
Copy link
Contributor Author

Oh, I found the item of resource identifier.

https://github.com/bytecodealliance/wit-bindgen/blob/main/WIT.md#item-resource

Wish it helps.

@dicej
Copy link
Contributor

dicej commented Jun 21, 2022

I agree that representing a Row as a WIT resource is the most natural option. Regarding get/try_get methods: since WIT does not support arbitrary generics, it may be necessary to split each of those methods separate versions for each index and value type. For example:

resource row {
  columns: func() -> list<column>
  is-empty: func() -> bool
  len: func() -> u64
  get-i64-by-index: func(index: u32) -> expected<i64, error>
  get-string-by-index: func(index: u32) -> expected<string, error>
  get-bytes-by-index: func(index: u32) -> expected<list<u8>, error>
  ...
  get-i64-by-name: func(name: string) -> expected<i64, error>
  get-string-by-name: func(name: string) -> expected<string, error>
  get-bytes-by-name: func(name: string) -> expected<list<u8>, error>
  ...
}

Since PostgreSQL has a lot of types, that would end up being a lot of methods, and even then it wouldn't support all the user-defined FromSql types tokio-postgres does. An alternative would be to only support get-bytes-by-index and get-bytes-by-name, using some language-agnostic serialization format (e.g. JSON or CBOR) to encode the data.

@miketang84
Copy link
Contributor Author

miketang84 commented Jun 23, 2022

I found that the redis outbound sample is too simple, on every publish/set/get, it open and close the connection in the host function implementation. This design avoids the definition of something like resource to keep the redis connection across the border of the wasmtime and the wasm sandbox. But for the prototype, I think it makes sense. And for the prototype of outbound-pg, I will follow the design.

@miketang84
Copy link
Contributor Author

miketang84 commented Jun 23, 2022

And for the returned results of query(), I decide to just iterate on the rows and columns and construct a two dimension of array of raw u8 data, that is Vec<Vec<Vec<u8>>> to pass to the sandbox wasm code. This also keeps it opaque and leave the host component code boilerplate without biz logic.

@miketang84
Copy link
Contributor Author

I've done the concept verification, later pull request.

@dicej
Copy link
Contributor

dicej commented Jun 29, 2022

@miketang84
Copy link
Contributor Author

Cool, thank you! @dicej

@kate-goldenring kate-goldenring moved this to 🆕 Triage Needed in Spin Triage Aug 16, 2022
@kate-goldenring kate-goldenring moved this from 🆕 Triage Needed to 📋 Investigating in Spin Triage Aug 17, 2022
@dicej
Copy link
Contributor

dicej commented Aug 15, 2023

Spin now has an official Postgres host component, and there's an ongoing effort to define a standard interface for SQL, so I'm going to close this. Further discussion should probably happen at the wasi-sql repo.

@dicej dicej closed this as completed Aug 15, 2023
@github-project-automation github-project-automation bot moved this from 📋 Investigating / Open for Comment to ✅ Done in Spin Triage Aug 15, 2023
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

No branches or pull requests

2 participants