zeroshade commented on code in PR #2481:
URL: https://github.com/apache/arrow-adbc/pull/2481#discussion_r1925825041
##########
go/adbc/driver/flightsql/flightsql_adbc_server_test.go:
##########
@@ -2014,72 +2195,114 @@ func (suite *GetObjectsTests)
TestMetadataGetObjectsColumnsXdbc() {
for tblIdx := tblIdxStart; tblIdx <
tblIdxEnd; tblIdx++ {
tableName :=
dbSchemaTables.Field(0).(*array.String).Value(int(tblIdx))
- if
strings.EqualFold(schemaName, suite.Quirks.DBSchema()) &&
strings.EqualFold("bulk_ingest", tableName) {
+ if
strings.EqualFold(schemaName, suite.schemaName) &&
strings.EqualFold(suite.tableName, tableName) {
foundExpected = true
colIdxStart, colIdxEnd
:= tableColumnsList.ValueOffsets(int(tblIdx))
for colIdx :=
colIdxStart; colIdx < colIdxEnd; colIdx++ {
name :=
tableColumns.Field(0).(*array.String).Value(int(colIdx))
- colnames =
append(colnames, strings.ToLower(name))
+ columnName =
append(columnName, name)
- pos :=
tableColumns.Field(1).(*array.Int32).Value(int(colIdx))
- positions =
append(positions, strconv.Itoa(int(pos)))
+ // pos :=
tableColumns.Field(1).(*array.Int32).Value(int(colIdx))
+ //
ordinalPosition = append(ordinalPosition, strconv.Itoa(int(pos)))
- comments =
append(comments, tableColumns.Field(2).(*array.String).Value(int(colIdx)))
+ rm :=
tableColumns.Field(2).(*array.String).Value(int(colIdx))
+ remarks =
append(remarks, name+"_"+rm)
xdt :=
tableColumns.Field(3).(*array.Int16).Value(int(colIdx))
- xdbcDataTypes =
append(xdbcDataTypes, strconv.Itoa(int(xdt)))
+ xdbcDataType =
append(xdbcDataType, name+"_"+strconv.Itoa(int(xdt)))
dataType :=
tableColumns.Field(4).(*array.String).Value(int(colIdx))
- dataTypes =
append(dataTypes, dataType)
- xdbcTypeNames =
append(xdbcTypeNames, dataType)
+ xdbcTypeName =
append(xdbcTypeName, name+"_"+dataType)
- // these are
column size attributes used for either precision for numbers OR the length for
text
-
maxLenOrPrecision := tableColumns.Field(5).(*array.Int32).Value(int(colIdx))
- xdbcCharMaxLens
= append(xdbcCharMaxLens, strconv.Itoa(int(maxLenOrPrecision)))
+ columnSize :=
tableColumns.Field(5).(*array.Int32).Value(int(colIdx))
+ xdbcColumnSize
= append(xdbcColumnSize, name+"_"+strconv.Itoa(int(columnSize)))
- scale :=
tableColumns.Field(6).(*array.Int16).Value(int(colIdx))
- xdbcScales =
append(xdbcScales, strconv.Itoa(int(scale)))
+ decimalDigits
:= tableColumns.Field(6).(*array.Int16).Value(int(colIdx))
+
xdbcDecimalDigits = append(xdbcDecimalDigits,
name+"_"+strconv.Itoa(int(decimalDigits)))
- radix :=
tableColumns.Field(7).(*array.Int16).Value(int(colIdx))
-
xdbcNumPrecRadixs = append(xdbcNumPrecRadixs, strconv.Itoa(int(radix)))
+ numPrecRadix :=
tableColumns.Field(7).(*array.Int16).Value(int(colIdx))
+
xdbcNumPrecRadix = append(xdbcNumPrecRadix,
name+"_"+strconv.Itoa(int(numPrecRadix)))
- isnull :=
tableColumns.Field(8).(*array.Int16).Value(int(colIdx))
- xdbcNullables =
append(xdbcNullables, strconv.Itoa(int(isnull)))
+ nullable :=
tableColumns.Field(8).(*array.Int16).Value(int(colIdx))
+ xdbcNullable =
append(xdbcNullable, name+"_"+strconv.Itoa(int(nullable)))
+
+ columnDef :=
tableColumns.Field(9).(*array.String).Value(int(colIdx))
+ xdbcColumnDef =
append(xdbcColumnDef, name+"_"+columnDef)
sqlType :=
tableColumns.Field(10).(*array.Int16).Value(int(colIdx))
-
xdbcSqlDataTypes = append(xdbcSqlDataTypes, strconv.Itoa(int(sqlType)))
+ xdbcSqlDataType
= append(xdbcSqlDataType, name+"_"+strconv.Itoa(int(sqlType)))
dtPrec :=
tableColumns.Field(11).(*array.Int16).Value(int(colIdx))
- xdbcDateTimeSub
= append(xdbcSqlDataTypes, strconv.Itoa(int(dtPrec)))
+ xdbcDatetimeSub
= append(xdbcDatetimeSub, name+"_"+strconv.Itoa(int(dtPrec)))
charOctetLen :=
tableColumns.Field(12).(*array.Int32).Value(int(colIdx))
-
xdbcCharOctetLen = append(xdbcCharOctetLen, strconv.Itoa(int(charOctetLen)))
+
xdbcCharOctetLength = append(xdbcCharOctetLength,
name+"_"+strconv.Itoa(int(charOctetLen)))
+
+ isNullable :=
tableColumns.Field(13).(*array.String).Value(int(colIdx))
+ xdbcIsNullable
= append(xdbcIsNullable, name+"_"+isNullable)
- xdbcIsNullables
= append(xdbcIsNullables,
tableColumns.Field(13).(*array.String).Value(int(colIdx)))
+ scopeCatalog :=
tableColumns.Field(14).(*array.String).Value(int(colIdx))
+
xdbcScopeCatalog = append(xdbcScopeCatalog, name+"_"+scopeCatalog)
+
+ scopeSchema :=
tableColumns.Field(15).(*array.String).Value(int(colIdx))
+ xdbcScopeSchema
= append(xdbcScopeSchema, name+"_"+scopeSchema)
+
+ scopeTable :=
tableColumns.Field(16).(*array.String).Value(int(colIdx))
+ xdbcScopeTable
= append(xdbcScopeTable, name+"_"+scopeTable)
+
+ isAutoIncrement
:= tableColumns.Field(17).(*array.Boolean).Value(int(colIdx))
+
xdbcIsAutoincrement = append(xdbcIsAutoincrement,
name+"_"+strconv.FormatBool(isAutoIncrement))
+
+ isAutoGenerated
:= tableColumns.Field(18).(*array.Boolean).Value(int(colIdx))
+
xdbcIsAutogeneratedColumn = append(xdbcIsAutogeneratedColumn,
name+"_"+strconv.FormatBool(isAutoGenerated))
}
}
}
}
}
+ sort.Strings(columnName)
+ sort.Strings(remarks)
+ sort.Strings(xdbcDataType)
+ sort.Strings(xdbcTypeName)
+ sort.Strings(xdbcColumnSize)
+ sort.Strings(xdbcDecimalDigits)
+ sort.Strings(xdbcNumPrecRadix)
+ sort.Strings(xdbcNullable)
+ sort.Strings(xdbcColumnDef)
+ sort.Strings(xdbcSqlDataType)
+ sort.Strings(xdbcDatetimeSub)
+ sort.Strings(xdbcCharOctetLength)
+ sort.Strings(xdbcIsNullable)
+ sort.Strings(xdbcScopeCatalog)
+ sort.Strings(xdbcScopeSchema)
+ sort.Strings(xdbcScopeTable)
+ sort.Strings(xdbcIsAutoincrement)
+ sort.Strings(xdbcIsAutogeneratedColumn)
+
suite.False(rdr.Next())
suite.True(foundExpected)
- suite.Equal(tt.colnames, colnames) //
colNames
- suite.Equal(tt.positions, positions) //
positions
- suite.Equal(tt.comments, comments) //
comments
- suite.Equal(tt.xdbcDataType, xdbcDataTypes) //
xdbcDataType
- suite.Equal(tt.dataTypes, dataTypes) //
dataTypes
- suite.Equal(tt.xdbcTypeName, xdbcTypeNames) //
xdbcTypeName
- suite.Equal(tt.xdbcCharMaxLen, xdbcCharMaxLens) //
xdbcCharMaxLen
- suite.Equal(tt.xdbcScale, xdbcScales) //
xdbcScale
- suite.Equal(tt.xdbcNumPrecRadix, xdbcNumPrecRadixs) //
xdbcNumPrecRadix
- suite.Equal(tt.xdbcNullable, xdbcNullables) //
xdbcNullable
- suite.Equal(tt.xdbcSqlDataType, xdbcSqlDataTypes) //
xdbcSqlDataType
- suite.Equal(tt.xdbcDateTimeSub, xdbcDateTimeSub) //
xdbcDateTimeSub
- suite.Equal(tt.xdbcCharOctetLen, xdbcCharOctetLen) //
xdbcCharOctetLen
- suite.Equal(tt.xdbcIsNullable, xdbcIsNullables) //
xdbcIsNullable
+ suite.Equal(tt.columnName, columnName, "columnName")
+ //suite.Equal(tt.ordinalPosition, ordinalPosition,
"ordinalPosition")
Review Comment:
why are we losing the ordinal positions for comparison?
##########
go/adbc/driver/internal/shared_utils.go:
##########
@@ -247,9 +219,117 @@ func (g *GetObjects) Init(mem memory.Allocator, getObj
GetObjDBSchemasFn, getTbl
g.columnUsageTableBuilder =
g.constraintColumnUsageItems.FieldBuilder(2).(*array.StringBuilder)
g.columnUsageColumnBuilder =
g.constraintColumnUsageItems.FieldBuilder(3).(*array.StringBuilder)
+ g.metadataBuilders = MetadataToBuilders{}
+ g.metadataBuilders[REMARKS] = g.tableColumnsItems.FieldBuilder(2)
+ g.metadataBuilders[XDBC_DATA_TYPE] = g.tableColumnsItems.FieldBuilder(3)
+ g.metadataBuilders[XDBC_TYPE_NAME] = g.tableColumnsItems.FieldBuilder(4)
+ g.metadataBuilders[XDBC_COLUMN_SIZE] =
g.tableColumnsItems.FieldBuilder(5)
+ g.metadataBuilders[XDBC_DECIMAL_DIGITS] =
g.tableColumnsItems.FieldBuilder(6)
+ g.metadataBuilders[XDBC_NUM_PREC_RADIX] =
g.tableColumnsItems.FieldBuilder(7)
+ g.metadataBuilders[XDBC_NULLABLE] = g.tableColumnsItems.FieldBuilder(8)
+ g.metadataBuilders[XDBC_COLUMN_DEF] =
g.tableColumnsItems.FieldBuilder(9)
+ g.metadataBuilders[XDBC_SQL_DATA_TYPE] =
g.tableColumnsItems.FieldBuilder(10)
+ g.metadataBuilders[XDBC_DATETIME_SUB] =
g.tableColumnsItems.FieldBuilder(11)
+ g.metadataBuilders[XDBC_CHAR_OCTET_LENGTH] =
g.tableColumnsItems.FieldBuilder(12)
+ g.metadataBuilders[XDBC_IS_NULLABLE] =
g.tableColumnsItems.FieldBuilder(13)
+ g.metadataBuilders[XDBC_SCOPE_CATALOG] =
g.tableColumnsItems.FieldBuilder(14)
+ g.metadataBuilders[XDBC_SCOPE_SCHEMA] =
g.tableColumnsItems.FieldBuilder(15)
+ g.metadataBuilders[XDBC_SCOPE_TABLE] =
g.tableColumnsItems.FieldBuilder(16)
+ g.metadataBuilders[XDBC_IS_AUTOINCREMENT] =
g.tableColumnsItems.FieldBuilder(17)
+ g.metadataBuilders[XDBC_IS_AUTOGENERATEDCOLUMN] =
g.tableColumnsItems.FieldBuilder(18)
+
+ g.metadataHandlers = g.initMetadataHandlers(overrideMetadataHandlers)
+
return nil
}
+func parseColumnMetadata(key string, column arrow.Field, builder
array.Builder) {
+ if column.HasMetadata() {
+ if value, ok := column.Metadata.GetValue(key); ok {
+ var parsedInt int64
+ var parsedBool bool
+ var err error
+ switch b := builder.(type) {
+ case *array.Int16Builder:
+ parsedInt, err = strconv.ParseInt(value, 10, 16)
+ if err != nil {
+ b.AppendNull()
+ } else {
+ b.Append(int16(parsedInt))
+ }
+ case *array.Int32Builder:
+ parsedInt, err = strconv.ParseInt(value, 10, 32)
+ if err != nil {
+ b.AppendNull()
+ } else {
+ b.Append(int32(parsedInt))
+ }
+ case *array.Int64Builder:
+ parsedInt, err = strconv.ParseInt(value, 10, 64)
+ if err != nil {
+ b.AppendNull()
+ } else {
+ b.Append(parsedInt)
+ }
+ case *array.BooleanBuilder:
+ parsedBool, err = strconv.ParseBool(value)
+ if err != nil {
+ b.AppendNull()
+ } else {
+ b.Append(parsedBool)
+ }
+ case *array.StringBuilder:
+ b.Append(value)
+ default:
+ b.AppendNull()
+ }
+ } else {
+ builder.AppendNull()
+ }
+ } else {
+ builder.AppendNull()
+ }
+}
+
+func (g *GetObjects) initMetadataHandlers(overrides MetadataToHandlers)
MetadataToHandlers {
+
+ createHandler := func(key string) MetadataHandlers {
+ return func(column arrow.Field, builder array.Builder) {
+ parseColumnMetadata(key, column, builder)
+ }
+ }
+
+ metadataHandlers := make(MetadataToHandlers)
+
+ checkOverrideOrDefault := func(key string) {
+ if overrides[key] != nil {
+ metadataHandlers[key] = overrides[key]
+ } else {
+ metadataHandlers[key] = createHandler(key)
+ }
+ }
+
+ checkOverrideOrDefault(REMARKS)
+ checkOverrideOrDefault(XDBC_DATA_TYPE)
+ checkOverrideOrDefault(XDBC_TYPE_NAME)
+ checkOverrideOrDefault(XDBC_COLUMN_SIZE)
+ checkOverrideOrDefault(XDBC_DECIMAL_DIGITS)
+ checkOverrideOrDefault(XDBC_NUM_PREC_RADIX)
+ checkOverrideOrDefault(XDBC_NULLABLE)
+ checkOverrideOrDefault(XDBC_COLUMN_DEF)
+ checkOverrideOrDefault(XDBC_SQL_DATA_TYPE)
+ checkOverrideOrDefault(XDBC_DATETIME_SUB)
+ checkOverrideOrDefault(XDBC_CHAR_OCTET_LENGTH)
+ checkOverrideOrDefault(XDBC_IS_NULLABLE)
+ checkOverrideOrDefault(XDBC_SCOPE_CATALOG)
+ checkOverrideOrDefault(XDBC_SCOPE_SCHEMA)
+ checkOverrideOrDefault(XDBC_SCOPE_TABLE)
+ checkOverrideOrDefault(XDBC_IS_AUTOINCREMENT)
+ checkOverrideOrDefault(XDBC_IS_AUTOGENERATEDCOLUMN)
+
+ return metadataHandlers
Review Comment:
rather than a map of overrides / defaults / builders, could we instead use
an interface that can be implemented (similar to the Java implementation) where
we could pass `flightsql.ColumnMetadata` and guarantee the types without having
to manually implement the parsing from strings in here. For instance, some
drivers may have this metadata already typed, so it's wasteful to force a
conversion to string and then back to something else.
##########
go/adbc/driver/flightsql/flightsql_adbc_server_test.go:
##########
@@ -2014,72 +2195,114 @@ func (suite *GetObjectsTests)
TestMetadataGetObjectsColumnsXdbc() {
for tblIdx := tblIdxStart; tblIdx <
tblIdxEnd; tblIdx++ {
tableName :=
dbSchemaTables.Field(0).(*array.String).Value(int(tblIdx))
- if
strings.EqualFold(schemaName, suite.Quirks.DBSchema()) &&
strings.EqualFold("bulk_ingest", tableName) {
+ if
strings.EqualFold(schemaName, suite.schemaName) &&
strings.EqualFold(suite.tableName, tableName) {
foundExpected = true
colIdxStart, colIdxEnd
:= tableColumnsList.ValueOffsets(int(tblIdx))
for colIdx :=
colIdxStart; colIdx < colIdxEnd; colIdx++ {
name :=
tableColumns.Field(0).(*array.String).Value(int(colIdx))
- colnames =
append(colnames, strings.ToLower(name))
+ columnName =
append(columnName, name)
- pos :=
tableColumns.Field(1).(*array.Int32).Value(int(colIdx))
- positions =
append(positions, strconv.Itoa(int(pos)))
+ // pos :=
tableColumns.Field(1).(*array.Int32).Value(int(colIdx))
+ //
ordinalPosition = append(ordinalPosition, strconv.Itoa(int(pos)))
- comments =
append(comments, tableColumns.Field(2).(*array.String).Value(int(colIdx)))
+ rm :=
tableColumns.Field(2).(*array.String).Value(int(colIdx))
+ remarks =
append(remarks, name+"_"+rm)
xdt :=
tableColumns.Field(3).(*array.Int16).Value(int(colIdx))
- xdbcDataTypes =
append(xdbcDataTypes, strconv.Itoa(int(xdt)))
+ xdbcDataType =
append(xdbcDataType, name+"_"+strconv.Itoa(int(xdt)))
dataType :=
tableColumns.Field(4).(*array.String).Value(int(colIdx))
- dataTypes =
append(dataTypes, dataType)
- xdbcTypeNames =
append(xdbcTypeNames, dataType)
+ xdbcTypeName =
append(xdbcTypeName, name+"_"+dataType)
- // these are
column size attributes used for either precision for numbers OR the length for
text
-
maxLenOrPrecision := tableColumns.Field(5).(*array.Int32).Value(int(colIdx))
- xdbcCharMaxLens
= append(xdbcCharMaxLens, strconv.Itoa(int(maxLenOrPrecision)))
+ columnSize :=
tableColumns.Field(5).(*array.Int32).Value(int(colIdx))
+ xdbcColumnSize
= append(xdbcColumnSize, name+"_"+strconv.Itoa(int(columnSize)))
- scale :=
tableColumns.Field(6).(*array.Int16).Value(int(colIdx))
- xdbcScales =
append(xdbcScales, strconv.Itoa(int(scale)))
+ decimalDigits
:= tableColumns.Field(6).(*array.Int16).Value(int(colIdx))
+
xdbcDecimalDigits = append(xdbcDecimalDigits,
name+"_"+strconv.Itoa(int(decimalDigits)))
- radix :=
tableColumns.Field(7).(*array.Int16).Value(int(colIdx))
-
xdbcNumPrecRadixs = append(xdbcNumPrecRadixs, strconv.Itoa(int(radix)))
+ numPrecRadix :=
tableColumns.Field(7).(*array.Int16).Value(int(colIdx))
+
xdbcNumPrecRadix = append(xdbcNumPrecRadix,
name+"_"+strconv.Itoa(int(numPrecRadix)))
- isnull :=
tableColumns.Field(8).(*array.Int16).Value(int(colIdx))
- xdbcNullables =
append(xdbcNullables, strconv.Itoa(int(isnull)))
+ nullable :=
tableColumns.Field(8).(*array.Int16).Value(int(colIdx))
+ xdbcNullable =
append(xdbcNullable, name+"_"+strconv.Itoa(int(nullable)))
+
+ columnDef :=
tableColumns.Field(9).(*array.String).Value(int(colIdx))
+ xdbcColumnDef =
append(xdbcColumnDef, name+"_"+columnDef)
sqlType :=
tableColumns.Field(10).(*array.Int16).Value(int(colIdx))
-
xdbcSqlDataTypes = append(xdbcSqlDataTypes, strconv.Itoa(int(sqlType)))
+ xdbcSqlDataType
= append(xdbcSqlDataType, name+"_"+strconv.Itoa(int(sqlType)))
dtPrec :=
tableColumns.Field(11).(*array.Int16).Value(int(colIdx))
- xdbcDateTimeSub
= append(xdbcSqlDataTypes, strconv.Itoa(int(dtPrec)))
+ xdbcDatetimeSub
= append(xdbcDatetimeSub, name+"_"+strconv.Itoa(int(dtPrec)))
charOctetLen :=
tableColumns.Field(12).(*array.Int32).Value(int(colIdx))
-
xdbcCharOctetLen = append(xdbcCharOctetLen, strconv.Itoa(int(charOctetLen)))
+
xdbcCharOctetLength = append(xdbcCharOctetLength,
name+"_"+strconv.Itoa(int(charOctetLen)))
+
+ isNullable :=
tableColumns.Field(13).(*array.String).Value(int(colIdx))
+ xdbcIsNullable
= append(xdbcIsNullable, name+"_"+isNullable)
- xdbcIsNullables
= append(xdbcIsNullables,
tableColumns.Field(13).(*array.String).Value(int(colIdx)))
+ scopeCatalog :=
tableColumns.Field(14).(*array.String).Value(int(colIdx))
+
xdbcScopeCatalog = append(xdbcScopeCatalog, name+"_"+scopeCatalog)
+
+ scopeSchema :=
tableColumns.Field(15).(*array.String).Value(int(colIdx))
+ xdbcScopeSchema
= append(xdbcScopeSchema, name+"_"+scopeSchema)
+
+ scopeTable :=
tableColumns.Field(16).(*array.String).Value(int(colIdx))
+ xdbcScopeTable
= append(xdbcScopeTable, name+"_"+scopeTable)
+
+ isAutoIncrement
:= tableColumns.Field(17).(*array.Boolean).Value(int(colIdx))
+
xdbcIsAutoincrement = append(xdbcIsAutoincrement,
name+"_"+strconv.FormatBool(isAutoIncrement))
+
+ isAutoGenerated
:= tableColumns.Field(18).(*array.Boolean).Value(int(colIdx))
+
xdbcIsAutogeneratedColumn = append(xdbcIsAutogeneratedColumn,
name+"_"+strconv.FormatBool(isAutoGenerated))
}
}
}
}
}
+ sort.Strings(columnName)
+ sort.Strings(remarks)
+ sort.Strings(xdbcDataType)
+ sort.Strings(xdbcTypeName)
+ sort.Strings(xdbcColumnSize)
+ sort.Strings(xdbcDecimalDigits)
+ sort.Strings(xdbcNumPrecRadix)
+ sort.Strings(xdbcNullable)
+ sort.Strings(xdbcColumnDef)
+ sort.Strings(xdbcSqlDataType)
+ sort.Strings(xdbcDatetimeSub)
+ sort.Strings(xdbcCharOctetLength)
+ sort.Strings(xdbcIsNullable)
+ sort.Strings(xdbcScopeCatalog)
+ sort.Strings(xdbcScopeSchema)
+ sort.Strings(xdbcScopeTable)
+ sort.Strings(xdbcIsAutoincrement)
+ sort.Strings(xdbcIsAutogeneratedColumn)
Review Comment:
why do we need to sort these? Could we use `suite.ElementsMatch` which will
perform the assertion without requiring the orders to match?
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -57,10 +58,90 @@ type connectionImpl struct {
supportInfo support
}
+func stringMetadataHandler(xdbc_key string, flight_key string)
func(arrow.Field, array.Builder) {
+ return func(column arrow.Field, builder array.Builder) {
+ if column.HasMetadata() {
+ md := column.Metadata
+ b := builder.(*array.StringBuilder)
+ if typeName, ok := md.GetValue(flight_key); ok {
+ b.Append(typeName)
+ } else if typeName, ok = md.GetValue(xdbc_key); ok {
+ b.Append(typeName)
+ } else {
+ b.AppendNull()
+ }
+ } else {
+ builder.AppendNull()
+ }
+ }
+}
Review Comment:
instead of this, could we leverage
https://pkg.go.dev/github.com/apache/arrow-go/v18/arrow/flight/flightsql#ColumnMetadata
to handle the type management and the metadata values?
--
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]