router icon indicating copy to clipboard operation
router copied to clipboard

new graphql data representation

Open Geal opened this issue 1 year ago • 2 comments

The router is currently using serde_json_bytes::Value, a type derived from the similar one in serde_json, to represent JSON data. It has been used well so far, and the optimization using the Bytes structure as underlying storage for strings helped a lot. But there's a few issues with this type, that we could address by changing it:

  • we reexport this type from another crate, while it is a fundamental part of the graphql response. We can't really evolve it (we have similar issues with the hyper crate). We can modify serde_json_bytes but it still has a lot of adherence to serde_json
  • we don't really use the data in the router. It is transformed a bit by inserting in the right array or object, and we validate enum values and number values, but we should not need to parse them
  • similarly, we use some complex and slow types like IndexMap that could be replaced with simpler vecs, because with some tweaks, we would not really require random access

Proposal

Instead of reexporting a type directly, we make an internal router type, with a similar API to serde_json_bytes::Value, but that hides the internal details. We make this API available from rhai too, to avoid large data transformations there. And once we have this API, we can look at improving the type freely in the future

Geal avatar Apr 11 '24 11:04 Geal

Not sure about not needing random access. One of the things that has come up is the need to be able to select via json path into these structures. Agree with the rest of the proposal though.

BrynCooke avatar Apr 11 '24 12:04 BrynCooke

it is useful some time, but the great majority of use cases (by number of times it is run in query execution) is iterating over the list of keys and values and filtering it

Geal avatar Apr 11 '24 14:04 Geal