What is acceptable as a PR?

[RomainMazB]

Hi there!
New in the Rust community, I have a decade of coding-skill in web backend development.
I've been active in the Laravel community, mostly in official-packages, OctoberCMS and WinterCMS (both based on the laravel/library but not the framework itself).

I was searching for a crate to create by myself to improve my rust-skills, and coming from the Laravel ecosystem, I felt like an ORM would be great since I didn't found a ORM with simplicity and "sentence-like" in mind, I thought that would benefit the entire Rust community. ... Until I found SeaORM, which seems to follow some of the Eloquent guidelines I like so much.

Instead of creating a crate by myself, I'm now interested into contributing into the SeaQL ecosystem. I read the codebase for two days long and I already spotted some important missing part like:

• SeaQuery can't generate a "basic asterisk query" like SELECT * FROM character, instead, you need to list all the columns.
• It seems like SeaQuery can't run or just insert a raw SQL statement inside a query which makes things difficult when you need aggregates or a function that is not supported yet or only supported in some DBMS so probably never supported inside SeaQuery.
• SeaORM doesn't support complex relationships like many to many with pivot table, polymorphic relationship.

But still, there is some few things that disturb me into the actual api. I don't want to change everything, I'm definitely not the boss in that room, and maintainers seems to be open in future to reduce some functions name probably some stuff are forced by the typing system which seems to be the priority in all crates but I wanted to know what would be acceptable to submit as a PR.

Example:

• in Eloquent, you don't need to specify the table name, which default to the plural name of the type of the model (i.e Post model are store into posts table, Category into categories table, and so on). Would it be acceptable to default the table name inside the macro, but still keeping a way to override it using the actual macro parameter? I know that Rust "reflection" may fail at some point, and is as of today only for debugging/logging purpose but I made a piece of code which seems to be not so ugly to my eyes.
• still in Eloquent, models come with two datetime columns created_at and updated_at which are automatically updated by the ORM to keep track of these without having that in mind. Is that acceptable to add them to the core with a way to disable this feature?

I have many more questions, which will come along my possible contributions, but the most important one is "How much welcoming is the maintainer team about public contributions?"

The project homepage, in comparison of most of the others crates make it feel like a "professional crate" which may become paid in future 😬.

Thanks to read,
Romain.

[billy1624]

Hey @RomainMazB, welcome to the party!

We are inspired by Laravel Eloquent and some other ORMs to develop similar ORM in Rust that focus on simplicity. Contributions are definitely welcome!

• SeaQuery can't generate a "basic asterisk query" like SELECT * FROM character, instead, you need to list all the columns.

I think that would be helpful for table with large number of columns.

• It seems like SeaQuery can't run or just insert a raw SQL statement inside a query which makes things difficult when you need aggregates or a function that is not supported yet or only supported in some DBMS so probably never supported inside SeaQuery.
• SeaORM doesn't support complex relationships like many to many with pivot table, polymorphic relationship.

We aim to support general SQL syntax in the SeaQuery core while keeping database specific features as extension. For example, type statement for PostgreSQL. We avoid introducing API that accept raw SQL (in String).

• in Eloquent, you don't need to specify the table name, which default to the plural name of the type of the model (i.e Post model are store into posts table, Category into categories table, and so on). Would it be acceptable to default the table name inside the macro, but still keeping a way to override it using the actual macro parameter? I know that Rust "reflection" may fail at some point, and is as of today only for debugging/logging purpose but I made a piece of code which seems to be not so ugly to my eyes.

We try to balance between explicit and verbose. I think just a line of proc_macro attribute is acceptable and seems to me clearer that having implicit table name provided under the hood.

sea-orm/src/tests_cfg/cake.rs

Lines 4 to 11 in 4b7234a

	#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
	#[sea_orm(table_name = "cake")]
	pub struct Model {
	#[sea_orm(primary_key)]
	pub id: i32,
	#[sea_orm(column_name = "name", enum_name = "Name")]
	pub name: String,
	}

• still in Eloquent, models come with two datetime columns created_at and updated_at which are automatically updated by the ORM to keep track of these without having that in mind. Is that acceptable to add them to the core with a way to disable this feature?

Yes, that are still missing. I think we can develop it alongside with soft delete.

The timestamp feature is better to be opt-in just like the soft delete. User can simply derive or implement a new trait for existing SeaORM entities. I guess it's more user friendly when upgrading SeaORM version.

Thanks for the comments!
Do share your thoughts on SeaORM and SeaQL ecosystem!

CC @tyt2y3

[tyt2y3]

I've been active in the Laravel community, mostly in official-packages, OctoberCMS and WinterCMS (both based on the laravel/library but not the framework itself).

Welcome!

Until I found SeaORM, which seems to follow some of the Eloquent guidelines I like so much.

I think in general Rust libraries are slightly less opinionated. Instead, would let the user opt-in to certain features.

SELECT * FROM character

The reason this is missing because in Rust we usually want to use a struct to hold the results, and that imply named fields.

insert a raw SQL statement inside a query

I think SeaQuery has some degree of cust expression support, e.g. https://docs.rs/sea-query/latest/sea_query/expr/struct.Expr.html#method.cust_with_values , but definitely not all parts of the syntax tree have that. We can add more though.

complex relationships like many to many with pivot table, polymorphic relationship

SeaORM does have 'many to many with pivot table', but not 'polymorphic relationship' for now.

table name default to the plural name of the type

I think this is a good idea! But do you have any idea how to handle when the plural spelling of the word is different or non-existent in English? e.g. mouse -> mice

models come with two datetime columns created_at and updated_at which are automatically

This has been discussed before (sorry I forgot where). Basically this cannot be done implicitly. We can't (or shouldn't) insert attributes to a struct. We can however attach ON UPDATE CURRENT_TIMESTAMP to updated_at, which is definitely a reasonable default behaviour.

How much welcoming is the maintainer team about public contributions?

Contribution from anyone is definitely welcomed! Actually, since 0.2, many features are proposed or contributed by non-core members, e.g. #105, #142, #252, with various degrees of involvement. We could take on feature proposals, but definitely it will be better in form of code contributions!

The project homepage feel like a "professional crate"

We did spend effort in planning/building SeaORM 'like a product/startup'. We do hope to establish a viable channel of revenue in the future. But for now the project is 100% open source 0% commercial.

All in all, thank you for your enthusiasm! Have fun in the Rust community.

[RomainMazB]

Thanks for these complete replies!

For the plural, before posting this discussion I made some tests with the Inflector crate, and had great results, but I just tried with the mouse use case, and it's not supported yet.
In laravel, mouse>mice is perfectly supported, I could dig into this and make a PR to Inflector to improve it at the same level as the Laravel pluralizer if this little table-name abstraction would be acceptable into Sea-ORM.

I hope to submit a PR soon, I'm currently trying to write good failing-tests to integrate related-saving feature discussed on Discord.