Fix nested field missing sub embedding field

[wdongyu]

Description

Detail description in #909, this pull add empty check before filling in the result

Related Issues

Resolves #909

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[martin-gaievski]

@wdongyu is this PR is work in progress or ready for review? bunch of tests are failing...

[wdongyu]

@martin-gaievski will rerun and fix them... :(

[yuye-aws]

@wdongyu Can you check failing CIs: https://github.com/opensearch-project/neural-search/actions/runs/10987606139/job/30536408453?pr=913?

[wdongyu]

@yuye-aws working in process, I have found the problem and try to reproduce it on my dev box. It's related to the inference text which is null but sent to mlCommonsClientAccessor

[yuye-aws]

Great! Looking forward to see your new commits.

[wdongyu]

In the last commit, I have done:

1. Filter out the null entry in createInferenceListForMapTypeInput, so that we can avoid sending null inference text to mlCommonsClientAccessor.
2. Not filter out the null entry in createInferenceList when sourceValue instanceof List, because if there exists any null entry for List, an IllegalArgumentException will be thrown, I also add a test to confirm that.

private List<String> createInferenceList(Map<String, Object> knnKeyMap) {
   ...
        if (sourceValue instanceof List) {
            texts.addAll(((List<String>) sourceValue));   --> not filter out null entry
        } else if (sourceValue instanceof Map) {
            createInferenceListForMapTypeInput(sourceValue, texts);    --> filter out null entry
        } 
   ...
}

3. Refine two tests, one is to make sure the failed doc count is corrected, another is to make sure the model is uploaded completely.

Please correct me if I am wrong. @martin-gaievski @yuye-aws

[yuye-aws]

Thanks! Will review this PR tomorrow. @martin-gaievski please start the CI workflow.

[yuye-aws]

Nice catch!

[yuye-aws]

@wdongyu Please run ./gradlew spotlessapply to fix linting isssue

[yuye-aws]

@wdongyu It is nice of you to implement high-quality ut and it code. Just need to resolve the following comments.

[vibrantvarun]

Triggered checks. I will review the PR today

[wdongyu]

Resolve all above conversations @yuye-aws @martin-gaievski @vibrantvarun

[yuye-aws]

Good. Taking a look

[yuye-aws]

The code looks good to me. Thanks for the contribution @wdongyu . Can @martin-gaievski or @vibrantvarun start the CI workflow?

[martin-gaievski]

Changes looks good to me, thank you! @wdongyu can you please rebase on main? you need to have changes from #916 in order to fix the CI. Once that's done and CI is green I'll approve and merge this PR.

[vibrantvarun]

Please rebase with main and LGTM, Thanks

[wdongyu]

Rebase done, please start the CI workflow when you are available @martin-gaievski @vibrantvarun.

[yuye-aws]

Rebase done, please start the CI workflow when you are available @martin-gaievski @vibrantvarun.

Thank you! CI is running now. Will approve your PR once the CI gets all passed.

[wdongyu]

Thank you! CI is running now. Will approve your PR once the CI gets all passed.

Bunch of tests are still failing, most of them are NoClassDefFoundError, and one of them is for testAgainstOldCluster. Am I missing somethings? @yuye-aws

[yuye-aws]

Hi @wdongyu ! I do not think you have missed anything. The following error log seems to be a problem from k-NN repo. Might have time to check it tomorrow.

Caused by:
java.lang.ExceptionInInitializerError: Exception java.util.ServiceConfigurationError: org.apache.lucene.codecs.Codec: Provider org.opensearch.knn.index.codec.KNN910Codec.KNN910Codec could not be instantiated [in thread "SUITE-MLCommonsClientAccessorTests-seed#[E12FC70B18037AED]-worker"]
at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:586)
at java.base/java.util.ServiceLoader$ProviderImpl.newInstance(ServiceLoader.java:813)
at java.base/java.util.ServiceLoader$ProviderImpl.get(ServiceLoader.java:729)
at java.base/java.util.ServiceLoader$3.next(ServiceLoader.java:1403)
at org.apache.lucene.util.NamedSPILoader.reload(NamedSPILoader.java:68)
at org.apache.lucene.util.NamedSPILoader.<init>(NamedSPILoader.java:52)
at org.apache.lucene.util.NamedSPILoader.<init>(NamedSPILoader.java:38)
at org.apache.lucene.codecs.Codec$Holder.<clinit>(Codec.java:45)
... 18 more

[yuye-aws]

A PR has been merged into the k-NN repo: opensearch-project/k-NN#2195. Hopefully the k-NN artifact can fix the CI error. I will rerun the CI later.

[martin-gaievski]

You need to wait for #927 to be merged, CI will keep failing until then