Skip to content

Fixes #891 - Bulk Insert for NVARCHAR(MAX) Fields.#901

Merged
willmorgan merged 11 commits intotediousjs:masterfrom
JP1016:patch-1
Aug 7, 2019
Merged

Fixes #891 - Bulk Insert for NVARCHAR(MAX) Fields.#901
willmorgan merged 11 commits intotediousjs:masterfrom
JP1016:patch-1

Conversation

@JP1016
Copy link
Contributor

@JP1016 JP1016 commented Aug 2, 2019

What this does:
Fixes #891

Adds the ability to set length for an NVARCHAR(MAX) column, which was breaking while bulk insertion.

What this does:
Fixes tediousjs#891

Adds the ability to set length for an NVARCHAR(MAX) column, which was breaking while bulk insertion.
dhensby
dhensby previously approved these changes Aug 2, 2019
Copy link
Collaborator

@dhensby dhensby left a comment

Choose a reason for hiding this comment

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

Thanks for this - ping @willmorgan for any thoughts

Copy link
Collaborator

@willmorgan willmorgan left a comment

Choose a reason for hiding this comment

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

Looks fine, needs tests though.

Updated options to check the property length

Co-Authored-By: Will Morgan <willmorgan@users.noreply.github.com>
@JP1016
Copy link
Contributor Author

JP1016 commented Aug 5, 2019

thanks @willmorgan , i will add the tests soon

Copy link
Contributor Author

@JP1016 JP1016 left a comment

Choose a reason for hiding this comment

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

@willmorgan @dhensby Added tests and refactored code to align with standard js styles

Copy link
Collaborator

@willmorgan willmorgan left a comment

Choose a reason for hiding this comment

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

Getting there @JP1016, just a few questions to answer and maybe a few more test scenarios then I think we're good to go. Thanks!

Copy link
Contributor Author

@JP1016 JP1016 left a comment

Choose a reason for hiding this comment

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

@willmorgan @darryl-davidson Updated test cases with length: infinity, max, undefined and random string value

Copy link
Contributor Author

@JP1016 JP1016 left a comment

Choose a reason for hiding this comment

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

Made changes accordingly.

Copy link
Contributor Author

@JP1016 JP1016 left a comment

Choose a reason for hiding this comment

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

Added test cases for the scenarios

Copy link
Collaborator

@willmorgan willmorgan left a comment

Choose a reason for hiding this comment

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

Nice.

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.

Unable to bulk insert when there is an nvarchar(max) column in the table

3 participants