Skip to content

Conversation

@zhulipeng
Copy link
Contributor

What changes were proposed in this pull request?

Change the binary type mapping from default blob to varbinary(max) for mssql server.
https://docs.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-2017
image

How was this patch tested?

Unit test.

@srowen
Copy link
Member

srowen commented Mar 14, 2019

What's the difference in SQL Server? I'm just wondering if there are any compatibility problems with older versions, or whether there's an upside to changing the type?

@dongjoon-hyun
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Mar 15, 2019

Test build #103523 has finished for PR 24091 at commit fb6493e.

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

@zhulipeng
Copy link
Contributor Author

zhulipeng commented Mar 15, 2019

What's the difference in SQL Server? I'm just wondering if there are any compatibility problems with older versions, or whether there's an upside to changing the type?

What's the difference in SQL Server? I'm just wondering if there are any compatibility problems with older versions, or whether there's an upside to changing the type?

@srowen This is a bug fix for writing binary data back to MsSql Server.
#24099

image

@SparkQA
Copy link

SparkQA commented Mar 15, 2019

Test build #103527 has finished for PR 24091 at commit ebe4b93.

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

@srowen
Copy link
Member

srowen commented Mar 15, 2019

What is this test that is failing -- something you have added locally? Looks OK though. Indeed BLOB doesn't seem to be supported.

@zhulipeng
Copy link
Contributor Author

What is this test that is failing -- something you have added locally? Looks OK though. Indeed BLOB doesn't seem to be supported.

Yes, I just add a unit test to read/write Mssql Server through jdbc.
image

I have added this test case to MsSql server docker integration test suite in another PR. #24099

@srowen
Copy link
Member

srowen commented Mar 16, 2019

Merged to master

@srowen srowen closed this in 8ee09f2 Mar 16, 2019
shivsood pushed a commit to shivsood/spark that referenced this pull request Jul 24, 2019
## What changes were proposed in this pull request?

Change the binary type mapping from default blob to varbinary(max) for mssql server.
https://docs.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-2017
![image](https://user-images.githubusercontent.com/698621/54351715-0e8c8780-468b-11e9-8931-7ecb85c5ad6b.png)

## How was this patch tested?

Unit test.

Closes apache#24091 from lipzhu/SPARK-27159.

Authored-by: Zhu, Lipeng <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@dongjoon-hyun
Copy link
Member

Hi, @lipzhu and @srowen .
I'll backport this bugfix to branch-2.4.

I did a clean cherry-pick to branch-2.4 and tested this locally.

Thank you, @lipzhu and @srowen .

dongjoon-hyun pushed a commit that referenced this pull request Jul 24, 2019
## What changes were proposed in this pull request?

Change the binary type mapping from default blob to varbinary(max) for mssql server.
https://docs.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-2017
![image](https://user-images.githubusercontent.com/698621/54351715-0e8c8780-468b-11e9-8931-7ecb85c5ad6b.png)

## How was this patch tested?

Unit test.

Closes #24091 from lipzhu/SPARK-27159.

Authored-by: Zhu, Lipeng <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
rluta pushed a commit to rluta/spark that referenced this pull request Sep 17, 2019
## What changes were proposed in this pull request?

Change the binary type mapping from default blob to varbinary(max) for mssql server.
https://docs.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-2017
![image](https://user-images.githubusercontent.com/698621/54351715-0e8c8780-468b-11e9-8931-7ecb85c5ad6b.png)

## How was this patch tested?

Unit test.

Closes apache#24091 from lipzhu/SPARK-27159.

Authored-by: Zhu, Lipeng <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Sep 26, 2019
## What changes were proposed in this pull request?

Change the binary type mapping from default blob to varbinary(max) for mssql server.
https://docs.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-2017
![image](https://user-images.githubusercontent.com/698621/54351715-0e8c8780-468b-11e9-8931-7ecb85c5ad6b.png)

## How was this patch tested?

Unit test.

Closes apache#24091 from lipzhu/SPARK-27159.

Authored-by: Zhu, Lipeng <[email protected]>
Signed-off-by: Sean Owen <[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