Skip to content

Conversation

@zsxwing
Copy link
Member

@zsxwing zsxwing commented May 26, 2014

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented May 26, 2014

There is another instance of this on line 457.

@zsxwing
Copy link
Member Author

zsxwing commented May 26, 2014

Good catch. Already fixed it. Thanks!

@srowen
Copy link
Member

srowen commented May 26, 2014

Speaking of this operator, one day I was watching Josh Bloch give a presentation about the dark corners of Java, and he was covering short-circuit boolean operations. He mentioned in passing that sometimes they are faster. I had to stop him to ask how it could ever be faster to always evaluate both conditions.

His reply was that it saved the branch that decided whether to evaluate the second condition, at the expense of having to always evaluate it of course. When the first condition was frequently false, could be faster to let the processor do the out-of-order evaluation rather than wait. He said this was only rarely useful, and we should always be defaulting to &&. So this is the right change :)

@zsxwing
Copy link
Member Author

zsxwing commented May 26, 2014

Nice explanation:) Thank you, @srowen

@rxin
Copy link
Contributor

rxin commented May 26, 2014

Jenkins, test this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15211/

@rxin
Copy link
Contributor

rxin commented May 26, 2014

Jenkins, retest this please.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15212/

@rxin
Copy link
Contributor

rxin commented May 26, 2014

Thanks. I've merged this into master & branch-1.0.

asfgit pushed a commit that referenced this pull request May 26, 2014
JIRA: https://issues.apache.org/jira/browse/SPARK-1925

Author: zsxwing <[email protected]>

Closes #879 from zsxwing/SPARK-1925 and squashes the following commits:

5cf5a6d [zsxwing] SPARK-1925: Replace '&' with '&&'

(cherry picked from commit cb7fe50)
Signed-off-by: Reynold Xin <[email protected]>
@asfgit asfgit closed this in cb7fe50 May 26, 2014
@zsxwing zsxwing deleted the SPARK-1925 branch May 27, 2014 02:40
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
JIRA: https://issues.apache.org/jira/browse/SPARK-1925

Author: zsxwing <[email protected]>

Closes apache#879 from zsxwing/SPARK-1925 and squashes the following commits:

5cf5a6d [zsxwing] SPARK-1925: Replace '&' with '&&'
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
JIRA: https://issues.apache.org/jira/browse/SPARK-1925

Author: zsxwing <[email protected]>

Closes apache#879 from zsxwing/SPARK-1925 and squashes the following commits:

5cf5a6d [zsxwing] SPARK-1925: Replace '&' with '&&'
wangyum added a commit that referenced this pull request May 26, 2023
…lan (#879)

* [SPARK-37199][SQL] Add deterministic field to QueryPlan

### What changes were proposed in this pull request?
We have a deterministic field in Expressions to check if an expression is deterministic, but we do not have a similar field in QueryPlan.

We have a need for such a check in the QueryPlan sometimes, like in InlineCTE

This proposal is to add a deterministic field to QueryPlan.

More details in this document: https://docs.google.com/document/d/1eIiaSJf-Co2HhjsaQxFNGwUxobnHID4ZGmJMcVytREc/edit#heading=h.4cz970y1mk93

### Why are the changes needed?

We have a need for such a check in the QueryPlan sometimes, like in InlineCTE

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Added unit tests

Closes #34470 from somani/isDeterministic.

Authored-by: Abhishek Somani <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>

(cherry picked from commit fe41d18)

* Fix test error

Co-authored-by: Abhishek Somani <[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.

4 participants