feat(elasticsearch): advanced query, identity autocomplete, exact match weight #7354

david-leifker · 2023-02-16T16:48:29Z

Restore advanced field level queries using /q for backwards compatibility
Enable autocomplete on corp users/groups
Documentation updates around advanced queries
Add additional weight to exact vs prefix match

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

…-tests

david-leifker · 2023-02-16T23:18:17Z

Need to look at that search test closer. Probably needs addressing.

xiphl · 2023-02-17T01:50:48Z

Thanks for updating the search guide! We specifically covered the ways to search as part of our on boarding for new users, and it will be very helpful to have examples

szalai1 · 2023-02-19T18:48:31Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

    return finalQuery.should().size() > 0 ? Optional.of(finalQuery) : Optional.empty();
  }

-  private static QueryBuilder getPhraseQuery(@Nonnull EntitySpec entitySpec, String query) {


jjoyce0510 · 2023-02-20T18:42:49Z

docs/how/search.md

-BrowsePath is stored as a complete string, for instance ```/datasets/prod/hive/SampleKafkaDataset```, hence the need for wildcards on both ends of the term to return a result. 
+If you want to:
+
+- Exact match on term or phrase


Thank you!!!

jjoyce0510 · 2023-02-20T18:43:52Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

@@ -32,6 +32,9 @@


 public class SearchQueryBuilder {
+
+  public static final String STRUCTURED_QUERY_PREFIX = "\\\\/q ";
+  public static final float EXACT_MATCH_BOOST_FACTOR = 10.0f;


Exact match on ANY field?

It's a multiplier, so the annotation * this factor.

jjoyce0510 · 2023-02-20T18:46:04Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

+    BoolQueryBuilder finalQuery = QueryBuilders.boolQuery();
+    if (query.contains("\"")) {
+      finalQuery.should(QueryBuilders.termQuery("urn", query.replaceAll("\"", ""))
+              .boost(Float.parseFloat((String) PRIMARY_URN_SEARCH_PROPERTIES.get("boostScore")) * EXACT_MATCH_BOOST_FACTOR)


Wait wait so this on occurs if the query contains " " quotations (at least one) and only applies to the urn field?

Is that really what we want? My experience is that exact match on NAME fields is not very good, which may or may not be in the URN at all.

Also, do users really want to have to put quotes in order to do exact match? If I search for a Table with the exact name I expect to see it - quotes or no quotes.

The urn field is not part of any of the models' search specs. It is always treated as an addition added everywhere. This would only be used in a quoted urn, exact match. Below is where we handle the other fields.
More important for the non-urn field, the quotes become important when considering synonyms, stemming, and other tokenization steps should not be considered. Fulltext search is not the same as a query where exact matches are implicit, both the field and the search terms are manipulated for getting relevant results over exact match.

jjoyce0510 · 2023-02-20T18:47:34Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

+              finalQuery.should(QueryBuilders.matchPhrasePrefixQuery(fieldSpec.getFieldName() + ".delimited", query)
+                      .boost((float) fieldSpec.getBoostScore())
+                      .queryName(fieldSpec.getFieldName())); // less than exact
+              if (query.contains("\"")) {


This seems like kinda opaque functionality.

If the user provides quotes for a field, we automatically hit the keyword index?

See comment above.

jjoyce0510 · 2023-02-20T18:48:03Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

+                      .queryName(fieldSpec.getFieldName())); // less than exact
+              if (query.contains("\"")) {
+                finalQuery.should(QueryBuilders.termQuery(fieldSpec.getFieldName() + ".keyword", query.replaceAll("\"", ""))
+                        .boost(Float.parseFloat((String) PRIMARY_URN_SEARCH_PROPERTIES.get("boostScore")) * EXACT_MATCH_BOOST_FACTOR)


A bit confused, why are we using PRIMARY_URN_SEARCH_PROPERTIES here?

Also, can we factor out the constants?

Err, this should. not be based on urn properties. Fixing.

jjoyce0510 · 2023-02-20T18:48:56Z

...c/main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java

+                      .queryName(fieldSpec.getFieldName())); // less than exact
+              if (query.contains("\"")) {
+                finalQuery.should(QueryBuilders.termQuery(fieldSpec.getFieldName() + ".keyword", query.replaceAll("\"", ""))
+                        .boost(Float.parseFloat((String) PRIMARY_URN_SEARCH_PROPERTIES.get("boostScore")) * EXACT_MATCH_BOOST_FACTOR)


So again expressing what I mentioned above, boosting cases where there are quotations in the query string I don't think will be sufficient for what users expect. They expect that searching for a name of a Table by exact string will find that table, with or without quotations

I'll remove the quote conditional

jjoyce0510 · 2023-02-20T18:49:34Z

...main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchRequestHandler.java

-    highlightBuilder.field("urn.delimited");
+    _defaultQueryFieldNames.stream()
+            .flatMap(fieldName -> Stream.of(fieldName, fieldName + ".*")).distinct()
+            .forEach(highlightBuilder::field);


Did we verify the column name highlighting still works after this?

jjoyce0510 · 2023-02-20T18:50:12Z

...main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchRequestHandler.java

-    _defaultQueryFieldNames.forEach(fieldName -> highlightBuilder
-            .field(fieldName)
-            .field(fieldName + ".*"));
-    highlightBuilder.field("urn.delimited");


Is removing this safe? Is there history showing why we added this initially?

jjoyce0510 · 2023-02-20T18:51:14Z

...main/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchRequestHandler.java

@@ -288,6 +293,12 @@ private List<MatchedField> extractMatchedFields(@Nonnull Map<String, HighlightFi
        highlightedFieldNamesAndValues.get(fieldName.get()).add(fieldValue.string());
      }
    }
+    // fallback matched query, non-analyzed field
+    for (String queryName : hit.getMatchedQueries()) {


What is the purpose of this change? I'm not familiar enough

For exact match, keyword fields + term queries are not analyzed, therefore they do not return highlighted fields. This is to populate which field is a matching when the field is unanalyzed. Let me see if I can populate the value. The queries I've seen that hit this were only returning the urn, however if the hit does have the matched field, it could be used to fill the value in.

jjoyce0510 · 2023-02-20T18:51:39Z

...rc/test/java/com/linkedin/metadata/search/elasticsearch/fixtures/SampleDataFixtureTests.java

+    }
+
+    @Test
+    public void testPrefixVsExact() {


awesome tests!

jjoyce0510 · 2023-02-20T18:53:25Z

metadata-models/src/main/pegasus/com/linkedin/identity/CorpGroupInfo.pdl

@@ -20,6 +20,7 @@ record CorpGroupInfo {
  @Searchable = {
    "fieldType": "TEXT_PARTIAL"
    "queryByDefault": true,
+    "enableAutocomplete": true,


What this previously enabled and now it is removed? Curious about the reasoning for including this? Or did a customer ask for it?

You're saying this was enabled previously and removed? I think it is natural to search for the display name of the group. I added this specifically because a customer was running a search and I'd suggested autocomplete for users/groups. I looked to write a test for this (in this pr in fact) and it didn't work because of missing annotations.

jjoyce0510 · 2023-02-20T18:53:38Z

metadata-models/src/main/pegasus/com/linkedin/identity/CorpUserInfo.pdl

@@ -90,6 +91,7 @@ record CorpUserInfo includes CustomProperties {
  @Searchable = {
    "fieldType": "TEXT_PARTIAL",
    "queryByDefault": true,
+    "enableAutocomplete": true,


Was this removed? I recall we used to have this

I didn't remove it, so makes sense to add it, especially given that I am suggesting enabling autocomplete for users/groups. Also I added tests for autocomplete on users/groups to ensure it works as expected.

jjoyce0510 · 2023-02-20T18:55:18Z

metadata-models/src/main/pegasus/com/linkedin/metadata/key/CorpGroupKey.pdl

@@ -11,7 +11,10 @@ record CorpGroupKey {
  * The URL-encoded name of the AD/LDAP group. Serves as a globally unique identifier within DataHub.
  */
  @Searchable = {
-    "fieldType": "TEXT_PARTIAL"
+    "fieldType": "TEXT_PARTIAL",
+    "queryByDefault": true,


Nice, thanks!

…ch weight (datahub-project#7354)

…ch weight (#7354)

david-leifker and others added 7 commits February 11, 2023 11:27

gradle(development): add additional commands for development

fcacc60

Merge branch 'master' into debugging-helpers

5f36408

wip

017dc6c

development(docs): Update documentation around development/debugging

de13bda

Merge branch 'debugging-helpers' into adv-search-docs-and-tests

d9804c5

Tests, user/group autocomplete

bfe2b44

Merge remote-tracking branch 'origin/master' into adv-search-docs-and…

dad431b

…-tests

david-leifker requested a review from RyanHolstien February 16, 2023 16:48

github-actions bot added docs Issues and Improvements to docs product PR or Issue related to the DataHub UI/UX labels Feb 16, 2023

Merge branch 'master' into adv-search-docs-and-tests

e362fff

david-leifker added platform PR-s that make changes to core parts of the platform and removed product PR or Issue related to the DataHub UI/UX labels Feb 16, 2023

Merge branch 'master' into adv-search-docs-and-tests

9d99d9f

david-leifker marked this pull request as draft February 16, 2023 23:18

david-leifker and others added 3 commits February 18, 2023 16:36

exact match weight

3776ea2

Merge branch 'master' into adv-search-docs-and-tests

aad58e0

fix tests

b632c4e

david-leifker force-pushed the adv-search-docs-and-tests branch from 020a3f5 to b632c4e Compare February 19, 2023 01:49

david-leifker marked this pull request as ready for review February 19, 2023 03:24

szalai1 reviewed Feb 19, 2023

View reviewed changes

szalai1 approved these changes Feb 19, 2023

View reviewed changes

david-leifker merged commit 7bbabe0 into datahub-project:master Feb 20, 2023

jjoyce0510 reviewed Feb 20, 2023

View reviewed changes

oleg-ruban pushed a commit to RChygir/datahub that referenced this pull request Feb 28, 2023

feat(elasticsearch): advanced query, identity autocomplete, exact mat…

546a39e

…ch weight (datahub-project#7354)

yoonhyejin pushed a commit that referenced this pull request Mar 3, 2023

feat(elasticsearch): advanced query, identity autocomplete, exact mat…

54be17e

…ch weight (#7354)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticsearch): advanced query, identity autocomplete, exact match weight #7354

feat(elasticsearch): advanced query, identity autocomplete, exact match weight #7354

david-leifker commented Feb 16, 2023

david-leifker commented Feb 16, 2023

xiphl commented Feb 17, 2023

szalai1 Feb 19, 2023

jjoyce0510 Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

david-leifker Feb 20, 2023

jjoyce0510 Feb 20, 2023

feat(elasticsearch): advanced query, identity autocomplete, exact match weight #7354

feat(elasticsearch): advanced query, identity autocomplete, exact match weight #7354

Conversation

david-leifker commented Feb 16, 2023

Checklist

david-leifker commented Feb 16, 2023

xiphl commented Feb 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment