fix(schema_cache): return results

[juleswritescode]

We were calling .unwrap() on all database queries that loaded the schemas for the SchemaCache. I changed them so they (and the parent SchemaCache.load(..)) return Results.

Also:

• I adjusted the callsites were SchemaCache.load() is called
• I added a SchemaCacheError that implements the Diagnostic trait and implemented From<SchemaCacheError> for WorkspaceError
• I made sure to use the get_new_test_db() from pg_test_utils in some tests – they were failing for me because of a missing env var, hope this didn't break anything?

Please check:
I'm not sure yet how the whole Diagnostic thing works. Does it need to be implemented for the SchemaCache?
I also wondered if we even want a specific SchemaCacheError type – we could also return a sqlx::Error, since there are only database connections and there's no other fallible business logic?

Lastly, the test_check_sql test in pg_typecheck/src/lib.rs suddenly failed because we're now getting a "contact" instead of an "unknown" string – is this the correct output? :)

Thank yooouuu

[psteinroe]

Thanks! Regarding the error: I think both approaches are totally valid. I don't think the schema cache will ever return any other error than the one emitted by sqlx. Since the workspace already implements a conversion for sqlx (I think), we could also simplify that here. But we can also keep it like it is now to have it more explicit. Whatever you prefer 🙌

[psteinroe]

I think the typecheck error could be coming from me accidentally changing the db url to a supabase db url. I have it running always so I tend to just change the url to it so that I don't have to run another db 😅 we can keep the fix in the test, or revert my change.

[psteinroe]

awesome! Check my comments for answers to your open points :)

[juleswritescode]

Thanks! Regarding the error: I think both approaches are totally valid. I don't think the schema cache will ever return any other error than the one emitted by sqlx. Since the workspace already implements a conversion for sqlx (I think), we could also simplify that here. But we can also keep it like it is now to have it more explicit. Whatever you prefer 🙌

Let's remove it. It was a nice exercise but the code is simpler without it. :)

Edit: Done ✅