Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-8305] [SPARK-8190] [SQL] improve codegen #6755

Closed
wants to merge 3 commits into from

Conversation

davies
Copy link
Contributor

@davies davies commented Jun 11, 2015

This PR fix a few small issues about codgen:

  1. cast decimal to boolean
  2. do not inline literal with null
  3. improve SpecificRow.equals()
  4. test expressions with optimized express
  5. fix compare with BinaryType

cc @rxin @chenghao-intel

@SparkQA
Copy link

SparkQA commented Jun 11, 2015

Test build #34659 has finished for PR 6755 at commit 70b7dda.

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

return false;
}
for (int i = 0; i < n; i ++) {
if (isNullAt(i) != row.isNullAt(i) || !isNullAt(i) && !get(i).equals(row.get(i))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will that cause NPE for !get(i).equals(row.get(i))

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, ignore the above comment. But it's better to add a bracket for !isNullAt(i) && !get(i).equals(row.get(i))

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on adding parentheses.

@SparkQA
Copy link

SparkQA commented Jun 11, 2015

Test build #34664 has finished for PR 6755 at commit 6617ea6.

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

@davies davies changed the title [SPARK-8305] [SQL] improve codegen [SPARK-8305] [SPARK-8190] [SQL] improve codegen Jun 11, 2015
(c1, c3) => s"$c1 $symbol $c3"
if (ctx.isPrimitiveType(left.dataType)) {
// faster version
defineCodeGen(ctx, ev, {
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 just do

defineCodeGen(ctx, ev, (c1, c2) => s"$c1 $symbol $c2")

in one line for everything here?

@SparkQA
Copy link

SparkQA commented Jun 11, 2015

Test build #34699 has finished for PR 6755 at commit ef27343.

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

@asfgit asfgit closed this in 1191c3e Jun 11, 2015
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
This PR fix a few small issues about codgen:

1. cast decimal to boolean
2. do not inline literal with null
3. improve SpecificRow.equals()
4. test expressions with optimized express
5. fix compare with BinaryType

cc rxin chenghao-intel

Author: Davies Liu <davies@databricks.com>

Closes apache#6755 from davies/fix_codegen and squashes the following commits:

ef27343 [Davies Liu] address comments
6617ea6 [Davies Liu] fix scala tyle
70b7dda [Davies Liu] improve codegen
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