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.

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

Bug #42997 #48270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Bug #42997 #48270

wants to merge 2 commits into from

Conversation

Suchita94
Copy link

Added null check and modified test cases

…and handling it in the same way it is handled for parentIdFieldMapper
…and handling it in the same way it is handled for parentIdFieldMapper
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

Thanks @Suchita94, I left two comments. Can you also update the title to be more descriptive ? Something like: "fixed children aggregation with unmapped fields".

@@ -112,16 +112,21 @@ protected void innerWriteTo(StreamOutput out) throws IOException {

private void joinFieldResolveConfig(QueryShardContext queryShardContext, ValuesSourceConfig<WithOrdinals> config) {
ParentJoinFieldMapper parentJoinFieldMapper = ParentJoinFieldMapper.getMapper(queryShardContext.getMapperService());
if (parentJoinFieldMapper != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be:

Suggested change
if (parentJoinFieldMapper != null) {
if (parentJoinFieldMapper == null) {

?

if (parentJoinFieldMapper != null) {
config.unmapped(true);
return;
}
ParentIdFieldMapper parentIdFieldMapper = parentJoinFieldMapper.getParentIdFieldMapper(childType, false);
if (parentIdFieldMapper != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here ?

Suggested change
if (parentIdFieldMapper != null) {
if (parentIdFieldMapper == null) {

@elasticmachine
Copy link
Collaborator

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

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
@bpintea bpintea added v7.8.0 and removed v7.7.0 labels Mar 25, 2020
@rjernst rjernst added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 4, 2020
@andreidan andreidan added v7.11.0 and removed v7.10.0 labels Oct 7, 2020
@danhermann danhermann added v8.1.0 and removed v7.16.0 labels Oct 27, 2021
@arteam arteam removed the v8.0.0 label Jan 12, 2022
@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:14
@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
@elasticsearchmachine
Copy link
Collaborator

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

@nik9000 nik9000 removed the v8.5.0 label Jul 29, 2022
@nik9000
Copy link
Member

nik9000 commented Jul 29, 2022

Looks like we got this in #57089. Sorry for letting this languish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet