Why is the Schema not part of the Adapter?

[DrRuhe]

I am wondering why the schema is not integrated into the Adapter trait but why it is decoupled instead. It seems to me like the adapter can only query the things specified in the schema anyway.
What implications would occur when integrating the Schema definition into the Adapter trait by e.g. a new Adapter::schema method?

[obi1kenobi]

Great question! Many adapters do in fact define an inherent method for getting a schema, exactly as you suggest. For example: https://github.com/obi1kenobi/trustfall-rustdoc-adapter/blob/rustdoc-v24/src/adapter/mod.rs#L44

The method isn't part of the trait itself because there wasn't a clear and obvious advantage to requiring it, and there was a downside of "it isn't clear whether the method should return Schema or Arc<Schema> or &'static Schema or something else entirely." At the point where Trustfall uses the adapter, Trustfall itself no longer needs a schema. The adapter might want a schema for its own internal use, but it didn't make sense to then demand it in the trait.

Also, in principle, one might want to be able to get the schema without making (or even knowing about) the adapter. For example, if a user is writing a query in their browser, having a schema (but not an adapter!) in the browser means you can type-check their query, offer autocompletions, etc. The adapter is simply not necessary here, and requiring impl Adapter to provide that functionality would just get in the way.

[DrRuhe]

Is the schema required for execution?
It seems a bit weird, that the Schema is required for execute_query but not part of the adapter.

I will experiment a bit with it, but currently, I think having a method that returns an Option<Schema> (or something similar) might be sensible. In general, the Schema can still be used on its own, without requiring an Adapter.

For #273, I'd like to be general across adapters, and the decoupled schema makes this difficult, as the schema has to be provided again for each adapter.

[obi1kenobi]

From the perspective of #273 I think it makes sense to make another trait (perhaps a subtrait of Adapter, or at least with a blanket impl of Adapter) that knows about the schema and about the adapter at the same time.

Whether the schema is required for execution depends on what counts or doesn't count as "execution." Overall, I lean toward "no" as the answer, and I'm outlining my reasoning below.

If execution is the entire process of going from a query string to results, then yes, a schema is required.

But that process is actually a series of steps: parsing, validation and type-checking, translation into IR, and then finally execution. The schema is only needed for type-checking (step 2), and the adapter is only needed for the execution part (step 4). The two steps aren't even adjacent.

It's actually possible to execute a query without an adapter — for example, by compiling the IR directly into SQL and running that query on a SQL database. This is something I've done before in a prior project, and I've made sure it's possible in Trustfall too. In this case, the schema is still required, though, because we replaced step 4 and not step 2 in the process above.

Similarly, if you have a query that runs frequently without changing, it doesn't need to be re-parsed, re-type-checked, re-translated etc. each time. The program could complete all those steps once (perhaps even at compile-time in a proc-macro!) and then just evaluate the IR on the adapter each time the results are needed. Again, in this process the schema is only needed in the initial stages, and not when the adapter is used.

Like I mentioned, I'm in favor of having another trait (initially outside Trustfall itself, perhaps in the future upstreamed into Trustfall when we're confident in the design) that knows about the schema and its adapter at the same time. I think that's a better option than requiring adapters to also be schema-builders.