Add Keyword Field and Refactor FieldMapper with ParameterizedFieldMapper

[mingshl]

Description

This is an WIP towards approach 1. Adding this PR to further discuss about the format of parsing dot path notation approach for key value pairs in keyword field.

In this PR,
-Added keyword field,
-refactor FieldMapper with ParameterizedFieldMapper
-refactor FieldMapperTest with ParameterizedFieldMapper

Next step,

To be determined to write a new parser to read context and tokenized into key-value pairs
* option 1：catalog.title=Lucene in Action
* option 2: catalog.title=Lucene, catalog.title=in, catalog.title=Action

Issues Resolved

#1018

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.

[lukas-vlcek]

I am running into issues with FlatObjectFieldMapperTests class.

The following is a recreation CLI command.

Navigate to <project_root>/plugins/flat-object folder and run:

../../gradlew ':plugins:flat-object:test' --tests "org.opensearch.flatobject.mapper.FlatObjectFieldMapperTests"

[mingshl]

@lukas-vlcek Thanks for the doc!

I am able to run the plugin now. This Pr now can create the flat-object field type index, upload the document and search on string on the path and also search on the value.

example:

—————————Able to create "flat-object" index

curl -XPUT localhost:9200/test-index --data '{
  "mappings": {
    "dynamic": "strict",
    "properties": {
      "catalog": {
        "type": "flat-object"
      }
    }
  }
} ' -H "Content-Type:Application/json"

—————————Able to load doc to "flat-object" index

curl -XPUT localhost:9200/test-index0/_doc/1 -d '{
  "catalog": "{title: Lucene in Action}"
}' -H "Content-Type:Application/json"

{"_index":"test-index0","_id":"1","_version":3,"result":"updated","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":2,"_primary_term":1}%

———————Able to search and fetch

curl 'localhost:9200/test-index0/_search?pretty' --data '{
  "query": {
    "match": {
      "catalog": "Lucene In Action"
    }
  }
}
' -H "Content-Type:Application/json"

————— Able to search and fetch

curl 'localhost:9200/test-index0/_search?pretty' --data '{
  "query": {
    "match": {
      "catalog": "title"
    }
  }
}
' -H "Content-Type:Application/json"

————— Fetch result

{
  "took" : 755,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 1,
      "relation" : "eq"
    },
    "max_score" : 0.8630463,
    "hits" : [
      {
        "_index" : "test-index0",
        "_id" : "1",
        "_score" : 0.8630463,
        "_source" : {
          "catalog" : "{title: Lucene in Action}"
        }
      }
    ]
  }
}

The next step would work on to parse the json object into a string into "key value" pair format.

[lukas-vlcek]

@mingshl That is nice progress!

• I am currently still trying to figure out why is CI failing. There are couple of reasons but one I am focusing on now is why the FlatObjectQueryBuilder tests fail on missing geo support... (geo shouldn't be relevant at all in this case, so I expect that we need to "instruct" the abstract test cases that we inherit from) let's see if I can figure this out by tomorrow. I will give you an update.

• If you feel like jumping on the JSON processing, go for it 👍

• Please, just a reminder, do not forget about DCO on your commits.

[lukas-vlcek]

Finally, I found the reason why FlatObjectQueryBuilderTests are failing due to "missing" geo_shape type.

The reason is explained here:

https:/opensearch-project/OpenSearch/blob/ac322595eb1838b68e4d9a46cf97cbe56b811aed/test/framework/src/main/java/org/opensearch/test/TestGeoShapeFieldMapperPlugin.java#L44-L47

Simply put, the test will need to depend on TestGeoShapeFieldMapperPlugin plugin for now. It is known testing framework issue, hopefully it will be fixed in the future.

@mingshl I will be pushing fix for this tomorrow.

[mingshl]

lukas-vlcek I made the KeyValueJsonXContentParser in the commit
Enable Search for each subfields so currently now can use the flat-object mapper to search for values for every subfield name and also the subfield value.

            /**
             * KeyValueJsonXContentParser does : 
             * this is "value" only format for each subfield
             * parsedFields will contain {"key_path": "key", "key_value": "value"}
             */

This is the test case that I am using and it's working properly for create index, query filed name, query field value, and update docs, and query again, it's working properly.

Requesting review from @froh

Next step is to enable search for dot path.

Here is the logging that I have for testings:

----create index ---

`curl -XPUT localhost:9200/demo-flattened1 --data '{
  "mappings": {
    "properties": {
      "host": {
        "type": "flat-object"
      }
    }
  }
} ' -H "Content-Type:Application/json"`

{"acknowledged":true,"shards_acknowledged":true,"index":"demo-flattened1"}

---upload doc---

curl -XPUT localhost:9200/demo-flattened1/_doc/1 -d '{
  "message": "[5592:1:0309/123054.737712:ERROR:child_process_sandbox_support_impl_linux.cc(79)] FontService unique font name matching request did not receive a response.",
  "fileset": {
    "name": "syslog"
  },
  "process": {
    "name": "org.gnome.Shell.desktop",
    "pid": 3383
  },
  "@timestamp": "2020-03-09T18:00:54.000+05:30",
  "host": {
    "hostname": "bionic",
    "name": "bionic"
  }
}'  -H "Content-Type:Application/json"

