zeroshade commented on code in PR #3239:
URL: https://github.com/apache/arrow-adbc/pull/3239#discussion_r2254738122
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
+ stream, err := cnxn.GetInfo(ctx, []InfoCode{
+ InfoDriverName,
+ InfoDriverVersion,
+ InfoDriverArrowVersion,
+ InfoDriverADBCVersion,
+ InfoVendorName,
+ InfoVendorVersion,
+ InfoVendorArrowVersion,
+ InfoVendorSql,
+ InfoVendorSubstrait,
+ InfoVendorSubstraitMinVersion,
+ InfoVendorSubstraitMaxVersion,
Review Comment:
Should some of these end up as first-class members instead of just being
pushed to the `LibraryInfo` map?
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
+ stream, err := cnxn.GetInfo(ctx, []InfoCode{
+ InfoDriverName,
+ InfoDriverVersion,
+ InfoDriverArrowVersion,
+ InfoDriverADBCVersion,
+ InfoVendorName,
+ InfoVendorVersion,
+ InfoVendorArrowVersion,
+ InfoVendorSql,
+ InfoVendorSubstrait,
+ InfoVendorSubstraitMinVersion,
+ InfoVendorSubstraitMaxVersion,
+ })
+ if err != nil {
+ return nil, fmt.Errorf("error during GetInfo: %w", err)
+ }
+ defer stream.Release()
+
+ var name, version string
+ libInfo := make(map[string]string)
+
+ for stream.Next() {
+ batch := stream.Record()
+ defer batch.Release()
+ codeArr := batch.Column(0).(*array.Uint32)
+ unionArr := batch.Column(1).(*array.DenseUnion)
+
+ for i := 0; i < int(batch.NumRows()); i++ {
+ code := InfoCode(codeArr.Value(i))
+ child := unionArr.Field(unionArr.ChildID(i))
+ offset := int(unionArr.ValueOffset(i))
+ unionArr.GetOneForMarshal(i)
Review Comment:
why are we calling this and then ignoring the return? remove this line?
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
+ stream, err := cnxn.GetInfo(ctx, []InfoCode{
+ InfoDriverName,
+ InfoDriverVersion,
+ InfoDriverArrowVersion,
+ InfoDriverADBCVersion,
+ InfoVendorName,
+ InfoVendorVersion,
+ InfoVendorArrowVersion,
+ InfoVendorSql,
+ InfoVendorSubstrait,
+ InfoVendorSubstraitMinVersion,
+ InfoVendorSubstraitMaxVersion,
+ })
+ if err != nil {
+ return nil, fmt.Errorf("error during GetInfo: %w", err)
+ }
+ defer stream.Release()
+
+ var name, version string
+ libInfo := make(map[string]string)
+
+ for stream.Next() {
+ batch := stream.Record()
+ defer batch.Release()
+ codeArr := batch.Column(0).(*array.Uint32)
+ unionArr := batch.Column(1).(*array.DenseUnion)
+
+ for i := 0; i < int(batch.NumRows()); i++ {
+ code := InfoCode(codeArr.Value(i))
+ child := unionArr.Field(unionArr.ChildID(i))
+ offset := int(unionArr.ValueOffset(i))
+ unionArr.GetOneForMarshal(i)
+
+ if child.IsNull(offset) {
+ continue
+ }
+
+ // Handle different union types based on child ID
(similar to validation tests)
+ switch unionArr.ChildID(i) {
+ case 0: // String values
+ if strArray, ok := child.(*array.String); ok {
+ // Create a copy of the string to avoid
memory issues
+ val :=
strings.Clone(strArray.Value(offset))
Review Comment:
might be worthwhile to expand on this comment to explain why there would be
memory issues that the GC doesn't handle.
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
+ stream, err := cnxn.GetInfo(ctx, []InfoCode{
+ InfoDriverName,
+ InfoDriverVersion,
+ InfoDriverArrowVersion,
+ InfoDriverADBCVersion,
+ InfoVendorName,
+ InfoVendorVersion,
+ InfoVendorArrowVersion,
+ InfoVendorSql,
+ InfoVendorSubstrait,
+ InfoVendorSubstraitMinVersion,
+ InfoVendorSubstraitMaxVersion,
+ })
+ if err != nil {
+ return nil, fmt.Errorf("error during GetInfo: %w", err)
+ }
+ defer stream.Release()
+
+ var name, version string
+ libInfo := make(map[string]string)
+
+ for stream.Next() {
+ batch := stream.Record()
+ defer batch.Release()
+ codeArr := batch.Column(0).(*array.Uint32)
+ unionArr := batch.Column(1).(*array.DenseUnion)
+
+ for i := 0; i < int(batch.NumRows()); i++ {
+ code := InfoCode(codeArr.Value(i))
+ child := unionArr.Field(unionArr.ChildID(i))
+ offset := int(unionArr.ValueOffset(i))
+ unionArr.GetOneForMarshal(i)
+
+ if child.IsNull(offset) {
+ continue
+ }
+
+ // Handle different union types based on child ID
(similar to validation tests)
+ switch unionArr.ChildID(i) {
+ case 0: // String values
+ if strArray, ok := child.(*array.String); ok {
+ // Create a copy of the string to avoid
memory issues
+ val :=
strings.Clone(strArray.Value(offset))
+
+ switch code {
+ case InfoDriverName:
+ name = val
+ case InfoDriverVersion:
+ version = val
+ case InfoDriverArrowVersion:
+ libInfo["driver_arrow_version"]
= val
+ case InfoVendorName:
+ libInfo["vendor_name"] = val
+ case InfoVendorVersion:
+ libInfo["vendor_version"] = val
+ case InfoVendorArrowVersion:
+ libInfo["vendor_arrow_version"]
= val
+ case InfoVendorSubstraitMinVersion:
+
libInfo["vendor_substrait_min_version"] = val
+ case InfoVendorSubstraitMaxVersion:
+
libInfo["vendor_substrait_max_version"] = val
Review Comment:
set constants for these keys?
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
Review Comment:
I might just return a `VersionInfo` not a pointer, it's small enough, maps
are reference types, and strings are just a pointer + len. So if this somehow
ends up in a loop, the heap allocations + GC would be worse than the copy
##########
go/adbc/ext.go:
##########
@@ -159,3 +160,136 @@ func IngestStream(ctx context.Context, cnxn Connection,
reader array.RecordReade
return count, nil
}
+
+// VersionInfo contains comprehensive driver and library version information.
+type VersionInfo struct {
+ DriverName string `json:"driver_name"` // e.g., "ADBC
PostgreSQL Driver"
+ DriverVersion string `json:"driver_version"` // e.g.,
"15.13.0000"
+ LibraryInfo map[string]string `json:"library_info"` // Additional
library versions, protocol info, etc.
+}
+
+// GetVersionInfo retrieves comprehensive driver version information from a
connection.
+// This helper function encapsulates the complex process of querying driver
info codes,
+// handling streaming record batches and union arrays, extracting and safely
cloning
+// string values, and managing errors with fallback defaults.
+//
+// Returns detailed version information including driver name, version, and
additional
+// library information such as Arrow version, vendor details, and ADBC version.
+//
+// This is not part of the ADBC API specification.
+func GetVersionInfo(ctx context.Context, cnxn Connection) (*VersionInfo,
error) {
+ stream, err := cnxn.GetInfo(ctx, []InfoCode{
+ InfoDriverName,
+ InfoDriverVersion,
+ InfoDriverArrowVersion,
+ InfoDriverADBCVersion,
+ InfoVendorName,
+ InfoVendorVersion,
+ InfoVendorArrowVersion,
+ InfoVendorSql,
+ InfoVendorSubstrait,
+ InfoVendorSubstraitMinVersion,
+ InfoVendorSubstraitMaxVersion,
+ })
+ if err != nil {
+ return nil, fmt.Errorf("error during GetInfo: %w", err)
+ }
+ defer stream.Release()
+
+ var name, version string
+ libInfo := make(map[string]string)
+
+ for stream.Next() {
+ batch := stream.Record()
+ defer batch.Release()
Review Comment:
this is unnecessary. If you don't call `Retain` on the record, then each
call to `Next` will release the previous one
--
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]
