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 OPENROWSET with numeric codepage as string #69

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

lBilali
Copy link
Contributor

@lBilali lBilali commented Jan 31, 2024

This pull request fixes #63

since when codepage is a number no check is done if the value is a valid codepage this change does the same when the code page is a number as string

@lBilali
Copy link
Contributor Author

lBilali commented Jan 31, 2024

@microsoft-github-policy-service agree

if (vResult.OptionKind == BulkInsertOptionKind.CodePage) // Check for code page string constants
// Check for code page string constants.
// If codepage is int just use the value as is. Same as above when value is integerOrNumeric
if (vResult.OptionKind == BulkInsertOptionKind.CodePage && !int.TryParse(vValue.Value, out int value))
Copy link
Member

Choose a reason for hiding this comment

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

the change seems correct but I'm wondering how the value is parsed as int if it's with single quotes. can you please verify with test that this work if the value is a CODEPAGE = '65001'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a test case for CODEPAGE = '866' on the first commit but now I did one extra test case for CODEPAGE = '65001' as well
and both work

BTW I verifyed that it fails if there is a not number value other then 'ACP', 'OEM' and 'RAW' and it failes as expected.
NO test case added on that though

@llali llali merged commit 04708b2 into microsoft:main Feb 14, 2024
5 checks passed
@jashwood
Copy link

I'm still getting this in the latest version of VS. The OPENROWSET documentation clearly has single quotes around the value expected, making it look like it should expect a string

@dzsquared
Copy link
Contributor

You're right @jashwood, I'm also seeing an old version of scriptdom in VS. Checking with the team if the update can be picked into the next preview release.

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.

Codepage='65001' or any numeric value is throwing Incorrect Syntax error in OPENROWSET query
5 participants