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

Fix incorrect results when FTE and bulk INSERT is enabled in SQL Server #14926

Merged
merged 2 commits into from
Nov 7, 2022

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Nov 7, 2022

Description

Fixes #14856

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# SQL Server
* Fix incorrect results when non transactional INSERT is disabled and bulk INSERT is enabled. ({issue}`14856`)

This is helpful to debug the generated INSERT statement.
@cla-bot cla-bot bot added the cla-signed label Nov 7, 2022
@ebyhr ebyhr requested review from losipiuk, findepi and mwd410 November 7, 2022 09:02
@ebyhr
Copy link
Member Author

ebyhr commented Nov 7, 2022

Looking CI failures.

@ebyhr ebyhr marked this pull request as draft November 7, 2022 09:17
@ebyhr ebyhr force-pushed the ebi/sqlserver-bulk-insert-null branch from 1e5c322 to c5364e1 Compare November 7, 2022 09:40
Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

LGTM. Does new test cover that?

@ebyhr
Copy link
Member Author

ebyhr commented Nov 7, 2022

Yes, the new assertion failed without this change.

Column orders between INSERT statement and table definition
must be same when using bulk insert in SQL Server.
@ebyhr ebyhr force-pushed the ebi/sqlserver-bulk-insert-null branch from c5364e1 to f90f773 Compare November 7, 2022 11:10
@ebyhr ebyhr marked this pull request as ready for review November 7, 2022 11:48
@ebyhr
Copy link
Member Author

ebyhr commented Nov 7, 2022

@losipiuk I pushed small change. Could you take another look?

Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

Thx

@ebyhr ebyhr merged commit 2e702c0 into trinodb:master Nov 7, 2022
@ebyhr ebyhr deleted the ebi/sqlserver-bulk-insert-null branch November 7, 2022 13:38
@ebyhr ebyhr mentioned this pull request Nov 7, 2022
@github-actions github-actions bot added this to the 403 milestone Nov 7, 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.

Trino loading table with all nulls if bulk copy setting enabled for sql server connector
2 participants