Skip to content

Fix dim validation for bit element_type#114533

Merged
elasticsearchmachine merged 3 commits intoelastic:mainfrom
benwtrent:bugfix/fix-dim-support-bit
Oct 11, 2024
Merged

Fix dim validation for bit element_type#114533
elasticsearchmachine merged 3 commits intoelastic:mainfrom
benwtrent:bugfix/fix-dim-support-bit

Conversation

@benwtrent
Copy link
Copy Markdown
Member

A silly bug has reared its ugly head. Apparently, our dimension validations are predicated on JSON parsing order, that is not good.

So, this commit adjusts the dim validations so that it is an actual validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding getMaxDimensions correctly. Typically, and in production, this isn't that big of a deal, but I have found it useful to do this for other testing purposes (so that we don't have to rely on the perfield codec for more direct and advanced testing).

@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Oct 10, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @benwtrent, I've created a changelog YAML for you.

Copy link
Copy Markdown
Contributor

@john-wagster john-wagster left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@tteofili tteofili left a comment

Choose a reason for hiding this comment

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

LGTM, good catch Ben!

@benwtrent
Copy link
Copy Markdown
Member Author

@elasticmachine update branch

@benwtrent benwtrent added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Oct 11, 2024
@elasticsearchmachine elasticsearchmachine merged commit 7942f3e into elastic:main Oct 11, 2024
@benwtrent benwtrent deleted the bugfix/fix-dim-support-bit branch October 11, 2024 12:36
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
8.x
8.15 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 114533

benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Oct 11, 2024
A silly bug has reared its ugly head. Apparently, our dimension
validations are predicated on JSON parsing order, that is not good. 

So, this commit adjusts the dim validations so that it is an actual
validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding
`getMaxDimensions` correctly. Typically, and in production, this isn't
that big of a deal, but I have found it useful to do this for other
testing purposes (so that we don't have to rely on the perfield codec
for more direct and advanced testing).
@benwtrent
Copy link
Copy Markdown
Member Author

💚 All backports created successfully

Status Branch Result
8.15

Questions ?

Please refer to the Backport tool documentation

benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Oct 11, 2024
A silly bug has reared its ugly head. Apparently, our dimension
validations are predicated on JSON parsing order, that is not good.

So, this commit adjusts the dim validations so that it is an actual
validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding
`getMaxDimensions` correctly. Typically, and in production, this isn't
that big of a deal, but I have found it useful to do this for other
testing purposes (so that we don't have to rely on the perfield codec
for more direct and advanced testing).

(cherry picked from commit 7942f3e)
elasticsearchmachine pushed a commit that referenced this pull request Oct 11, 2024
A silly bug has reared its ugly head. Apparently, our dimension
validations are predicated on JSON parsing order, that is not good.

So, this commit adjusts the dim validations so that it is an actual
validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding
`getMaxDimensions` correctly. Typically, and in production, this isn't
that big of a deal, but I have found it useful to do this for other
testing purposes (so that we don't have to rely on the perfield codec
for more direct and advanced testing).

(cherry picked from commit 7942f3e)
elasticsearchmachine pushed a commit that referenced this pull request Oct 11, 2024
A silly bug has reared its ugly head. Apparently, our dimension
validations are predicated on JSON parsing order, that is not good. 

So, this commit adjusts the dim validations so that it is an actual
validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding
`getMaxDimensions` correctly. Typically, and in production, this isn't
that big of a deal, but I have found it useful to do this for other
testing purposes (so that we don't have to rely on the perfield codec
for more direct and advanced testing).

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this pull request Oct 13, 2024
A silly bug has reared its ugly head. Apparently, our dimension
validations are predicated on JSON parsing order, that is not good. 

So, this commit adjusts the dim validations so that it is an actual
validation, instead of something that occurs during parsing.

Additionally, I found that our custom formats were not overriding
`getMaxDimensions` correctly. Typically, and in production, this isn't
that big of a deal, but I have found it useful to do this for other
testing purposes (so that we don't have to rely on the perfield codec
for more direct and advanced testing).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.15.4 v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants