Skip to content

[SPARK-16644][SQL] Aggregate should not propagate constraints containing aggregate expressions#14281

Closed
cloud-fan wants to merge 2 commits into
apache:masterfrom
cloud-fan:bug
Closed

[SPARK-16644][SQL] Aggregate should not propagate constraints containing aggregate expressions#14281
cloud-fan wants to merge 2 commits into
apache:masterfrom
cloud-fan:bug

Conversation

@cloud-fan
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

aggregate expressions can only be executed inside Aggregate, if we propagate it up with constraints, the parent operator can not execute it and will fail at runtime.

How was this patch tested?

new test in SQLQuerySuite

@cloud-fan
Copy link
Copy Markdown
Contributor Author

cc @sameeragarwal @yhuai @liancheng

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 20, 2016

Test build #62589 has finished for PR 14281 at commit bc0157f.

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

}
}

test("Aggregate should not propagate constraints containing aggregate expressions") {
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.

put a jira number, and see if we can move this into constraint suite?

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.

Nice find. I agree, it'd be good to put this in ConstraintPropagationSuite and explicitly check for the inferred constraints.


val aliasedRelation = tr.where('c.attr > 10 && 'a.attr < 5)
.groupBy('a, 'c, 'b)('a, 'c.as("c1"), count('a).as("a3")).select('c1, 'a).analyze
.groupBy('a, 'c, 'b)('a, 'c.as("c1"), count('a).as("a3")).select('c1, 'a, 'a3).analyze
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.

@sameeragarwal How's this one?

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.

looks good

@sameeragarwal
Copy link
Copy Markdown
Member

LGTM

@SparkQA
Copy link
Copy Markdown

SparkQA commented Jul 20, 2016

Test build #62634 has finished for PR 14281 at commit 48d81c8.

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

@yhuai
Copy link
Copy Markdown
Contributor

yhuai commented Jul 21, 2016

Thanks. I am merging this to master and branch 2.0.

@asfgit asfgit closed this in cfa5ae8 Jul 21, 2016
asfgit pushed a commit that referenced this pull request Jul 21, 2016
…ing aggregate expressions

aggregate expressions can only be executed inside `Aggregate`, if we propagate it up with constraints, the parent operator can not execute it and will fail at runtime.

new test in SQLQuerySuite

Author: Wenchen Fan <wenchen@databricks.com>
Author: Yin Huai <yhuai@databricks.com>

Closes #14281 from cloud-fan/bug.

(cherry picked from commit cfa5ae8)
Signed-off-by: Yin Huai <yhuai@databricks.com>
@liancheng liancheng deleted the bug branch July 21, 2016 09:05
@liancheng liancheng restored the bug branch July 21, 2016 09:05
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.

5 participants