Skip to content

Support synthetic _source for _doc_count field#91465

Merged
nik9000 merged 6 commits intoelastic:mainfrom
nik9000:synthetic_source_doc_Count
Nov 10, 2022
Merged

Support synthetic _source for _doc_count field#91465
nik9000 merged 6 commits intoelastic:mainfrom
nik9000:synthetic_source_doc_Count

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Nov 9, 2022

This add synthetic _source support for the _doc_count field so downsampling should play nicely with sythetic _source.

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Nov 9, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

}

@Override
public abstract SourceLoader.SyntheticFieldLoader syntheticFieldLoader();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I would like to make this abstract across the board just so everyone who makes a field has to think about it. If they don't want to support it, fine, but I'd like folks to think about it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But I didn't want to make it abstract in Mapper yet. It's just twice as many changes....

This add synthetic `_source` support for the `_doc_count` field so
downsampling should play nicely with sythetic `_source`.
Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

One question, LGTM otherwise!

public void testSyntheticSourceMany() throws IOException {
DocumentMapper mapper = createDocumentMapper(syntheticSourceMapping(b -> {}));
List<Integer> counts = randomList(2, 10000, () -> between(1, Integer.MAX_VALUE));
try (Directory directory = newDirectory()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does withLuceneIndex not work here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let me have another look. I think there was a reason, but it might not apply right here.

Copy link
Copy Markdown
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

LGTM!


@Override
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
postings = leafReader.postings(new Term(DocCountFieldMapper.NAME, DocCountFieldMapper.NAME));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Does it make sense to make new Term(DocCountFieldMapper.NAME, DocCountFieldMapper.NAME) a static final constant?

if (hasValue == false) {
return;
}
b.field("_doc_count", postings.freq());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

another nit: Maybe change "_doc_count" to DocCountFieldMapper.NAME?

@nik9000 nik9000 merged commit 9d0b0ba into elastic:main Nov 10, 2022
@nik9000 nik9000 mentioned this pull request Nov 14, 2022
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants