jduo commented on code in PR #1593:
URL: https://github.com/apache/arrow-adbc/pull/1593#discussion_r1526602922
##########
csharp/test/Drivers/Interop/Snowflake/SnowflakeTestingUtils.cs:
##########
@@ -133,11 +133,11 @@ SnowflakeTestConfiguration testConfiguration
/// Parses the queries from resources/SnowflakeData.sql
/// </summary>
/// <param name="testConfiguration"><see
cref="SnowflakeTestConfiguration"/></param>
- internal static string[] GetQueries(SnowflakeTestConfiguration
testConfiguration)
+ internal static string[] GetQueries(SnowflakeTestConfiguration
testConfiguration, string sqlFilePath = "resources/SnowflakeData.sql")
{
StringBuilder content = new StringBuilder();
- string[] sql = File.ReadAllLines("resources/SnowflakeData.sql");
+ string[] sql = File.ReadAllLines(sqlFilePath);
Review Comment:
I guess this is fine, but can we consider making this file an embedded test
resource instead of read from the file system? This is currently sensitive to
the what the current working directory is.
##########
csharp/test/Apache.Arrow.Adbc.Tests/Metadata/GetObjectsParser.cs:
##########
@@ -161,26 +161,34 @@ private static List<AdbcConstraint>
ParseConstraints(StructArray constraintsArra
StringArray name =
(StringArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_name")]; // constraint_name | utf8
StringArray type =
(StringArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_type")]; // constraint_type | utf8 not null
- ListArray column_names =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_names")]; // constraint_column_names |
list<utf8> not null
- ListArray column_usage =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_usage")]; // constraint_column_usage |
list<USAGE_SCHEMA>
+ ListArray columnNames =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_names")]; // constraint_column_names |
list<utf8> not null
+ ListArray columnUsages =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_usage")]; // constraint_column_usage |
list<USAGE_SCHEMA>
for (int i = 0; i < constraintsArray.Length; i++)
{
AdbcConstraint c = new AdbcConstraint();
c.Name = name.GetString(i);
c.Type = type.GetString(i);
- StringArray col_names = column_names.GetSlicedValues(i) as
StringArray;
- StructArray usage = column_usage.GetSlicedValues(i) as
StructArray;
+ StringArray colNames = columnNames.GetSlicedValues(i) as
StringArray;
+ StructArray usages = columnUsages.GetSlicedValues(i) as
StructArray;
- if (usage != null)
+ if (colNames != null)
{
- for (int j = 0; j < usage.Length; j++)
+ for (int j = 0; j < colNames.Length; j++)
{
- StringArray fkCatalog =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_catalog")]; // fk_catalog | utf8
- StringArray fkDbSchema =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_db_schema")]; //fk_db_schema | utf8
- StringArray fkTable =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_table")]; // fk_table | utf8 not null
- StringArray fkColumnName =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_column_name")]; // fk_column_name | utf8 not null
+ c.ColumnNames.Add(colNames.GetString(j));
+ }
+ }
+
+ if (usages != null)
+ {
+ for (int j = 0; j < usages.Length; j++)
Review Comment:
This for loop looks like it should start after all these Find() calls since
they don't depend on j -- we are redundantly running them on each iteration.
Additionally we should add an assertion that FindIndex() never returns -1.
##########
csharp/test/Apache.Arrow.Adbc.Tests/Metadata/GetObjectsParser.cs:
##########
@@ -161,26 +161,34 @@ private static List<AdbcConstraint>
ParseConstraints(StructArray constraintsArra
StringArray name =
(StringArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_name")]; // constraint_name | utf8
StringArray type =
(StringArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_type")]; // constraint_type | utf8 not null
- ListArray column_names =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_names")]; // constraint_column_names |
list<utf8> not null
- ListArray column_usage =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_usage")]; // constraint_column_usage |
list<USAGE_SCHEMA>
+ ListArray columnNames =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_names")]; // constraint_column_names |
list<utf8> not null
+ ListArray columnUsages =
(ListArray)constraintsArray.Fields[StandardSchemas.ConstraintSchema.FindIndex(f
=> f.Name == "constraint_column_usage")]; // constraint_column_usage |
list<USAGE_SCHEMA>
for (int i = 0; i < constraintsArray.Length; i++)
{
AdbcConstraint c = new AdbcConstraint();
c.Name = name.GetString(i);
c.Type = type.GetString(i);
- StringArray col_names = column_names.GetSlicedValues(i) as
StringArray;
- StructArray usage = column_usage.GetSlicedValues(i) as
StructArray;
+ StringArray colNames = columnNames.GetSlicedValues(i) as
StringArray;
+ StructArray usages = columnUsages.GetSlicedValues(i) as
StructArray;
- if (usage != null)
+ if (colNames != null)
{
- for (int j = 0; j < usage.Length; j++)
+ for (int j = 0; j < colNames.Length; j++)
{
- StringArray fkCatalog =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_catalog")]; // fk_catalog | utf8
- StringArray fkDbSchema =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_db_schema")]; //fk_db_schema | utf8
- StringArray fkTable =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_table")]; // fk_table | utf8 not null
- StringArray fkColumnName =
(StringArray)usage.Fields[StandardSchemas.UsageSchema.FindIndex(f => f.Name ==
"fk_column_name")]; // fk_column_name | utf8 not null
+ c.ColumnNames.Add(colNames.GetString(j));
+ }
+ }
+
+ if (usages != null)
+ {
+ for (int j = 0; j < usages.Length; j++)
Review Comment:
Let's examine if this can be applied elsewhere.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
