Skip to content

Add bulk insert support to SQL Server connector#12176

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
jirassimok:sqlserver-bulk-writes
May 6, 2022
Merged

Add bulk insert support to SQL Server connector#12176
ebyhr merged 1 commit intotrinodb:masterfrom
jirassimok:sqlserver-bulk-writes

Conversation

@jirassimok
Copy link
Copy Markdown
Member

@jirassimok jirassimok commented Apr 28, 2022

This allows SQL Server's bulk insert feature to be used when inserting data in Trino.

This feature was originally written for the Starburst Enterprise Platform, and is now being contributed to Trino.

This PR still needs the corresponding documentation changes.

@cla-bot cla-bot bot added the cla-signed label Apr 28, 2022
@jirassimok jirassimok force-pushed the sqlserver-bulk-writes branch from 1995ed6 to 0e1b7ca Compare April 28, 2022 14:32
@anusudarsan
Copy link
Copy Markdown
Member

LGTM

@jirassimok jirassimok force-pushed the sqlserver-bulk-writes branch from 0e1b7ca to 0216c46 Compare April 28, 2022 16:20
@jirassimok jirassimok requested a review from ssheikin April 28, 2022 16:22
@mosabua mosabua added needs-docs This pull request requires changes to the documentation docs labels Apr 28, 2022
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Apr 28, 2022

Can you please add the doc segment as well.. or alternatively we can add a separate PR to be merged at the same time .. let me know what you prefer

@jirassimok
Copy link
Copy Markdown
Member Author

jirassimok commented Apr 28, 2022

Can you please add the doc segment as well

I'll add them.

@jirassimok jirassimok force-pushed the sqlserver-bulk-writes branch 2 times, most recently from ba920b0 to 7b348c3 Compare April 29, 2022 14:44
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Apr 29, 2022

In terms of the docs we might have to see how this combines with either this stuff

https://docs.starburst.io/latest/connector/starburst-sqlserver.html#insert

or the new docs merged with

#12178

Also ... we should probably do the refactor of that property as well before we get this merge .. or even in here.. cc @hashhar @ebyhr @martint

@ebyhr ebyhr removed the needs-docs This pull request requires changes to the documentation label Apr 30, 2022
Copy link
Copy Markdown
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Please bind session properties class in SqlServerClientModule.

bindSessionPropertiesProvider(binder, SqlServerSessionProperties.class);

@jirassimok jirassimok force-pushed the sqlserver-bulk-writes branch 2 times, most recently from 5f8b708 to 0eb5b90 Compare May 3, 2022 19:32
Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Docs look good to me.

@jirassimok jirassimok requested a review from ebyhr May 4, 2022 21:11
@jirassimok jirassimok force-pushed the sqlserver-bulk-writes branch from 0eb5b90 to 011a854 Compare May 4, 2022 22:05
Copy link
Copy Markdown
Contributor

@ssheikin ssheikin left a comment

Choose a reason for hiding this comment

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

lgtm.
I think you could squash commits.

Co-Authored-By: Sasha Sheikin <myminitrue@gmail.com>
@ebyhr ebyhr force-pushed the sqlserver-bulk-writes branch from 011a854 to 65e6059 Compare May 6, 2022 00:29
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented May 6, 2022

Just squashed commits into one.

@ebyhr ebyhr merged commit 7ca52ff into trinodb:master May 6, 2022
@github-actions github-actions bot added this to the 380 milestone May 6, 2022
@ebyhr ebyhr mentioned this pull request May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants