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

Change a NPE to have a better error message #5482

Merged
merged 1 commit into from
Jun 28, 2020
Merged

Change a NPE to have a better error message #5482

merged 1 commit into from
Jun 28, 2020

Conversation

prm-dan
Copy link
Contributor

@prm-dan prm-dan commented Jun 2, 2020

Description

Improve an error message when a table spec is missing a dimension that is specified in a star tree schema.

#5479

@@ -60,7 +60,11 @@ public ImmutableSegmentImpl(SegmentDirectory segmentDirectory, SegmentMetadataIm

@Override
public Dictionary getDictionary(String column) {
return _indexContainerMap.get(column).getDictionary();
ColumnIndexContainer container = _indexContainerMap.get(column);
if (container == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Preconditions.checkNotNull(container, "Invalid column: " + column);

Copy link
Member

Choose a reason for hiding this comment

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

@snleee, was in pending state for too many days. Better to get the functionality committed asap

Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

I don't think this null check is needed though. We don't have this check for all other types of indexes, and this is internal only code

@kishoreg kishoreg merged commit a3faa2f into apache:master Jun 28, 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

5 participants