fixed misleading error message for max_fields_limit_exceeded

[CodeMan62]

Pull Request

Related issue

Fixes #5508

What does this PR do?

Fixes misleading error message for max_fields_limit_exceeded

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

[ManyTheFish]

Hello @CodeMan62, Thank you for this PR. Can you fix the failing test? In this case, it is quite simple:

1. run cargo insta test -- --test-threads=4 to run the tests
2. run cargo insta review to review all the modified snapshots

[CodeMan62]

@ManyTheFish Done

[ManyTheFish]

Hello @CodeMan62, The new error message is better than the previous one. 👍 However, the section Suggestion for the new error message of the related issue suggests a more explicit message with the previous count of unique fields, the number of additional fields, and some notes.

Could you please build a message like this? Don't hesitate to ask if you miss any information!

[CodeMan62]

Hey @ManyTheFish let me see what i can do here

[CodeMan62]

Hey @ManyTheFish I am here with a silly question UserError::AttributeLimitReached is used in so many function so I am completely confused here can you just tell me that how I will use this in this function If this is my updated AttributedLimitReached updated AttributedLimitReached

AttributeLimitReached{
        document_id: String,
        new_field_count: usize,
        no_of_existing_fields: usize,
        max: u32, 
    },

function to use in

fn create_fields_mapping(
    index_field_map: &mut FieldIdMapWithMetadata,
    batch_field_map: &DocumentsBatchIndex,
) -> Result<HashMap<FieldId, FieldId>> {
    batch_field_map
        .iter()
        // we sort by id here to ensure a deterministic mapping of the fields, that preserves
        // the original ordering.
        .sorted_by_key(|(&id, _)| id)
        .map(|(field, name)| match index_field_map.id(name) {
            Some(id) => Ok((*field, id)),
            None => index_field_map
                .insert(name)
                .ok_or(Error::UserError(UserError::AttributeLimitReached))
                .map(|id| (*field, id)),
        })
        .collect()
}

this will make this pr complete faster thanks :pray:

[ManyTheFish]

Hey @CodeMan62, Indeed, the issue is trickier than it seems. You have several different cases where AttributeLimitReached is returned:

1. When extracting document fields
2. When extracting searchable attributes
3. When extracting faceted attributes
4. When extracting geo fields
5. When extracting vector fields

Most of the time, you can access the document ID, but in the example you provided, you can't because it's a complete batch that we are processing. I want you to know that this code will be removed in the future because we are refactoring this part so that you could have an Option<String> for the document_id and fill it when you can. For the counters, you could refactor a bit of the code to:

1. Count the initial fields.
2. Collect all the refused fields in a vector before returning the error

To ease the implementation and see how much information we are missing, I suggest putting everything optional in your structure and filling it out as much as possible. We will remove the useless Options after.

Then, for the max value, don't store it, it's a constant value for now, so I suggest doing what I said before: https://github.com/meilisearch/meilisearch/pull/5522#discussion_r2065798808

[CodeMan62]

Thanks for clarification, will do it asap

[CodeMan62]

@ManyTheFish one think i want to point out is I will not put everything optional I will try to debug as much as I can and try to refactor the code

[CodeMan62]

Hey @ManyTheFish can you please check my latest commit and tell me if the modification I done in primary_key.rs file if it is good to go then just tell me so I can finish the work of remaining

[CodeMan62]

@ManyTheFish I request you to stop the CI until PR get's ready and also see my latest commit I found a solution for our warning that we were getting

[CodeMan62]

@ManyTheFish all the tests are passing everything is done let me know if anything left?

[irevoire]

Hey @CodeMan62 you must still fix clippy 😬

[CodeMan62]

Hey @CodeMan62 you must still fix clippy 😬

Hey @irevoire clippy fixed this time

[CodeMan62]

@ManyTheFish review if it looks fine then let me just do cargo insta review and then this PR can good to go

[ManyTheFish]

Hey @CodeMan62,

it seem to remain two failing tests:

• error_document_field_limit_reached_in_one_document (crates/meilisearch/tests/documents/add_documents.rs:1525)
• error_document_field_limit_reached_over_multiple_documents (crates/meilisearch/tests/documents/add_documents.rs:1608)

rust Fmt is not happy:

Diff in /home/runner/work/meilisearch/meilisearch/crates/meilisearch-types/src/error.rs:413:
                     UserError::InvalidStoreFile => Code::InvalidStoreFile,
                     UserError::NoSpaceLeftOnDevice => Code::NoSpaceLeftOnDevice,
                     UserError::MaxDatabaseSizeReached => Code::DatabaseSizeLimitReached,
-                    UserError::AttributeLimitReached{ .. } => Code::MaxFieldsLimitExceeded,
+                    UserError::AttributeLimitReached { .. } => Code::MaxFieldsLimitExceeded,
                     UserError::InvalidFilter(_) => Code::InvalidSearchFilter,
                     UserError::InvalidFilterExpression(..) => Code::InvalidSearchFilter,
                     UserError::FilterOperatorNotAllowed { .. } => Code::InvalidSearchFilter,

[CodeMan62]

yes that is what i asking if impl. is fine then i just fix the test and fmt let me do both now

[CodeMan62]

@ManyTheFish everything is passing i think it is good to go now

[CodeMan62]

Not making it draft the this work will be complete by Monday

[CodeMan62]

@ManyTheFish can you help me here adress what is wrong??