search

kurtbuilds / sqlmo

SQL data primitives. Use it to generate SQL queries, auto-generate SQL migrations, and more.
https://crates.io/crates/sqlmo
4 stars 2 forks source link

Thoughts on including sqlparser AST models? #3

Open philipcristiano opened 4 months ago

philipcristiano commented 4 months ago

While exploring adding foreign keys to sqlmo I came across sqlparser for parsing the constraint properly.

There are AST structs for ~all SQL elements needed.

What are your thoughts for including this library and separately, potentially migrating to the AST portion for internal representation where possible? The library also includes methods for generating SQL.

kurtbuilds commented 4 months ago

I considered it. Ultimately, the problem I found is that SQL is really complex, and using those structs was both way more complex than needed, and didn't store data to enable some shortcuts in other cases. Happy to here arguments on the benefit of doing so though, and given that, welcome PRs to accomplish it.

philipcristiano commented 4 months ago

didn't store data to enable some shortcuts in other cases

Do you happen to recall which data wasn't being stored? From my perusal sqlparser is very much not enough to figure out a schema delta but does have the elements for representing the required modifications. It doesn't seem to have the datastructures for ordering changes, which would fit well into this project.

The benefits as I see it from exploring foreign keys is mainly around completeness of the SQL implementation. As you remark, SQL is complex. As an example, it appears as if CreateIndex doesn't support column ordering yet as sqlparser does. Not all PG Index types are though :/ As this project implementation expands, it will wind up looking more like the sqlparser AST. If you goal is more along supporting ormlite and less for a complete implementation then it would make sense not to add that complexity.

External libs also mean more surface area, larger compile sizes, and slower iteration for changes when needed.

philipcristiano commented 4 months ago

sqlparser's Statement enum wraps ~all needed SQL objects. There isn't a struct though for each, only enum variants. This makes it very difficult/annoying to do any more than matching such as passing them around with any of Rust's type safety, without some custom wrapping.