{"_index":"demo-flattened1","_id":"1","_version":1,"result":"created","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":0,"_primary_term":1}%

---check mapping --

"localhost:9200/demo-flattened1/_mapping?pretty=true"
{
  "demo-flattened1" : {
    "mappings" : {
      "properties" : {
        "@timestamp" : {
          "type" : "date"
        },
        "fileset" : {
          "properties" : {
            "name" : {
              "type" : "text",
              "fields" : {
                "keyword" : {
                  "type" : "keyword",
                  "ignore_above" : 256
                }
              }
            }
          }
        },
        "host" : {
          "type" : "flat-object"
        },
        "message" : {
          "type" : "text",
          "fields" : {
            "keyword" : {
              "type" : "keyword",
              "ignore_above" : 256
            }
          }
        },
        "process" : {
          "properties" : {
            "name" : {
              "type" : "text",
              "fields" : {
                "keyword" : {
                  "type" : "keyword",
                  "ignore_above" : 256
                }
              }
            },
            "pid" : {
              "type" : "long"
            }
          }
        }
      }
    }
  }
}

---query on field name and field value ---

mingshl@bcd0741e226d flat-object % curl 'localhost:9200/demo-flattened1/_search?pretty' --data '{
  "query": {
    "match": {
      "host": "name"
    }
  }
}
' -H "Content-Type:Application/json"
{
  "took" : 1,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 1,
      "relation" : "eq"
    },
    "max_score" : 0.39556286,
    "hits" : [
      {
        "_index" : "demo-flattened1",
        "_id" : "1",
        "_score" : 0.39556286,
        "_source" : {
          "message" : "[5592:1:0309/123054.737712:ERROR:child_process_sandbox_support_impl_linux.cc(79)] FontService unique font name matching request did not receive a response.",
          "fileset" : {
            "name" : "syslog"
          },
          "process" : {
            "name" : "org.gnome.Shell.desktop",
            "pid" : 3383
          },
          "@timestamp" : "2020-03-09T18:00:54.000+05:30",
          "host" : {
            "hostname" : "bionic",
            "name" : "bionic"
          }
        }
      }
    ]
  }
}
mingshl@bcd0741e226d flat-object % curl 'localhost:9200/demo-flattened1/_search?pretty' --data '{
  "query": {
    "match": {
      "host": "bionic"
    }
  }
}
' -H "Content-Type:Application/json"
{
  "took" : 1,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 1,
      "relation" : "eq"
    },
    "max_score" : 0.39556286,
    "hits" : [
      {
        "_index" : "demo-flattened1",
        "_id" : "1",
        "_score" : 0.39556286,
        "_source" : {
          "message" : "[5592:1:0309/123054.737712:ERROR:child_process_sandbox_support_impl_linux.cc(79)] FontService unique font name matching request did not receive a response.",
          "fileset" : {
            "name" : "syslog"
          },
          "process" : {
            "name" : "org.gnome.Shell.desktop",
            "pid" : 3383
          },
          "@timestamp" : "2020-03-09T18:00:54.000+05:30",
          "host" : {
            "hostname" : "bionic",
            "name" : "bionic"
          }
        }
      }
    ]
  }
}

