Skip to content

add method getRequiredColumns for DimFilter#5872

Merged
jon-wei merged 2 commits intoapache:masterfrom
zhangxinyu1:feature-add-getRequiredColumns-method-to-dimFilter
Jun 27, 2018
Merged

add method getRequiredColumns for DimFilter#5872
jon-wei merged 2 commits intoapache:masterfrom
zhangxinyu1:feature-add-getRequiredColumns-method-to-dimFilter

Conversation

@zhangxinyu1
Copy link
Copy Markdown
Contributor

In order to get required columns in materialized-view feature, it's better to add a method to DimFilter which returns all required column names.
see issue #5710

@jihoonson
Copy link
Copy Markdown
Contributor

@zhangxinyu1 thank you for the patch! Would you check the CI failure?

@zhangxinyu1 zhangxinyu1 force-pushed the feature-add-getRequiredColumns-method-to-dimFilter branch from 40e5ede to 9a3bf55 Compare June 12, 2018 03:41
@zhangxinyu1 zhangxinyu1 reopened this Jun 12, 2018
@zhangxinyu1
Copy link
Copy Markdown
Contributor Author

@jihoonson I have fixed the bug. Could you please take a look at it?

@jon-wei jon-wei merged commit d857345 into apache:master Jun 27, 2018
/**
* @return a HashSet that represents all columns' name which the DimFilter required to do filter.
*/
HashSet<String> getRequiredColumns();
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.

Can we have the return type be Set<String> instead so that single column cases can just return ImmutableSet.of(col) ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm making this change to use Set as part of #8209.

@dclim dclim added this to the 0.13.0 milestone Oct 8, 2018
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.

6 participants