Skip to content

Conversation

@gatorsmile
Copy link
Member

What changes were proposed in this pull request?

This PR is to improve the code comments to document some common traits and traps about the expression.

How was this patch tested?

N/A

@gatorsmile
Copy link
Member Author

cc @rxin @rednaxelafx @cloud-fan

@gatorsmile gatorsmile changed the title [SPARK-26168] Update the code comments in Expression and Aggregate [SPARK-26168][SQL] Update the code comments in Expression and Aggregate Nov 25, 2018
* @param aggregateExpressions expressions for a project list, which could contain
* [[org.apache.spark.sql.catalyst.expressions.aggregate.AggregateFunction]]s.
*
* Note: Currently, aggregateExpressions correspond to both [[AggregateExpression]] and the output
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear what “resultExpressions” mean.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed it from the description.

@SparkQA
Copy link

SparkQA commented Nov 26, 2018

Test build #99246 has finished for PR 23135 at commit a6e7256.

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

* - [[HigherOrderFunction]]: a common base trait for higher order functions that take one or more
* (lambda) functions and applies these to some objects. The function
* produces a number of variables which can be consumed by some lambda
* function.
Copy link
Member

Choose a reason for hiding this comment

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

nit: function -> functions ?

* interpreted mode.
* - [[NullIntolerant]]: an expression that is null intolerant (i.e. any null input will result in
* null output).
* - [[NonSQLExpression]]: a common base trait for the expressions that doesn't have SQL
Copy link
Member

Choose a reason for hiding this comment

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

nit: doesn't -> do not

* produces a number of variables which can be consumed by some lambda
* function.
* - [[NamedExpression]]: An [[Expression]] that is named.
* - [[TimeZoneAwareExpression]]: A common base trait for time zone aware expressions.
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we also mention SubqueryExpression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

@SparkQA
Copy link

SparkQA commented Nov 26, 2018

Test build #99248 has finished for PR 23135 at commit fe8e385.

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

@cloud-fan
Copy link
Contributor

LGTM

1 similar comment
@kiszk
Copy link
Member

kiszk commented Nov 26, 2018

LGTM

@SparkQA
Copy link

SparkQA commented Nov 26, 2018

Test build #99257 has finished for PR 23135 at commit cd682ff.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in 6bb60b3 Nov 26, 2018
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?
This PR is to improve the code comments to document some common traits and traps about the expression.

## How was this patch tested?
N/A

Closes apache#23135 from gatorsmile/addcomments.

Authored-by: gatorsmile <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
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