Skip to content

Conversation

@jsoriano
Copy link
Member

@jsoriano jsoriano commented Oct 11, 2023

Accept null in test documents for values when an array of objects is expected.

@jsoriano
Copy link
Member Author

/test

1 similar comment
@jsoriano
Copy link
Member Author

/test

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 11, 2023

💔 Build Failed

Failed CI Steps

History

cc @jsoriano

Comment on lines +745 to +750
Fields: []FieldDefinition{
{
Name: "id",
Type: "keyword",
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an empty array ?

Copy link
Member Author

Choose a reason for hiding this comment

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

An empty array would work. The problem solved here is when a field defined as group or nested is null in a document, like onepassword.details here elastic/integrations@09a01f2#diff-76a51622c000706ccf925266e2e1255f954f7b55380135342a9d51aa3b81b5cdL55

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I misunderstood the definition field. I thought it was resulting definition built.

Just to be sure, it's being compared the value nil of a field null_array against that definition/schema in definition. Would that be right?

Copy link
Member Author

@jsoriano jsoriano Oct 11, 2023

Choose a reason for hiding this comment

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

Yes, this would be like validating a document with this JSON content:

{
  "null_array": null
}

With this fields definition:

- name: null_array
  type: group
  fields: ...

Validation fails without this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@jsoriano
Copy link
Member Author

Failure in AWS test package seems unrelated, merging.

@jsoriano jsoriano merged commit a9bc375 into elastic:main Oct 11, 2023
@jsoriano jsoriano deleted the null-arrays branch October 11, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants