insert_many failing if the models iterator is empty

[horacimacias]

Description

I'm executing an insert_many operations where the iterator is the result of mapping/filtering.
Sometimes, as a result of the mapping/filtering, the resulting iterator may be empty.
In this case, the insert_many operation fails; I'd expect this to succeed and the database not being hit at all as there is nothing to do.
I increased the logs and I saw the following:

INSERT INTO
  "rules"
VALUES
  (DEFAULT) RETURNING "id"
..... Db { source: Query("error returned from database: null value in column \"id\" violates not-null constraint") } 

the entity/struct is the following:

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "rules")]
pub struct Model {
    #[sea_orm(primary_key, auto_increment = false)]
    pub id: Uuid,
    pub rule_set: String,
    pub created: NaiveDateTime,
    pub version: i16,
    #[sea_orm(column_type = "JsonBinary")]
    pub rule: Value,
}

I'm using yugabyte for this which should be using postgres.

Steps to Reproduce

1. Execute an insert_many operations where the iterator is empty
2. Confirm success/failure

Expected Behavior

I'd expect this to succeed and do nothing on the database.

Actual Behavior

Operation fails and complains about null ìd` which is the primary key in my model.

Reproduces How Often

If the iterator is empty, always.

Versions

│   │   ├── sea-orm v0.9.0
│   │   │   ├── sea-orm-macros v0.9.0 (proc-macro)
│   │   │   ├── sea-query v0.26.0
│   │   │   │   ├── sea-query-derive v0.2.0 (proc-macro)
│   │   │   │   ├── sea-query-driver v0.2.0 (proc-macro)
│   │   │   ├── sea-strum v0.23.0
│   │   │   │   └── sea-strum_macros v0.23.0 (proc-macro)
│   │   ├── sea-orm-migration v0.9.0
│   │   │   ├── sea-orm v0.9.0 (*)
│   │   │   ├── sea-orm-cli v0.9.1
│   │   │   │   ├── sea-schema v0.9.2
│   │   │   │   │   ├── sea-query v0.26.0 (*)
│   │   │   │   │   └── sea-schema-derive v0.1.0 (proc-macro)
│   │   │   ├── sea-schema v0.9.2 (*)
│   │   ├── sea-schema v0.9.2 (*)

Additional Information

[horacimacias]

...as a workaround, I know I can collect() the iterator and check if it's empty, but if possible I'd like to avoid it as often the iterator is not empty and I don't want to have to collect always.

[billy1624]

Hey @horacimacias, thanks for catching this "logical error". This has to be checked on run-time. Panicking is definitely not a viable option. Perhaps, throwing a DbErr::Exec("Insert many without any values").

What do you think?

[horacimacias]

Thanks.
I don't think it's panicking, but it is resulting on an error for sure. I'm just starting with sea-orm so I was a bit confused by this.

I'm not familiar at all with sea-orm's internals but, isn't there anything that can be checked (e.g. when we're building the sql statement to be sent to db or when we're processing the entity) to see if after consuming/traversing the iterator the entity wasn't added any items, and in this case bypass the operation completely?

so, checking this at run-time but doing it in sea-orm and not on the code using it?

[tyt2y3]

I think insert many without values should be no-op. Or is it because that it might be valid due to INSERT DEFAULT?

[billy1624]

Or is it because that it might be valid due to INSERT DEFAULT?

This is correct. Insert many without value will result in a insert query with default value. i.e.

INSERT INTO
  "rules"
VALUES
  (DEFAULT) RETURNING "id"

[billy1624]

I'm not familiar at all with sea-orm's internals but, isn't there anything that can be checked (e.g. when we're building the sql statement to be sent to db or when we're processing the entity) to see if after consuming/traversing the iterator the entity wasn't added any items, and in this case bypass the operation completely?

I think no-op (skip insert operation) under the hood and without throwing any error is bad. We should throw an error explicitly.

[horacimacias]

sorry, why would a no-op be a bad thing here?

Whenever we're looping over an Iterator that happens to be empty, the loop just does nothing.

    let items : Vec<String> = vec![];
    for i in items.iter(){
        println!("handling {i}");
    }

this will print nothing, and it's fine.
Why would throwing an error be any better here?

insert_many is taking an IntoIterator and it is receiving an empty but perfectly valid one.
When would we ever want insert_many to do any work if it has been passed an empty IntoIterator?

Also regarding INSERT DEFAULT, can somebody provide an example of when we would want to use this and which rust code would be using for this?

sorry if I'm missing some fundamentals of how sea-orm works.

[tyt2y3]

To SeaORM's concern, insert many without a model should be no-op.

In SeaQuery, insert many with default but no value could be a valid operation, but that should not affect the semantic here.

[MartinKavik]

Hi, I've just got bitten by the described empty iterator handling, too.
My use-case for context:

1. Model:

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "post_facebook_page")]
pub struct Model {
    #[sea_orm(primary_key)]
    pub post_id: Uuid,
    #[sea_orm(primary_key)]
    pub facebook_page_id: String, 
}

2. Failing code inside a transaction block:

entity::post_facebook_page::Entity::insert_many(facebook_pages_to_add).exec(db).await?;

3. The error:

Query Error: error returned from database: null value in column "post_id" of relation "post_facebook_page" violates not-null constraint

4. The code with the if workaround:

let facebook_pages_to_add = facebook_pages_to_add.collect::<Vec<_>>();
if not(facebook_pages_to_add.is_empty()) {
    entity::post_facebook_page::Entity::insert_many(facebook_pages_to_add).exec(db).await?;
}

5. The error mentioned above is surprising, breaks the Rust Model type because post_id is not Option<_> and crashes the app in runtime.
6. I would expect no DB request at all in the case of empty iterator.
7. Tested with sea-orm v. 0.9.2.

Thank you guys, very useful library :)

[Diwakar-Gupta]

@billy1624 @tyt2y3 this issue is still in Triage so asking if any fix to this is decided.
This issue could be related #1407

[tyt2y3]

I am more inclined to make it a no-op, but we need to figure out if there is any edge case.

[github-actions]

🎉 Released In 0.12.1 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.