-- update doc ---

 curl -XPOST "localhost:9200/demo-flattened1/_update/1" -d'{
    "doc" : {
        "host" : {
          "osVersion": "Bionic Beaver",
          "osArchitecture":"x86_64"
        }
    }
}' -H "Content-Type:Application/json"

--- query on added sub field name --

curl 'localhost:9200/demo-flattened1/_search?pretty' --data '{
  "query": {
    "match": {
      "host": "osArchitecture"
    }
  }
}
' -H "Content-Type:Application/json"

{
  "took" : 2,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 1,
      "relation" : "eq"
    },
    "max_score" : 1.0303539,
    "hits" : [
      {
        "_index" : "demo-flattened1",
        "_id" : "1",
        "_score" : 1.0303539,
        "_source" : {
          "message" : "[5592:1:0309/123054.737712:ERROR:child_process_sandbox_support_impl_linux.cc(79)] FontService unique font name matching request did not receive a response.",
          "fileset" : {
            "name" : "syslog"
          },
          "process" : {
            "name" : "org.gnome.Shell.desktop",
            "pid" : 3383
          },
          "@timestamp" : "2020-03-09T18:00:54.000+05:30",
          "host" : {
            "hostname" : "bionic",
            "name" : "bionic",
            "osVersion" : "Bionic Beaver",
            "osArchitecture" : "x86_64"
          }
        }
      }
    ]
  }
}

[lukas-vlcek]

@mingshl Great! I will have a look tmrw.
BTW, do you think we will need FlatObjectQueryBuilder at all? In other words, do you think the plugin will need to implement SearchPlugin interface? Wouldn't be MapperPlugin interface sufficient?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: 48a1226
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[mingshl]

@lukas-vlcek Made another commit I made for flat multiple nested fields which should flat multiple layers, so in regarding to how many inner layer of nested JSON, it should be handed in the KeyValueJsonXContentParser.

I tested with the following nested Json documents, this is a three levels nested documents:

curl -XPOST "localhost:9200/demo-flattened1/_update/1" -d'{
    "doc" : {
        "host" : {
          "osVersion": {"versionNumber": "one", 
                       "versionName": "Bionic Beaver",
                       "versionContact":{
                          "email": "[email protected]",
                          "first name": "me"
                       }
                       },
          "osArchitecture":"x86_64"
        }
    }
}' -H "Content-Type:Application/json"

{"_index":"demo-flattened1","_id":"1","_version":2,"result":"updated","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":1,"_primary_term":1}%

``

