You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "vleslief-ms (via GitHub)" <gi...@apache.org> on 2024/01/11 23:59:25 UTC

[PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

vleslief-ms opened a new pull request, #1455:
URL: https://github.com/apache/arrow-adbc/pull/1455

   When Snowflake Go driver calls GetObjects against tables and columns, it will now also return table constraints (name and type) if any are found related to the table.
   
   The tables and columns SQL queries were modified to include a left outer join against INFORMATION_SCHEMA.TABLE_CONSTRAINTS to retrieve constraint information. The metadata object also had to be updated to include the constraint fields.
   
   Pre-commit had to be updated as I could not commit until it was done, and the previous versions of the pre-commit dependencies kept returning errors when I tried to execute them.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "vleslief-ms (via GitHub)" <gi...@apache.org>.
vleslief-ms closed pull request #1455: feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects
URL: https://github.com/apache/arrow-adbc/pull/1455


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
ryan-syed commented on code in PR #1455:
URL: https://github.com/apache/arrow-adbc/pull/1455#discussion_r1449598871


##########
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:
   Adding a test similar to [TestMetadataGetObjectsColumnsXdbc](https://github.com/apache/arrow-adbc/blob/2afbbb7d4d471d21fe20eed976bf4c0b20151732/go/adbc/driver/snowflake/driver_test.go#L409) will be helpful to verify the table constraints are returned correctly. Adding tests in the CSharp layer would also work.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
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 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.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
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. 
   
   I couldn't locate them being 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.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(go/adbc/driver/snowflake): Add support for table constraints when calling GetObjects [arrow-adbc]

Posted by "ryan-syed (via GitHub)" <gi...@apache.org>.
ryan-syed commented on code in PR #1455:
URL: https://github.com/apache/arrow-adbc/pull/1455#discussion_r1449575911


##########
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, `constraint_column_names` and, `constraint_column_usage`	need to be included, however, I couldn't locate them in the queries.		
   [[
   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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org