Skip to content

Conversation

@liancheng
Copy link
Contributor

What changes were proposed in this pull request?

MAX(COUNT(*)) is invalid since aggregate expression can't be nested within another aggregate expression. This case should be captured at analysis phase, but somehow sneaks off to runtime.

The reason is that when checking aggregate expressions in CheckAnalysis, a checking branch treats all expressions that reference no input attributes as valid ones. However, MAX(COUNT(*)) is translated into MAX(COUNT(1)) at analysis phase and also references no input attribute.

This PR fixes this issue by removing the aforementioned branch.

How was this patch tested?

New test case added in AnalysisErrorSuite.

@liancheng
Copy link
Contributor Author

cc @yhuai @cloud-fan @clockfly

@liancheng liancheng force-pushed the spark-16291-nested-agg-functions branch from 18e7485 to 9a1f711 Compare June 29, 2016 09:25
@SparkQA
Copy link

SparkQA commented Jun 29, 2016

Test build #61456 has finished for PR 13968 at commit 18e7485.

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

@cloud-fan
Copy link
Contributor

LGTM

asfgit pushed a commit that referenced this pull request Jun 29, 2016
…tions that reference no input attributes

## What changes were proposed in this pull request?

`MAX(COUNT(*))` is invalid since aggregate expression can't be nested within another aggregate expression. This case should be captured at analysis phase, but somehow sneaks off to runtime.

The reason is that when checking aggregate expressions in `CheckAnalysis`, a checking branch treats all expressions that reference no input attributes as valid ones. However, `MAX(COUNT(*))` is translated into `MAX(COUNT(1))` at analysis phase and also references no input attribute.

This PR fixes this issue by removing the aforementioned branch.

## How was this patch tested?

New test case added in `AnalysisErrorSuite`.

Author: Cheng Lian <[email protected]>

Closes #13968 from liancheng/spark-16291-nested-agg-functions.

(cherry picked from commit d1e8108)
Signed-off-by: Wenchen Fan <[email protected]>
@cloud-fan
Copy link
Contributor

merging to master/2.0

@asfgit asfgit closed this in d1e8108 Jun 29, 2016
@SparkQA
Copy link

SparkQA commented Jun 29, 2016

Test build #61457 has finished for PR 13968 at commit 9a1f711.

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

@liancheng liancheng deleted the spark-16291-nested-agg-functions branch June 29, 2016 20:00
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.

3 participants