`mingshl@bcd0741e226d OpenSearch-Mingshl-2 % curl -XGET "localhost:9200/demo-flattened1/_search?pretty=true" -d'{
  "query": {
    "term": {
      "host": "[email protected]"
    }
  }
}' -H "Content-Type:Application/json"

{
  "took" : 44,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 1,
      "relation" : "eq"
    },
    "max_score" : 1.0933416,
    "hits" : [
      {
        "_index" : "demo-flattened1",
        "_id" : "1",
        "_score" : 1.0933416,
        "_source" : {
          "message" : "[5592:1:0309/123054.737712:ERROR:child_process_sandbox_support_impl_linux.cc(79)] FontService unique font name matching request did not receive a response.",
          "fileset" : {
            "name" : "syslog"
          },
          "process" : {
            "name" : "org.gnome.Shell.desktop",
            "pid" : 3383
          },
          "@timestamp" : "2020-03-09T18:00:54.000+05:30",
          "host" : {
            "hostname" : "bionic",
            "name" : "bionic",
            "osVersion" : {
              "versionContact" : {
                "first name" : "me",
                "email" : "[email protected]"
              },
              "versionName" : "Bionic Beaver",
              "versionNumber" : "one"
            },
            "osArchitecture" : "x86_64"
          }
        }
      }
    ]
  }
}
 

In the past commits, I can achieve the following functionalities:

1. create an flat-object index with mapping to flatobject type
2. upload Json documents and mapped as string
3. update fields with new subfields, including nested Json subfields.
4. query subfield's field name under field name. for example "field name = subfield's fieldname"
5. query subfield's value with under field name. for example "field name = subfield's value"

But there are a couple things in my list that I didn't go through yet

• 1. query through dot path. example "field name.subfield = subfield's fieldname"
• 2. query with filters
• 3. add normalizers

....

I am still working on the dot.path notation query, so might keep the querybuilder file until that.

the plugin gradle file is updated in the commit Enable flat-object Plugin to run

[froh]

@mingshl wrote:

Requesting review from @froh

I think you meant @msfroh?

[mingshl]

@mingshl wrote:

Requesting review from @froh

I think you meant @msfroh?

yes, thank you!

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: 892edb3
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[lukas-vlcek]

@mingshl I pushed a commit to fix some of the failing tests.

Specifically:

• FlatObjectFieldMapperTests now passes without any failures. Please see how I changed the use of the builder in writeFieldValue method.
• FlatObjectQueryBuilderTests still fails but is providing a lot more reasonable explanations. And these can now be addressed one at a time (we needed this hack, see [BUG] Refactor unnecessary dependency on TestGeoShapeFieldMapperPlugin tests opensearch-project/OpenSearch#6322).

[lukas-vlcek]

@mingshl

I fixed a naming bug with yaml resource folder that was preventing the testcluster to find the yaml tests.

We can create and run real yaml tests now!

You can use ../../gradlew clean :plugins:flat-object:yamlRestTest or you can run/debug them directly from Idea! (right-click on the yamlRestText folder and select Run and then the gradle option).

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: c9a9d22
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: 1b41f9f
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: e4f576b
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: null ❌
• URL:
• CommitID: c0e2337
  Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
  Is the failure a flaky test unrelated to your change?

[lukas-vlcek]

@peterzhuamazon I am trying to understand more about gradle-check.yml. It seems to be the last check that is failing on our PR. Is this check expected to work in this situation (upstream repo-clone/branch PR)? Shall we just remove this file in our case (or rename if that will turn the check off) and only put it back when we move to feature branch in the upstream repo?

Let me add more details here.

• Perhaps the most important aspect is that we are NOT doing PR against upstream, this is PR against the fork itself.
• The closest I could get to the issue is this part of the code: https:/opensearch-project/opensearch-build/blob/main/scripts/gradle/gradle-check.sh#L24-L34
• According to failed job log this line https:/opensearch-project/opensearch-build/blob/main/scripts/gradle/gradle-check.sh#L34 produced the following output: QUEUE_URL null (see the log)
• This means that this curl command did not yield expected response/data. And this is probably expected behaviour of fork, when doing PR to itself...?

[lukas-vlcek]

@mingshl
FYI, according to https://docs.github.com/en/actions/managing-workflow-runs/disabling-and-enabling-a-workflow it is possible to disable individual workflows. I am going to disable https:/mingshl/OpenSearch-Mingshl/actions/workflows/gradle-check.yml

However, if this was the only workflow check that start gradle check or gradle test task then this is counter productive.

cc @peterzhuamazon

[mingshl]

Thanks @peterzhuamazon for the suggestions. Hi @lukas-vlcek since we are not pushing over the upstream, that's fine to remove the gradle check yml file. We will include the gradle check when we are ready to push to main.

[mingshl]

Hi @lukas-vlcek To test the performance of flatObject, I spent some time added a new class FlatObjectMappingBenchmark,

I tested with a document with 100 nested level , each with 10 subfields, in using Dynamic Mapping, it gets exception of fields over 1000. BUT it works pretty nicely with Flat-Object Mapping.

here are some benchmark outcomes:
Benchmark Mode Cnt Score Error Units
FlatObjectMappingBenchmark.CreateDynamicIndex avgt 100 255.615 ± 9.534 ms/op
FlatObjectMappingBenchmark.CreateFlatObjectIndex avgt 100 279.247 ± 9.157 ms/op
FlatObjectMappingBenchmark.indexDynamicMapping avgt 100 365.284 ± 16.736 ms/op
FlatObjectMappingBenchmark.indexFlatObjectMapping avgt 100 361.555 ± 14.835 ms/op
FlatObjectMappingBenchmark.searchDynamicMapping avgt 100 404.177 ± 17.067 ms/op
FlatObjectMappingBenchmark.searchFlatObjectMapping avgt 100 407.120 ± 25.673 ms/op
FlatObjectMappingBenchmark.searchFlatObjectMappingWithOneHundredNestedJSON avgt 100 1145.303 ± 29.177 ms/op

I shall also test some edge case like 999 subfields in the future commits in using DynamicMapping and FlatObjectMapping, please expect them in the coming week.

Thanks @dai-chen for introducing micro-benchmark tool. Requesting review from @dai-chen about this Add FlatObjectMappingBenchmark class.

[peterzhuamazon]

Hi,

I was not aware I was tagged here as well.
But just like @mingshl mentioned I talked with here, that this PR is not send to upstream.
Therefore, the gradle check will not trigger correctly as it is missing key secrets.
Just run gradle check locally for this, and when you send the PR to upstream, it will trigger correct with no issues.

Thanks.

[lukas-vlcek]

@mingshl Just FYI, I have been looking at some options how we could manage more than a single field to store the content to (using different "analyzers") and one particular option I like a lot is using the multi-field field type. The multi-field is already used by several existing field types under the hood, for instance by autocomplete field types – so that one is a nice source of inspiration atm.

[mingshl]

@lukas-vlcek I successfully created two subfields as keyword in the new commit!!!!

The lesson learn I have is that each field would need a fieldmapper, a field type and a field. So I created the ValueFieldMapper and ValueAndPathFieldMapper, and also their field type and fields. It works nicely in the benchmarking class. Please see the new commit in detail

I think we have enough to put up a draft PR to the core. I am about to create a new PR again core as API just as we discuss in the issue.

[mingshl]

After talking to the maintainers, because we are going to release in 2.7, we don't need a feature branch. I got the suggestions that we should start raising a pull request to the main branch. Please see the draft PR 6507 @lukas-vlcek I don't have permission at assign you, maybe you can try assign yourself in the draft PR?

[lukas-vlcek]

@mingshl Looking at new commits now. A real quick note - do you think you can remove binary QueryFlatObjectTests.class file? I am not sure if QueryFlatObjectTests.java file would pass style check (formatting).

[mingshl]

Sure. We don’t need a new querybuilder, I will remove it in today’s commit. From: Lukáš Vlček ***@***.***> Reply-To: mingshl/OpenSearch-Mingshl ***@***.***> Date: Tuesday, March 21, 2023 at 7:02 AM To: mingshl/OpenSearch-Mingshl ***@***.***> Cc: "Liu, Mingshi" ***@***.***>, Mention ***@***.***> Subject: Re: [mingshl/OpenSearch-Mingshl] Add Keyword Field and Refactor FieldMapper with ParameterizedFieldMapper (PR #3) @mingshl<https:/mingshl> Looking at new commits now. A real quick note - do you think you can remove binary QueryFlatObjectTests.class file? I am not sure if QueryFlatObjectTests.java file would pass style check (formatting). — Reply to this email directly, view it on GitHub<#3 (comment)>, or unsubscribe<https:/notifications/unsubscribe-auth/A3BBKSRTP5UTMCPU4AP7ZSDW5GYFRANCNFSM6AAAAAAUWHIDTI>. You are receiving this because you were mentioned.Message ID: ***@***.***>