Support parsing documents with flattened value

[qianheng-aws]

Description

It supports parsing flattened document value by populating the ExprTupleValue recursively, so we can access the nested fields as well as the root fields as before.

Related Issues

Resolves #3477

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• 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.

[penghuo]

add UT

[qianheng-aws]

Done

[penghuo]

add UT

[qianheng-aws]

Done

[penghuo]

yamlTest is more readable?

[LantaoJin]

Or add this test data to resource folder and load it. It is not as simple as test index, adding the mapping and data to resource could be readable and reusable.

[qianheng-aws]

@penghuo If we want to test this case w/wo Calcite enabled, in yamlTest, is there a convenient way to implement this with less code duplication?

In Java code, we can use inheritance to make it simple like the current code. But in yaml, we may need to duplicate the process with extra settings.

I've thought of using yamlTest but didn't have much information whether it can do such thing conveniently, so I keep using IT to do the verification.

[qianheng-aws]

Added a yaml test with only Calcite enabled.

[penghuo]

yamlTest support index any doc.

[LantaoJin]

can this be deleted now?

[penghuo]

what if ExprIpValue mergeTo ExprIntValue?

[LantaoJin]

Seems it is replace. Could you comment in code what's the purpose of adding this mergeTo interface?

[qianheng-aws]

As said in the doc, it should override the base under the current implementation. Such case will only happen when there is duplication value for a field.

1. In normal cases, only ExprTupleValue::mergeTo should be invoked. Cases like:

{"log.json.time": 100, "log.json": { "status": "SUCCESS" }}

For "log.json.time": 100, it will be finally populated into an ExprValue ExprTupleValue(Map.of(\"log\": ExprTupleValue(Map.of({\"json\": ExprTupleValue(Map.of({\"time\": 100}))}))

For "log.json": { "status": "SUCCESS" }, it will be parsed into path: log.json, value: ExprTupleValue(Map.of({\"status\": SUCCESS}))

So after populating the first field, there is key conflict when populating the second field. In this case, we need to invoke ExprTupleValue::mergeTo` to merge 2 ExprTupleValue deeply.

2. In some edge cases where there is conflict value in customer's document, like:

{"log.json.status": "FAILED", "log.json": { "status": "SUCCESS" }}

There is value conflict for the field log.json.status, so I want the value added most recently to override any previously added values, and ExprValue::megeTo will be invoked.

But, in json, since the sequence of the fields in the same level are unstable actually, we cannot assert which value will be put firstly. Maybe throw exception to tell customer there is incompatible value conflict will also be an option. What's your opinion on this edge case? @LantaoJin @penghuo

[LantaoJin]

Maybe throw exception to tell customer there is incompatible value conflict will also be an option.

Do you mean throw exception in ppl querying or data ingesting? Data ingesting seems out of the scope here.

[LantaoJin]

IMO, if there are conflict fields. Display the any of them would be all fine as long as the field populated out is fixed. For example:

{"id": 1, "log.json.status": "FAILED", "log.json": { "status": "SUCCESS" }}
{"id": 2, "log.json.status": "FAILED", "log.json": { "status": "SUCCESS" }}
{"id": 3, "log.json.status": "FAILED", "log.json": { "status": "SUCCESS" }}

PPL source=t | fields id, log.json.status return

1, FAILED
2, FAILED
3, FAILED

or

1, SUCCESS
2, SUCCESS
3, SUCCESS

are all acceptable. This could be treated as data quality issue.

[LantaoJin]

Could you add a test for key conflict case?

[qianheng-aws]

The problem is value conflict case will flakey because fields order are not ensured in json. We cannot assert which value will be put in the end.

[qianheng-aws]

Added in the UT of testPopulateValueRecursive, where I can control the populate order

[penghuo]

Maybe throw exception to tell customer there is incompatible value conflict will also be an option. What's your opinion on this edge case?

do not throw exception, current approach looks good. OpenSearch also return the LATEST field.

###
POST {{baseUrl}}/test00001/_doc
Content-Type: application/x-ndjson

{
    "log.json.status": "failed1",
    "log.json": {
      "status": "failed2"
    },
    "log": {
      "json": {
        "status": "failed3"
      }
    },
}

###
GET {{baseUrl}}/test00001/_search
Content-Type: application/x-ndjson

{
    "fields": [
        "log.json.status"
    ],
    "_source": false
}

### Return 
        "fields": {
          "log.json.status": [
            "failed3"
          ]
        }

[penghuo]

what case is coverred by mergeTo, same field with different value?

[qianheng-aws]

See the comment [here](#3577 (comment))

[LantaoJin]

Or add this test data to resource folder and load it. It is not as simple as test index, adding the mapping and data to resource could be readable and reusable.

[LantaoJin]

Seems it is replace. Could you comment in code what's the purpose of adding this mergeTo interface?

[LantaoJin]

@qianheng-aws could you change this log level from warn to debug in this PR? This PR will be merge to 3.0.0 GA
This warning log here will write out thousands of unnecessary logs when the data is a nested type.

sql/opensearch/src/main/java/org/opensearch/sql/opensearch/util/JdbcOpenSearchDataTypeConvertor.java

Line 128 in 47e6a2a

LOG.warn(

[LantaoJin]

ping @penghuo @dai-chen

[penghuo]

Maybe throw exception to tell customer there is incompatible value conflict will also be an option. What's your opinion on this edge case?

do not throw exception, current approach looks good. OpenSearch also return the LATEST field.

###
POST {{baseUrl}}/test00001/_doc
Content-Type: application/x-ndjson

{
    "log.json.status": "failed1",
    "log.json": {
      "status": "failed2"
    },
    "log": {
      "json": {
        "status": "failed3"
      }
    },
}

###
GET {{baseUrl}}/test00001/_search
Content-Type: application/x-ndjson

{
    "fields": [
        "log.json.status"
    ],
    "_source": false
}

### Return 
        "fields": {
          "log.json.status": [
            "failed3"
          ]
        }