Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data views] Remove regex for creating dot notation #193795

Merged
merged 1 commit into from
Sep 24, 2024
+11 −3
Merged

[data views] Remove regex for creating dot notation #193795

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/plugins/data_views/common/fields/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,22 @@ export const isMultiField = isDataViewFieldSubtypeMulti;
export const getFieldSubtypeMulti = getDataViewFieldSubtypeMulti;
export const getFieldSubtypeNested = getDataViewFieldSubtypeNested;

const DOT_PREFIX_RE = /(.).+?\./g;

/**
* Convert a dot.notated.string into a short
* version (d.n.string)
*/
export function shortenDottedString(input: string): string {
return typeof input !== 'string' ? input : input.replace(DOT_PREFIX_RE, '$1.');
if (typeof input === 'string') {
const split = input.split('.');
Copy link
Contributor

@jughosta jughosta Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can some parts be empty? For example, can input be something like foo..bar.foo?

Copy link
Contributor Author

@mattkime mattkime Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, field names can be anything. having two dots in a row would result in f.undefined.b.foo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we address undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#193874

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI I actually tried ingesting a document like foo..bar.foo and I got an error:

POST tmp-00001/_doc
{
  "asdf..basdf": "bar"
}

// Results in 
{
  "error": {
    "root_cause": [
      {
        "type": "document_parsing_exception",
        "reason": "[1:4] failed to parse: field name cannot contain only whitespace: ['asdf..basdf']"
      }
    ],
    "type": "document_parsing_exception",
    "reason": "[1:4] failed to parse: field name cannot contain only whitespace: ['asdf..basdf']",
    "caused_by": {
      "type": "illegal_argument_exception",
      "reason": "field name cannot contain only whitespace: ['asdf..basdf']"
    }
  },
  "status": 400
}

Copy link
Contributor Author

@mattkime mattkime Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, I stand corrected. I swear this must be the only limitation on field names. Might be best to talk to an ES engineer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll skip the follow up until someone can produce a field name with two dots in a row

return split.reduce((acc, part, i) => {
if (i === split.length - 1) {
return acc + part;
}
return acc + part[0] + '.';
}, '');
}

return input;
}

// Note - this code is duplicated from @kbn/es-query
Expand Down