Skip to content

Conversation

@sameeragarwal
Copy link
Member

What changes were proposed in this pull request?

This PR adds support for automatically inferring IsNotNull constraints from any non-nullable attributes that are part of an operator's output. This also fixes the issue that causes the optimizer to hit the maximum number of iterations for certain queries in #11828.

How was this patch tested?

Unit test in ConstraintPropagationSuite

Copy link
Contributor

Choose a reason for hiding this comment

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

can we break chains like this down with intermediate variables and comments? they are becoming fairly difficult to understand.

Also the foldLeft/union ... seems like a pretty inefficient way to run this and also difficult to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, for sure

@SparkQA
Copy link

SparkQA commented Mar 25, 2016

Test build #54157 has finished for PR 11953 at commit b630c17.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sameeragarwal
Copy link
Member Author

cc @davies: I verified that this patch fixes union20 in HiveCompatibilitySuite (with your max-iteration changes). The issue apparently was that when NullFiltering rule removed the filters for non-nullable attributes, we lost track of their corresponding constraints (and they'd then be pushed back by InferFiltersFromConstraints)

@SparkQA
Copy link

SparkQA commented Mar 25, 2016

Test build #54159 has finished for PR 11953 at commit 6421ba7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@davies
Copy link
Contributor

davies commented Mar 25, 2016

#11828 Has been merged, could you enable union20 here ?

@sameeragarwal
Copy link
Member Author

thanks, done

@davies
Copy link
Contributor

davies commented Mar 25, 2016

LGTM

@SparkQA
Copy link

SparkQA commented Mar 25, 2016

Test build #54191 has finished for PR 11953 at commit 2f43d41.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@davies
Copy link
Contributor

davies commented Mar 25, 2016

Merging this into master

@asfgit asfgit closed this in afd0deb Mar 25, 2016
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.

4 participants