Re: [PR] feat(go/adbc/sqldriver): handle timestamp/time.Time values for input [arrow-adbc]

2025-07-09 Thread via GitHub 


zeroshade merged PR #3109:
URL: https://github.com/apache/arrow-adbc/pull/3109


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] feat(go/adbc/sqldriver): handle timestamp/time.Time values for input [arrow-adbc]

2025-07-08 Thread via GitHub 


lidavidm commented on code in PR #3109:
URL: https://github.com/apache/arrow-adbc/pull/3109#discussion_r2193619691


##
go/adbc/sqldriver/driver.go:
##
@@ -459,15 +458,63 @@ func arrFromVal(val any) arrow.Array {
dt = arrow.PrimitiveTypes.Date64
buffers[1] = 
memory.NewBufferBytes((*[8]byte)(unsafe.Pointer(&v))[:])
case []byte:
-   dt = arrow.BinaryTypes.Binary
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   if dt == nil || dt.ID() == arrow.BINARY {
+   dt = arrow.BinaryTypes.Binary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_BINARY {
+   dt = arrow.BinaryTypes.LargeBinary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buffers = append(buffers, memory.NewBufferBytes(v))
case string:
-   dt = arrow.BinaryTypes.String
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
-
+   if dt == nil || dt.ID() == arrow.STRING {
+   dt = arrow.BinaryTypes.String
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_STRING {
+   dt = arrow.BinaryTypes.LargeString
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buf := unsafe.Slice(unsafe.StringData(v), len(v))
buffers = append(buffers, memory.NewBufferBytes(buf))
+   case arrow.Time32:
+   if dt == nil || dt.ID() != arrow.TIME32 {
+   panic(errors.New("can only create array from 
arrow.Time32 with known type"))

Review Comment:
   Right, I'm suggesting that the error message should say what they need to do



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] feat(go/adbc/sqldriver): handle timestamp/time.Time values for input [arrow-adbc]

2025-07-08 Thread via GitHub 


zeroshade commented on code in PR #3109:
URL: https://github.com/apache/arrow-adbc/pull/3109#discussion_r2193014302


##
go/adbc/sqldriver/driver.go:
##
@@ -459,15 +458,63 @@ func arrFromVal(val any) arrow.Array {
dt = arrow.PrimitiveTypes.Date64
buffers[1] = 
memory.NewBufferBytes((*[8]byte)(unsafe.Pointer(&v))[:])
case []byte:
-   dt = arrow.BinaryTypes.Binary
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   if dt == nil || dt.ID() == arrow.BINARY {
+   dt = arrow.BinaryTypes.Binary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_BINARY {
+   dt = arrow.BinaryTypes.LargeBinary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buffers = append(buffers, memory.NewBufferBytes(v))
case string:
-   dt = arrow.BinaryTypes.String
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
-
+   if dt == nil || dt.ID() == arrow.STRING {
+   dt = arrow.BinaryTypes.String
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_STRING {
+   dt = arrow.BinaryTypes.LargeString
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buf := unsafe.Slice(unsafe.StringData(v), len(v))
buffers = append(buffers, memory.NewBufferBytes(buf))
+   case arrow.Time32:
+   if dt == nil || dt.ID() != arrow.TIME32 {
+   panic(errors.New("can only create array from 
arrow.Time32 with known type"))

Review Comment:
   We would only run into the issue when they try to execute with the passed in 
params, not when it is prepared. The issue is if they pass an `arrow.Time32` 
when the server didn't provide a parameter schema, we don't know what type this 
should be (because of the time unit).
   
   That said, it's easy enough to propagate the error upwards instead of 
panicking, which i'll do.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] feat(go/adbc/sqldriver): handle timestamp/time.Time values for input [arrow-adbc]

2025-07-07 Thread via GitHub 


lidavidm commented on code in PR #3109:
URL: https://github.com/apache/arrow-adbc/pull/3109#discussion_r2191229174


##
go/adbc/sqldriver/driver.go:
##
@@ -459,15 +458,63 @@ func arrFromVal(val any) arrow.Array {
dt = arrow.PrimitiveTypes.Date64
buffers[1] = 
memory.NewBufferBytes((*[8]byte)(unsafe.Pointer(&v))[:])
case []byte:
-   dt = arrow.BinaryTypes.Binary
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   if dt == nil || dt.ID() == arrow.BINARY {
+   dt = arrow.BinaryTypes.Binary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_BINARY {
+   dt = arrow.BinaryTypes.LargeBinary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buffers = append(buffers, memory.NewBufferBytes(v))
case string:
-   dt = arrow.BinaryTypes.String
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
-
+   if dt == nil || dt.ID() == arrow.STRING {
+   dt = arrow.BinaryTypes.String
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_STRING {
+   dt = arrow.BinaryTypes.LargeString
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buf := unsafe.Slice(unsafe.StringData(v), len(v))
buffers = append(buffers, memory.NewBufferBytes(buf))
+   case arrow.Time32:
+   if dt == nil || dt.ID() != arrow.TIME32 {
+   panic(errors.New("can only create array from 
arrow.Time32 with known type"))

Review Comment:
   Instead of panicking, I think we should propagate the error upwards



##
go/adbc/sqldriver/driver.go:
##
@@ -459,15 +458,63 @@ func arrFromVal(val any) arrow.Array {
dt = arrow.PrimitiveTypes.Date64
buffers[1] = 
memory.NewBufferBytes((*[8]byte)(unsafe.Pointer(&v))[:])
case []byte:
-   dt = arrow.BinaryTypes.Binary
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   if dt == nil || dt.ID() == arrow.BINARY {
+   dt = arrow.BinaryTypes.Binary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_BINARY {
+   dt = arrow.BinaryTypes.LargeBinary
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buffers = append(buffers, memory.NewBufferBytes(v))
case string:
-   dt = arrow.BinaryTypes.String
-   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
-
+   if dt == nil || dt.ID() == arrow.STRING {
+   dt = arrow.BinaryTypes.String
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int32Traits.CastToBytes([]int32{0, int32(len(v))}))
+   } else if dt.ID() == arrow.LARGE_STRING {
+   dt = arrow.BinaryTypes.LargeString
+   buffers[1] = 
memory.NewBufferBytes(arrow.Int64Traits.CastToBytes([]int64{0, int64(len(v))}))
+   }
buf := unsafe.Slice(unsafe.StringData(v), len(v))
buffers = append(buffers, memory.NewBufferBytes(buf))
+   case arrow.Time32:
+   if dt == nil || dt.ID() != arrow.TIME32 {
+   panic(errors.New("can only create array from 
arrow.Time32 with known type"))

Review Comment:
   It might also be better to say something like "can only create array from 
... when statement is prepared and server provides parameter schema"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]