Add delete if exist method

[bidoubiwa]

State of deleting

As for now, to delete an index, one should first be sure the index does not exist already.

Problem

Re-indexing all documents is often slower than deleting the index, and push the documents on a fresh index.
Meaning that this is an operation that is potentially often used by our users (me for example)

Other Motivations

In milli we will be introduced with swap, meilisearch/transplant#28 (see @curquiza response).

Nontheless:

The swap is great, but nonetheless user may still want that method for tests/security/shutdown. It would avoid that in our e2e tests we have to delete all indexes before running the tests (at least in javascript SDK).
It can be used in a teardown method or a onstart as it cleans the instance.
It would avoid having home-made deleteIfExists function in our code that is not tested.

Solution and Vote

We agreed on the necessity of this function. We still have to agree on a naming.

Examples are done using javascript

All in function name 👀

We provide a new function with an explicit naming

client.index("myindex").deleteIfExist()
client.deleteIndexIfExists("myindex")

Pros

No need to search into references to know available parameters.
Only need to see available functions.

Cons

How longer the method name, how unreadable it becomes.

Optionnal parameter 🎉

We provide a new option object to already existing functions.

client.index("myindex").delete({ ifExists: true })
client.deleteIndex("myindex", { ifExists: true })

Pros

No need to create a new function, also more readable.

Cons

Need to know the existence of the options object and which options are available

Return value.

Boolean

If there was a delete, it would return true. If the index did not exist, and thus none have been deleted, it would return false.

const deleteOccured = client.index('myindex').deleteIfExists() // true of false

Index instance

the method would return the instance in itself to let the user chain its actions without having to create a new index object.

Example:

const index = client.index('myindex').deleteIfExists()
index.addDocuments(myDocuments) 

Without this option we have to do this:

client.index('myindex').deleteIfExists()
client.index('myindex').addDocuments(myDocuments)

TODO

• meilisearch-dart
• meilisearch-dotnet
• meilisearch-go
• meilisearch-java
• meilisearch-js
• meilisearch-php
• meilisearch-python (Adding delete if exists methods meilisearch-python#268)
• meilisearch-ruby
• meilisearch-rust
• meilisearch-swift

[curquiza]

Hello! Not sure to get it! Can you detail the context in which you would use this method?

[bidoubiwa]

for example on docs-scrapper. Before adding the new document you would typically delete the index before re-creating one add the settings and the documents. When updating databases I often go with the same solution, first delete if it existed, then add documents and settings.

[curquiza]

Oh you want to add the deleteIndexIfExist() to avoid doing the try/catch to check if the index already exists!

Ok, this would be a temporary solution because MeilISearch does not handle the swap index yet. We have to think if it's really worth taking time to add it to every SDK and then remove it before the release of the swap index.

[bidoubiwa]

This is something I used a lot in different databases:

POSTGRES/ MySQL/ most sql db's

deleting

DROP TABLE IF EXISTS mytable:

database delete

DROP DATABASE [IF EXISTS] database_name;

Mongo

returns false but not an error when attempting to delete a collection that exists.

Elasticsearch

Delete a document if it exist

{
"query": {
  "exists": {
    "field": "name"
  }
 }
}

and then remove it before the release of the swap index

Is this related to this issue https://github.com/meilisearch/transplant/issues/28 ?

[curquiza]

Is this related to this issue meilisearch/transplant#28 ?

Yes 🙂

[bidoubiwa]

The swap is great, but I think nonetheless user may still want that method for tests/security/shutdown. It would avoid that in our e2e tests we have to delete all indexes before running the tests (at least in javascript SDK).
It can be used in ateardown method or a onstart as it cleans the instance.

[bidoubiwa]

@curquiza @alallema the issue is waiting for a vote!

[curquiza]

The winner is:

client.index("myindex").deleteIfExist()
client.deleteIndexIfExists("myindex")

I pass it as ready to implement

[sanders41]

I have this ready for the Python client, but have one question. Do you want the methods to return anything?

[bidoubiwa]

It could return true or false based on the fact that something has been deleted or not. Thats what the drop function of mongo does:

Extract :

So now that there's no longer an artists collection in our database, let's try to drop it and see what message we get:

db.artists.drop()

Resulting message:

false

It returned false because the collection doesn't exist.

[sanders41]

I came across what I think could be an issue with this variant client.index("myindex").deleteIfExist() and wanted to see what you all think. Frequently indexes are created and used like:

index = client.index('myindex')
index.addDocuments(myDocuments)

However this is an issue with deleteIfExists

index = client.index('myindex').deleteIfExists()
index.addDocuments(myDocuments)  // Can't do this because this isn't and Index object

The motivation behind adding this is so the index can be removed before doing more work with the index so I don't think you would want to do this as it seems redundant correct?

client.index('myindex').deleteIfExists()
index = client.index('myindex')
index.addDocuments(myDocuments)

If you all think this is an issue I think it would apply to all SDKs. One possible solution is deleteIfExists could return an Index object.

index = client.index('myindex').deleteIfExists()
index.addDocuments(myDocuments)  // Now this works because deleteIfExists() returned an instance of Index

Or maybe most people would do it like this anyways so this isn't an issue at all?

client.index('myindex').deleteIfExists()
client.index('myindex').addDocuments(myDocuments)

client.deleteIndexIfExists("myindex") doesn't have this same issue and could return true or false without issue since this would not be expected to return an Index instance.

[bidoubiwa]

It is a very interesting suggestion. Nonetheless, since most of our methods do not return an instance of index, we are not really into a chainable-method strategy (not sure if that's the term tell me if it's not clear).

I guess in such a strategy we would be able to do things like (the waitFor does not exist but helps the example):

client
    .index('myindex')
    .deleteIfExists()
    .addDocuments(data, { waitFor: true })
    .updateSettings(settings, { waitFor: true })

Today I think only, get, update and create index methods returns an instance.
So creating a new line for each action is already something we do and are used to.

But it raises a very interesting question as I didn't thought about having chainable methods.

[sanders41]

Yes, as far as I am aware if client.index('myindex').deleteIfExists() would be the only chained strategy like this that would return the Index. It's fine for me for this one to also return true/false, just as I was setting that up it got me thinking about this so I wanted to make sure I was doing the correct thing here.

[bidoubiwa]

I think client.index('myindex').create() should do the same? I think that true/false for now is more consistent with the other methods.

[sanders41]

I know for sure client.index('myindex').create() returns an Index in the python client. In the other clients I would have to check to be sure. But yes, create() is more of a "special case" and returning true/false from the delete would be more like the others.

[bidoubiwa]

I'll confirm with @curquiza @alallema and @mdubus to have an official decision on the matter :)

[bidoubiwa]

Hello @sanders41, we agreed that returning true/false is the best solution.

A few other arguments have been raised:

• No communication on what happened in the return value
• Confusion between creating and deleting an index in MeiliSearch, and creating and deleting an index instance in the code.

So we stay with the idea that if something was deleted we return true, if not, we return false

[curquiza]

The issues are opened in the concerned repositories

[curquiza]

According to #157 (comment), we decided to remove this method from the SDKs. I'm closing this issue

RIP deleteIfExists 😢