Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error with deleting multiple documents #6

Closed
manavo opened this issue Aug 11, 2021 · 5 comments
Closed

Error with deleting multiple documents #6

manavo opened this issue Aug 11, 2021 · 5 comments

Comments

@manavo
Copy link
Contributor

manavo commented Aug 11, 2021

Description

Bulk deleting isn't working for me, I get an error like:

Typesense\Exceptions\ObjectNotFound: Could not find a filter field named `feedback.id` in the schema

My collection schema is:

[
  {
    "created_at":1602963950,
    "default_sorting_field":"last_reply_time",
    "fields":[
      {
        "facet":false,
        "index":true,
        "name":"application_id",
        "optional":false,
        "type":"int64"
      },
      {
        "facet":false,
        "index":true,
        "name":"message",
        "optional":true,
        "type":"string"
      },
      {
        "facet":false,
        "index":true,
        "name":"email",
        "optional":true,
        "type":"string"
      },
      {
        "facet":false,
        "index":true,
        "name":"last_reply_time",
        "optional":false,
        "type":"int64"
      },
      {
        "facet":false,
        "index":true,
        "name":"properties",
        "optional":true,
        "type":"string[]"
      },
      {
        "facet":false,
        "index":true,
        "name":"member_name",
        "optional":true,
        "type":"string"
      },
      {
        "facet":false,
        "index":true,
        "name":"member_email",
        "optional":true,
        "type":"string"
      },
      {
        "facet":false,
        "index":true,
        "name":"reply_messages",
        "optional":true,
        "type":"string[]"
      },
      {
        "facet":false,
        "index":true,
        "name":"reply_emails",
        "optional":true,
        "type":"string[]"
      }
    ],
    "name":"feedback",
    "num_documents":2459169,
    "num_memory_shards":4
  }
]

The issue might be that I haven't explicitly defined id as a field?

For what it's worth, even the example in the readme doesn't indicate that we should have id as a field.

Metadata

Typsense Version: 0.17.0 and 0.21.0 both tested

OS: Ubuntu 20.04.2 LTS

@hi019
Copy link
Collaborator

hi019 commented Aug 11, 2021

In order to link Typesense Documents to Laravel Models, the package uses getScoutKeyName and getScoutKey (more info). The first problem I see with this is that getScoutKeyName includes the table name, ex table_name.primary_key. In the delete method, we assume that getScoutKeyName is only primary_key. In my codebase, I've overriden the getScoutKeyName for an unrelated reason, prior to this package's development, so that's probably why I didn't notice the problem.

With that said, the error you're getting is due to the delete method trying to query the affected documents based on the getScoutKeyName field which as mentioned above is invalid.

Looking at your schema, maybe getScoutKeyName should be email? Try adding these to your model:

    public function getScoutKey()
    {
        return $this->email;
    }

    public function getScoutKeyName()
    {
        return 'email';
    }

@manavo
Copy link
Contributor Author

manavo commented Aug 12, 2021

I see what you're saying, but this library also needs to work with the default configuration of Scout and models, which isn't to override the Scout Key.

So yes what you're saying would in theory work (although email would not be the correct primary key for my use case).

But we also need to make it work with the default id of the model being the id in Typesense.

@manavo manavo mentioned this issue Aug 12, 2021
1 task
@manavo
Copy link
Contributor Author

manavo commented Aug 12, 2021

I've proposed #9 to fix this, and go back to deleting one document at a time in a loop.

As much as I agree that it is a good optimisation to do if deleting multiple documents, it is more important for the library to work with normal defaults, and then try and optimise it.

Just my opinion, at it at least allows a first proper version of the library to be tagged and released. This can then be further optimised if there is a way for it to work with the defaults.

@hi019
Copy link
Collaborator

hi019 commented Aug 12, 2021

I agree that this is a bug - perhaps I wasn't clear in my previous reply. Anyway, thanks for the contributions and I will test your PR later today.

@manavo
Copy link
Contributor Author

manavo commented Aug 12, 2021

Thanks @hi019 🙂

@hi019 hi019 closed this as completed Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants