Skip to content

[CALCITE-5974] Elasticsearch adapter throws ClassCastException when index mapping sets dynamic_templates without properties#3406

Closed
zoov-w wants to merge 1 commit intoapache:mainfrom
zoov-w:es-adapter-visit-mapping
Closed

[CALCITE-5974] Elasticsearch adapter throws ClassCastException when index mapping sets dynamic_templates without properties#3406
zoov-w wants to merge 1 commit intoapache:mainfrom
zoov-w:es-adapter-visit-mapping

Conversation

@zoov-w
Copy link
Copy Markdown

@zoov-w zoov-w commented Sep 1, 2023

when use es-adapter, A index set dynamic_templates bug no mappings causing exceptions:

java.lang.ClassCastException: com.fasterxml.jackson.databind.node.ArrayNode cannot be cast to com.fasterxml.jackson.databind.node.ObjectNode
at org.apache.calcite.adapter.elasticsearch.ElasticsearchJson.visitMappingProperties(ElasticsearchJson.java:133)

this pr fix it.

@jiefei30
Copy link
Copy Markdown
Contributor

jiefei30 commented Sep 2, 2023

when use es-adapter, A index set dynamic_templates bug no mappings causing exceptions:

java.lang.ClassCastException: com.fasterxml.jackson.databind.node.ArrayNode cannot be cast to com.fasterxml.jackson.databind.node.ObjectNode at org.apache.calcite.adapter.elasticsearch.ElasticsearchJson.visitMappingProperties(ElasticsearchJson.java:133)

this pr fix it.

@zoov-w thank you for opening this PR, but please read the contribution rules before you do this

@zoov-w zoov-w changed the title fix bug when es index mapping set dynamic_templates without properties [5974] fix bug when es index mapping set dynamic_templates without properties Sep 3, 2023
@zoov-w zoov-w changed the title [5974] fix bug when es index mapping set dynamic_templates without properties [CALCITE-5974] fix bug when es index mapping set dynamic_templates without properties Sep 3, 2023
@zoov-w zoov-w force-pushed the es-adapter-visit-mapping branch from e10f0ae to 964e56a Compare September 3, 2023 11:12
@zoov-w
Copy link
Copy Markdown
Author

zoov-w commented Sep 3, 2023

@jiefei30 Thanks, I pushed PR again according to contribution rules.

@zoov-w zoov-w changed the title [CALCITE-5974] fix bug when es index mapping set dynamic_templates without properties [CALCITE-5974] Fix bug when es index mapping set dynamic_templates without properties Sep 4, 2023
@LakeShen
Copy link
Copy Markdown
Contributor

LakeShen commented Sep 7, 2023

Hi @zoov-w ,don't begin with fix to the title of a JIRA, should describe the problem behavior.

BTW, you could rebase the latest code of main branch,the commits and files changed will be back to normal.

@zoov-w zoov-w force-pushed the es-adapter-visit-mapping branch from 964e56a to 2c8c6ae Compare September 9, 2023 18:34
@zoov-w zoov-w changed the title [CALCITE-5974] Fix bug when es index mapping set dynamic_templates without properties [CALCITE-5974] Elasticsearch adapter throws ClassCastException when index mapping sets dynamic_templates without properties Sep 9, 2023
@zoov-w zoov-w force-pushed the es-adapter-visit-mapping branch from 2c8c6ae to a6b794e Compare September 9, 2023 18:53
@zoov-w
Copy link
Copy Markdown
Author

zoov-w commented Sep 9, 2023

@LakeShen Thanks, I changed JIRA title and commits.

@zoov-w zoov-w force-pushed the es-adapter-visit-mapping branch 2 times, most recently from cabb6b4 to 88ad480 Compare September 10, 2023 09:17
…n es index set dynamic_templates without properties
@zoov-w zoov-w force-pushed the es-adapter-visit-mapping branch from ef4c033 to 9372109 Compare September 17, 2023 17:35
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Map<String, String> result = new HashMap<>();
ElasticsearchJson.visitMappingProperties(mapping, result::put);

assertThat(result.isEmpty(), is(true));
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.

The logic of this assertThat check isn't obvious,could you explain it in more code comment?

julianhyde pushed a commit to julianhyde/calcite that referenced this pull request Sep 21, 2023
…ndex mapping sets dynamic_templates without properties

Close apache#3406
@asfgit asfgit closed this in e1991e0 Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants