Copilot commented on code in PR #4318:
URL: https://github.com/apache/arrow-adbc/pull/4318#discussion_r3255553180
##########
csharp/src/Apache.Arrow.Adbc/C/CAdbcDriverExporter.cs:
##########
@@ -502,6 +518,63 @@ private unsafe static AdbcStatusCode
SetStatementSqlQuery(CAdbcStatement* native
}
}
+#if NET5_0_OR_GREATER
+ [UnmanagedCallersOnly]
+#endif
+ private unsafe static AdbcStatusCode
SetStatementOption(CAdbcStatement* nativeStatement, byte* name, byte* value,
CAdbcError* error)
+ {
+ try
+ {
+ if (name == null) throw new
ArgumentNullException(nameof(name));
+
+ GCHandle gch =
GCHandle.FromIntPtr((IntPtr)nativeStatement->private_data);
+ AdbcStatement stub = (AdbcStatement)gch.Target!;
+
+ stub.SetOption(MarshalExtensions.PtrToStringUTF8(name)!,
MarshalExtensions.PtrToStringUTF8(value)!);
+ return AdbcStatusCode.Success;
Review Comment:
`SetStatementOption` validates `name` but not `value`. If a driver manager
passes a null `value` pointer, this ends up calling `PtrToStringUTF8(value)!`
and then `stub.SetOption(..., null)` (or throws later), which is inconsistent
with other SetOption entry points (e.g. `ConnectionStub.SetOption` checks
`value == null`). Consider adding an explicit `if (value == null) throw new
ArgumentNullException(nameof(value));` (or mapping to an `AdbcException` with
`InvalidArgument`) so the exported ABI returns a predictable status/message.
--
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]
