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

Add null check for ParentJoinFieldMapper in ChildrenAggregationBuilder.joinFieldResolveConfig #42997

Closed
ebadyano opened this issue Jun 7, 2019 · 15 comments · Fixed by #57089
Closed
Labels
:Analytics/Aggregations Aggregations >bug good first issue low hanging fruit Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@ebadyano
Copy link
Contributor

ebadyano commented Jun 7, 2019

A user received a NPE. It was reproduced on 6.3.1 with the following stack trace:

2019-06-06T13:26:49,963][DEBUG][o.e.a.s.TransportSearchAction] [hamilton] [mi_indice][2], node[ABwM3KQJQdWuKfnzwJ8pxw], [P], s[STARTED], a[id=cRQiXG2DTmijJ2IMjtYq5g]: Failed to execute [SearchRequest{searchType=QUERY_THEN_FETCH, indices=[mi_indice], indicesOptions=IndicesOptions[id=38, ignore_unavailable=false, allow_no_indices=true, expand_wildcards_open=true, expand_wildcards_closed=false, allow_aliases_to_multiple_indices=true, forbid_closed_indices=true, ignore_aliases=false], types=[], routing='null', preference='null', requestCache=null, scroll=null, maxConcurrentShardRequests=15, batchedReduceSize=512, preFilterShardSize=128, allowPartialSearchResults=true, source={"query":{"bool":{"must":[{"bool":{"must":[{"bool":{"should":[{"range":{"posting_date":{"from":"1900-01-01T00:00:00.000Z","to":"2019-05-30T18:25:49.967Z","include_lower":true,"include_upper":true,"boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}},{"term":{"externaljob_to_child_fields.name":{"value":"externaljob","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}}],"adjust_pure_negative":true,"boost":1.0}},"aggregations":{"POSTING_DATES":{"date_range":{"field":"posting_date","ranges":[{"key":"ORA_LT07","from":"2019-05-23T18:25:50.006Z","to":"2019-05-30T18:25:50.006Z"},{"key":"ORA_LT30","from":"2019-04-30T18:25:50.006Z","to":"2019-05-30T18:25:50.006Z"},{"key":"ORA_GT30","from":"1900-01-01T00:00:00.000Z","to":"2019-04-30T18:25:50.006Z"}],"keyed":false}},"LOCATIONS":{"children":{"type":"externaljob_locs"}}}}}] lastShard [true]
org.elasticsearch.transport.RemoteTransportException: [washington][127.0.0.1:9302][indices:data/read/search[phase/query]]
Caused by: java.lang.NullPointerException
	at org.elasticsearch.join.aggregations.ChildrenAggregationBuilder.joinFieldResolveConfig(ChildrenAggregationBuilder.java:122) ~[?:?]
	at org.elasticsearch.join.aggregations.ChildrenAggregationBuilder.resolveConfig(ChildrenAggregationBuilder.java:113) ~[?:?]
	at org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder.doBuild(ValuesSourceAggregationBuilder.java:311) ~[elasticsearch-6.3.1.jar:6.3.1]
	at org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder.doBuild(ValuesSourceAggregationBuilder.java:38) ~[elasticsearch-6.3.1.jar:6.3.1]
	at org.elasticsearch.search.aggregations.AbstractAggregationBuilder.build(AbstractAggregationBuilder.java:139) ~[elasticsearch-6.3.1.jar:6.3.1]
	at org.elasticsearch.search.aggregations.AggregatorFactories$Builder.build(AggregatorFactories.java:329) ~[elasticsearch-6.3.1.jar:6.3.1]
	at org.elasticsearch.search.SearchService.parseSource(SearchService.java:766) ~[elasticsearch-6.3.1.jar:6.3.1]
	at org.elasticsearch.search.SearchService.createContext(SearchService.java:575) ~[elasticsearch-6.3.1.jar:6.3.1]

in ChildrenAggregationBuilder.joinFieldResolveConfig

private void joinFieldResolveConfig(SearchContext context, ValuesSourceConfig<WithOrdinals> config) {
        ParentJoinFieldMapper parentJoinFieldMapper = ParentJoinFieldMapper.getMapper(context.mapperService()); 
        ParentIdFieldMapper parentIdFieldMapper = parentJoinFieldMapper.getParentIdFieldMapper(childType, false);
        if (parentIdFieldMapper != null) {
...
        } else {
            config.unmapped(true);
        }

ParentJoinFieldMapper.getMapper returns null when there is no parent-join field in the mapping. We already check to see if parentIdFieldMapper is null and use an unmapped. Should probably be doing the same for parentJoinFieldMapper as well.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@0xamogh
Copy link

0xamogh commented Jun 7, 2019

Hi can I be assigned this issue?

@jasontedor
Copy link
Member

We can not assign issues to non-members (GitHub restriction), though you are free to submit a pull request for this.

@0xamogh
Copy link

0xamogh commented Jun 9, 2019

Okay! I am working on it

@shubh49
Copy link

shubh49 commented Jun 10, 2019

@amogh-jrules Are you working on this? If not, i am willing to take this up.

@0xamogh
Copy link

0xamogh commented Jun 15, 2019

@amogh-jrules Are you working on this? If not, i am willing to take this up.

I am working on it, as i said. about to submit a pull request

@hassan-alam
Copy link

@amogh-jrules Are you still working on this? Your PR hasn't been changed in a while, so if you're not on this anymore, I can take this up.

@MachSphere
Copy link

What is happening with this PR? It has stalled for about 2 months, is it because the commits were not squashed into 1 commit as stated in the CONTRIBUTING.md?

Suchita94 added a commit to Suchita94/elasticsearch that referenced this issue Oct 20, 2019
…and handling it in the same way it is handled for parentIdFieldMapper
Suchita94 added a commit to Suchita94/elasticsearch that referenced this issue Oct 20, 2019
…and handling it in the same way it is handled for parentIdFieldMapper
@Suchita94
Copy link

Since this bug has been open for a while, created pull request with code changes and test cases

@ebadyano
Copy link
Contributor Author

ebadyano commented May 4, 2020

@SD1998 It looks like it might still exist, but the method has been renamed to protected ValuesSourceConfig resolveConfig. Please feel free to submit a pull request for this.

@rjernst rjernst added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 4, 2020
@parthpunkster
Copy link
Contributor

I see that this issue is still open, can I work on this?

@parthpunkster
Copy link
Contributor

Can someone assign this to me?

@jimczi
Copy link
Contributor

jimczi commented May 13, 2020

I see that this issue is still open, can I work on this?

Sure, please do. We don't assign issue so you can open a pr that links this issue and we'll review it.

@parthpunkster
Copy link
Contributor

I see that this issue is still open, can I work on this?

Sure, please do. We don't assign issue so you can open a pr that links this issue and we'll review it.

I have worked on this one wanted some help around verifying the change and on the process of raising a pull request.

parthpunkster added a commit to parthpunkster/elasticsearch that referenced this issue May 24, 2020
… ChildrenAggregationBuilder.joinFieldResolveConfig
@parthpunkster
Copy link
Contributor

@jimczi I have attached the pull request. Can you please review it.

nik9000 pushed a commit that referenced this issue Jul 6, 2020
…#57089)

Adding null check for ParentJoinFieldMapper in ChildrenAggregationBuilder.joinFieldResolveConfig

Closes #42997
nik9000 pushed a commit to nik9000/elasticsearch that referenced this issue Jul 6, 2020
…elastic#57089)

Adding null check for ParentJoinFieldMapper in ChildrenAggregationBuilder.joinFieldResolveConfig

Closes elastic#42997
nik9000 added a commit that referenced this issue Jul 6, 2020
…#57089) (#59074)

Adding null check for ParentJoinFieldMapper in ChildrenAggregationBuilder.joinFieldResolveConfig

Closes #42997

Co-authored-by: ParthPunkster <parthjain.pj1994@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug good first issue low hanging fruit Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging a pull request may close this issue.