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

Ignore ES indexes without mappings #4535

Merged
merged 1 commit into from
Jul 28, 2020
Merged

Conversation

aalbu
Copy link
Member

@aalbu aalbu commented Jul 22, 2020

Empty indexes can cause exceptions when trying to query the corresponding table or the information schema (e.g. the columns table).

@cla-bot cla-bot bot added the cla-signed label Jul 22, 2020
@aalbu aalbu requested review from martint and losipiuk July 22, 2020 12:33
Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

LGTM but I would prefer to wait for Martin as I am not too familiar with ES connector.

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

(skimming)

private void createIndex(String indexName)
throws IOException
{
client.getLowLevelClient().performRequest("PUT", "/" + indexName, ImmutableMap.of());
Copy link
Member

Choose a reason for hiding this comment

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

is it equiv to createIndex(indexName, '{}')?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, ElasticsearchClient expects at least one property to be present when processing mappings.

@martint martint merged commit b8807b4 into trinodb:master Jul 28, 2020
@martint martint added this to the 340 milestone Jul 28, 2020
@martint martint mentioned this pull request Aug 7, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants