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-8235][SPARK-8236][SQL] misc functions: sha1/sha, crc32 #6970

Closed
wants to merge 7 commits into from

Conversation

qiansl127
Copy link
Contributor

@AmplabJenkins
Copy link

Can one of the admins verify this patch?


override def dataType: DataType = StringType

override def expectedChildTypes: Seq[DataType] = Seq(BinaryType, StringType)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not right, should be Seq(BinaryType)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure. I already implemented the sha/sha1 (#6963). @davies mentioned that Seq(BinaryType, StringType) is the way to go, because it should support BinaryType and StringType.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tarekauel Sorry, I was wrong, didn't figure out the difference between checkInputDataTypes and expectedChildTypes of ExpectsInputTypes.

In order to get better error message, we should use checkInputDataTypes.

expectedChildTypes is used to widen the types we support.

For this case, we could use Seq(BinaryType) as expectedChildTypes, which means it will also support StringType, because StringType could be casted to BinaryType (which is cheap).

In eval and genCode, you can expect that child.dataType is BinaryType.

@chenghao-intel
Copy link
Contributor

LGTM
@rxin can you trigger the unit test?

@SparkQA
Copy link

SparkQA commented Jun 24, 2015

Test build #959 has started for PR 6970 at commit 69f18a7.

@SparkQA
Copy link

SparkQA commented Jun 24, 2015

Test build #959 has finished for PR 6970 at commit 69f18a7.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class Sha(child: Expression)
    • case class Crc32(child: Expression)

checksum.update(value.asInstanceOf[Array[Byte]], 0, value.asInstanceOf[Array[Byte]].length)
checksum.getValue
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add codegen support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davies Done!

Copy link

Choose a reason for hiding this comment

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

NAVER - http://www.naver.com/

sujkh@naver.com 님께 보내신 메일 <Re: [spark] [SPARK-8235][SPARK-8236][SQL] misc functions: sha1/sha, crc32 (#6970)> 이 다음과 같은 이유로 전송 실패했습니다.


받는 사람이 회원님의 메일을 수신차단 하였습니다.


@davies
Copy link
Contributor

davies commented Jun 26, 2015

LGTM, Could you rebase it to master?

@qiansl127
Copy link
Contributor Author

@davies Rebase done! Please help to check it.

@tarekbecker
Copy link
Contributor

@qiansl127
I guess this one can be closed: #6963 (sha1) #7108 (crc32)

@qiansl127
Copy link
Contributor Author

Yes, please close this PR.

@qiansl127 qiansl127 closed this Jul 1, 2015
@davies
Copy link
Contributor

davies commented Jul 1, 2015

Forget to follow up this one, sorry. Next time, it will be better if you guys could comment on the JIRA to avoid crush each other, thanks you all the hard work, anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants