-
Notifications
You must be signed in to change notification settings - Fork 144
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
Resolve the issue of Chinese truncation during varchar insertion #195
base: master
Are you sure you want to change the base?
Conversation
"unsafe" | ||
) | ||
|
||
func TestBufferLen_IsNull(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see that you added test as I requested here #182 (comment) , but I was asking for a different test. Your test does not prove that there is a problem with current code.
Perhaps if you adjust TestMSSQLTypes
Line 704 in bcbcb68
func TestMSSQLTypes(t *testing.T) { |
TestMSSQLTextColumnParamTypes
Line 1271 in bcbcb68
func TestMSSQLTextColumnParamTypes(t *testing.T) { |
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added test
Your test is wrong. For any Unicode values you should be using nvarchar instead of varchar type.
I just added this commit 25af894#diff-fa7d078a77ac77b6c32112f98183f4a221d30d0f460e50456b2ba995efaf0f23R675 with proper tests, and they all pass here.
I will not review your PR, because I don't believe it is required. Nothing is broken.
Alex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue does not occur in all databases, only some databases exist. I think setting the size to p.size is the correct approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue does not occur in all databases, only some databases exist.
If you have a way for me to reproduce the issue, I will look at it again.
I think setting the size to p.size is the correct approach
I disagree.
No description provided.