Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

BulkLoad: Fix some types #239

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

BulkLoad: Fix some types #239

wants to merge 5 commits into from

Conversation

dlapko
Copy link

@dlapko dlapko commented Mar 4, 2025

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 66.29213% with 30 lines in your changes missing coverage. Please review.

Project coverage is 74.98%. Comparing base (ba24acc) to head (6babea6).

Files with missing lines Patch % Lines
bulkcopy.go 60.29% 23 Missing and 4 partials ⚠️
types.go 85.71% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #239      +/-   ##
==========================================
+ Coverage   74.77%   74.98%   +0.21%     
==========================================
  Files          32       32              
  Lines        6457     6540      +83     
==========================================
+ Hits         4828     4904      +76     
- Misses       1338     1343       +5     
- Partials      291      293       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -578,6 +586,21 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata) interface{}
panic("shoulnd't get here")
}
func writeLongLenType(w io.Writer, ti typeInfo, buf []byte) (err error) {
if buf == nil {
// According to the documentation, we MUST NOT specify the text pointer and timestamp when the value is NULL.
Copy link
Collaborator

@shueybubbles shueybubbles Mar 4, 2025

Choose a reason for hiding this comment

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

would it be worthwhile to have a non-bulkinsert test case for this? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

@shueybubbles I don’t see any insert tests anywhere…
Besides, this code won’t run in non-bulkinsert scenario because:

func (s *Stmt) makeParam(val driver.Value) (res param, err error) {
	if val == nil {
		res.ti.TypeId = typeNull
		res.buffer = nil
		res.ti.Size = 0
		return
	}
        ...

And then

func writeTypeInfo(w io.Writer, ti *typeInfo, out bool, encoding msdsn.EncodeParameters) (err error) {
	err = binary.Write(w, binary.LittleEndian, ti.TypeId)
	if err != nil {
		return
	}
	switch ti.TypeId {
	case typeNull, typeInt1, typeBit, typeInt2, typeInt4, typeDateTim4,
		typeFlt4, typeMoney, typeDateTime, typeFlt8, typeMoney4, typeInt8:
		// those are fixed length
		// https://msdn.microsoft.com/en-us/library/dd341171.aspx
		ti.Writer = writeFixedType
	...

@dlapko
Copy link
Author

dlapko commented Mar 4, 2025

@microsoft-github-policy-service agree

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants