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

Table indexing config validation #6017

Merged
merged 4 commits into from Sep 18, 2020

Conversation

icefury71
Copy link
Contributor

Related to #5942

Description

This is to ensure any column name mentioned in the table indexing config is valid (i.e. it exists in the corresponding schema). Also checking for space in the table name.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
No

Does this PR fix a zero-downtime upgrade introduced earlier?
No

Does this PR otherwise need attention when creating release notes?
No

…Adding javadocs

* Bug fix in integration test regarding removal of existing column from schema
Copy link
Contributor

@siddharthteotia siddharthteotia left a comment

Choose a reason for hiding this comment

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

LGTM

@npawar
Copy link
Contributor

npawar commented Sep 17, 2020

As discussed offline, it would be nice to have some validations on index columns which cannot be in noDictionary columns (e.g. if column has inverted index, it should not be in noDictionary).
Are you planning to add that as part of same PR?

@icefury71
Copy link
Contributor Author

As discussed offline, it would be nice to have some validations on index columns which cannot be in noDictionary columns (e.g. if column has inverted index, it should not be in noDictionary).
Are you planning to add that as part of same PR?

Not in this PR. I'll create a separate one for that.

@haibow haibow merged commit 5548e79 into apache:master Sep 18, 2020
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.

None yet

6 participants