ryan-syed commented on code in PR #1455:
URL: https://github.com/apache/arrow-adbc/pull/1455#discussion_r1449590543
##########
go/adbc/driver/snowflake/connection.go:
##########
@@ -640,7 +639,8 @@ func (c *cnxn) getColumnsMetadata(ctx context.Context,
matchingCatalogNames []st
err = rows.Scan(&data.TblType, &data.Dbname, &data.Schema,
&data.TblName, &data.ColName,
&data.OrdinalPos, &data.IsNullable, &data.DataType,
&data.NumericPrec,
&data.NumericPrecRadix, &data.NumericScale,
&data.IsIdent, &data.IdentGen,
- &data.IdentIncrement, &data.CharMaxLength,
&data.CharOctetLength, &data.DatetimePrec, &data.Comment)
+ &data.IdentIncrement, &data.CharMaxLength,
&data.CharOctetLength, &data.DatetimePrec, &data.Comment,
+ &data.ConstraintName, &data.ConstraintType)
Review Comment:
[TableInfo
struct](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/internal/shared_utils.go#L38C1-L41C2)
doesn't currently capture table constraints.
Those might need to be captured and appended to
[tableConstraintsBuilder](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/internal/shared_utils.go#L269C1-L270C40)
which seems to be marked as unimplemented.
##########
go/adbc/driver/snowflake/connection_test.go:
##########
@@ -276,30 +320,45 @@ func TestPrepareColumnsSQL(t *testing.T) {
tableType := [2]string{"BASE TABLE", "VIEW"}
expected := `SELECT table_type, table_catalog, table_schema,
table_name, column_name,
- ordinal_position,
is_nullable::boolean, data_type, numeric_precision,
- numeric_precision_radix,
numeric_scale, is_identity::boolean,
- identity_generation,
identity_increment,
- character_maximum_length,
character_octet_length, datetime_precision, comment
- FROM
- (
- SELECT T.table_type, C.*
- FROM
-
"DEMO_DB".INFORMATION_SCHEMA.TABLES AS T
- JOIN
-
"DEMO_DB".INFORMATION_SCHEMA.COLUMNS AS C
- ON
- T.table_catalog
= C.table_catalog AND T.table_schema = C.table_schema AND t.table_name =
C.table_name
- UNION ALL
- SELECT T.table_type, C.*
- FROM
-
"DEMOADB".INFORMATION_SCHEMA.TABLES AS T
- JOIN
-
"DEMOADB".INFORMATION_SCHEMA.COLUMNS AS C
- ON
- T.table_catalog
= C.table_catalog AND T.table_schema = C.table_schema AND t.table_name =
C.table_name
- )
- WHERE TABLE_CATALOG ILIKE ?
AND TABLE_SCHEMA ILIKE ? AND TABLE_NAME ILIKE ? AND COLUMN_NAME ILIKE ? AND
TABLE_TYPE IN ('BASE TABLE','VIEW')
- ORDER BY table_catalog,
table_schema, table_name, ordinal_position`
+ ordinal_position, is_nullable::boolean, data_type, numeric_precision,
+ numeric_precision_radix, numeric_scale, is_identity::boolean,
+ identity_generation, identity_increment,
+ character_maximum_length, character_octet_length, datetime_precision,
comment,
+ constraint_name, constraint_type FROM (SELECT T.table_type,
+ C.*,
+ K.constraint_name, K.constraint_type
+ FROM
+ "DEMO_DB".INFORMATION_SCHEMA.TABLES AS T
+ JOIN
+ "DEMO_DB".INFORMATION_SCHEMA.COLUMNS AS C
+ ON
+ T.table_catalog = C.table_catalog
+ AND T.table_schema = C.table_schema
+ AND T.table_name = C.table_name
+ LEFT OUTER JOIN
+ "DEMO_DB".INFORMATION_SCHEMA.TABLE_CONSTRAINTS AS K
+ ON
+ T.table_catalog = K.table_catalog
+ AND T.table_schema = K.table_schema
+ AND T.table_name = K.table_name UNION ALL SELECT
T.table_type,
+ C.*,
+ K.constraint_name, K.constraint_type
Review Comment:
Consider adding a test similar to
[TestMetadataGetObjectsColumnsXdbc](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/snowflake/driver_test.go#L409)
to verify that the table constraints are returned correctly. Adding tests in
the CSharp layer would also work.
##########
go/adbc/driver/snowflake/connection.go:
##########
@@ -701,10 +701,16 @@ func prepareTablesSQL(matchingCatalogNames []string,
catalog *string, dbSchema *
if query != "" {
query += " UNION ALL "
}
- query += `SELECT * FROM "` + strings.ReplaceAll(catalog_name,
"\"", "\"\"") + `".INFORMATION_SCHEMA.TABLES`
+ query += `SELECT T.*, K.constraint_name, K.constraint_type FROM
"` + strings.ReplaceAll(catalog_name, "\"", "\"\"") +
`".INFORMATION_SCHEMA.TABLES AS T
Review Comment:
As per the schema, we would also need `constraint_column_names` and,
`constraint_column_usage`
[[
constraint_column_names | list<utf8> not null | (2)
constraint_column_usage | list<USAGE_SCHEMA> | (3)
](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/snowflake/connection.go#L210C1-L217C56)](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/snowflake/connection.go#L210C1-L217C56)
--
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]
