Skip to content

TextFieldMapperTests use synthetic id randomly#143069

Merged
burqen merged 1 commit intoelastic:mainfrom
burqen:ap/2026.02.25.synthetic-id-fix-TextFieldMapperTests
Feb 26, 2026
Merged

TextFieldMapperTests use synthetic id randomly#143069
burqen merged 1 commit intoelastic:mainfrom
burqen:ap/2026.02.25.synthetic-id-fix-TextFieldMapperTests

Conversation

@burqen
Copy link
Copy Markdown
Contributor

@burqen burqen commented Feb 25, 2026

When running with synthetic id we would calculate routing from id. This would hit the assertion in TsidExtractingIdFieldMapper .extractRoutingHashFromSyntheticId because the dummy id value is too short. Since it's not the intention of the test to exercise that code path we simply set routing explicitly instead. The alternative would have been to compute a valid synthetic id and that's too much work.

This is preparation for enabling synthetic id by default #142366

When running with synthetic id we would calculate routing from id. This
would hit the assertion in TsidExtractingIdFieldMapper
.extractRoutingHashFromSyntheticId because the dummy id value is too
short. Since it's not the intention of the test to exercise that code
path we simply set routing explicitly instead. The alternative would
have been to compute a valid synthetic id and that's too much work.
@burqen burqen requested review from fcofdez and tlrx February 25, 2026 14:33
@burqen burqen added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics v9.4.0 labels Feb 25, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

Copy link
Copy Markdown
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM - Just to be sure I understand, this is preparatory work for enabling synthetic id by default right? If so, maybe the description should mention it.

@burqen
Copy link
Copy Markdown
Contributor Author

burqen commented Feb 26, 2026

LGTM - Just to be sure I understand, this is preparatory work for enabling synthetic id by default right? If so, maybe the description should mention it.

Yes, good point. I've added it to the description.

@burqen burqen merged commit f70db3c into elastic:main Feb 26, 2026
35 checks passed
PeteGillinElastic pushed a commit to PeteGillinElastic/elasticsearch that referenced this pull request Feb 27, 2026
When running with synthetic id we would calculate routing from id. This
would hit the assertion in TsidExtractingIdFieldMapper
.extractRoutingHashFromSyntheticId because the dummy id value is too
short. Since it's not the intention of the test to exercise that code
path we simply set routing explicitly instead. The alternative would
have been to compute a valid synthetic id and that's too much work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:StorageEngine/TSDB You know, for Metrics Team:StorageEngine >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants