ryan-syed commented on code in PR #1338:
URL: https://github.com/apache/arrow-adbc/pull/1338#discussion_r1419556778
##########
csharp/test/Drivers/Snowflake/DriverTests.cs:
##########
@@ -279,17 +279,51 @@ public void CanGetObjectsAll()
{
IEnumerable<AdbcColumn> highPrecisionColumns = columns.Where(c
=> c.XdbcTypeName == "NUMBER");
- if(highPrecisionColumns.Count() > 0)
+ if (highPrecisionColumns.Count() > 0)
{
// ensure they all are coming back as
XdbcDataType_XDBC_DECIMAL because they are Decimal128
short XdbcDataType_XDBC_DECIMAL = 3;
- IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
+ IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
int count = invalidHighPrecisionColumns.Count();
Assert.True(count == 0, $"There are {count} columns that
do not map to the correct XdbcSqlDataType when UseHighPrecision=true");
}
}
}
+ /// <summary>
+ /// Validates if the driver can call GetObjects with GetObjectsDepth
as Tables with TableName as a Special Character.
+ /// </summary>
+ [SkippableFact, Order(3)]
+ public void CanGetObjectsTablesWithSpecialCharacter()
+ {
+ string databaseName = _testConfiguration.Metadata.Catalog;
+ string schemaName = _testConfiguration.Metadata.Schema;
+ string tableName = _testConfiguration.Metadata.Table;
+
+ using IArrowArrayStream stream = _connection.GetObjects(
+ depth: AdbcConnection.GetObjectsDepth.Tables,
+ catalogPattern: databaseName,
+ dbSchemaPattern: schemaName,
+ tableNamePattern: tableName,
+ tableTypes: new List<string> { "BASE TABLE", "VIEW" },
+ columnNamePattern: null);
+
+ using RecordBatch recordBatch =
stream.ReadNextRecordBatchAsync().Result;
+
+ List<AdbcCatalog> catalogs =
GetObjectsParser.ParseCatalog(recordBatch, databaseName, schemaName);
+
+ List<AdbcTable> tables = catalogs
+ .Where(s => s.Name.Equals(databaseName))
+ .Select(s => s.DbSchemas)
Review Comment:
We probably need to add `WHERE` filtering for `schemaName` and `tableName`
too as they can contain wild card characters and using only `FirstOrDefault`
could end up with a potential different indentifier.
##########
go/adbc/driver/snowflake/connection.go:
##########
@@ -281,11 +281,14 @@ func (c *cnxn) getObjectsDbSchemas(ctx context.Context,
depth adbc.ObjectDepth,
}
conditions := make([]string, 0)
+ queryArgs := make([]interface{}, 0, 2)
Review Comment:
Is there a specific reason to use `interface` here, when the type is know to
be `string`?
##########
csharp/test/Drivers/Snowflake/DriverTests.cs:
##########
@@ -279,17 +279,51 @@ public void CanGetObjectsAll()
{
IEnumerable<AdbcColumn> highPrecisionColumns = columns.Where(c
=> c.XdbcTypeName == "NUMBER");
- if(highPrecisionColumns.Count() > 0)
+ if (highPrecisionColumns.Count() > 0)
{
// ensure they all are coming back as
XdbcDataType_XDBC_DECIMAL because they are Decimal128
short XdbcDataType_XDBC_DECIMAL = 3;
- IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
+ IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
int count = invalidHighPrecisionColumns.Count();
Assert.True(count == 0, $"There are {count} columns that
do not map to the correct XdbcSqlDataType when UseHighPrecision=true");
}
}
}
+ /// <summary>
+ /// Validates if the driver can call GetObjects with GetObjectsDepth
as Tables with TableName as a Special Character.
+ /// </summary>
+ [SkippableFact, Order(3)]
+ public void CanGetObjectsTablesWithSpecialCharacter()
+ {
+ string databaseName = _testConfiguration.Metadata.Catalog;
+ string schemaName = _testConfiguration.Metadata.Schema;
+ string tableName = _testConfiguration.Metadata.Table;
+
+ using IArrowArrayStream stream = _connection.GetObjects(
+ depth: AdbcConnection.GetObjectsDepth.Tables,
+ catalogPattern: databaseName,
+ dbSchemaPattern: schemaName,
+ tableNamePattern: tableName,
+ tableTypes: new List<string> { "BASE TABLE", "VIEW" },
+ columnNamePattern: null);
+
+ using RecordBatch recordBatch =
stream.ReadNextRecordBatchAsync().Result;
+
+ List<AdbcCatalog> catalogs =
GetObjectsParser.ParseCatalog(recordBatch, databaseName, schemaName);
+
+ List<AdbcTable> tables = catalogs
+ .Where(s => s.Name.Equals(databaseName))
+ .Select(s => s.DbSchemas)
Review Comment:
Additionally special characters can be present not just in tables, and
therefore, consider adding tests for schemas and catalogs with special
characters.
##########
go/adbc/driver/snowflake/connection.go:
##########
@@ -828,7 +836,7 @@ func (c *cnxn) GetTableSchema(ctx context.Context, catalog
*string, dbSchema *st
if dbSchema != nil {
tblParts = append(tblParts,
strconv.Quote(strings.ToUpper(*dbSchema)))
}
- tblParts = append(tblParts, strconv.Quote(strings.ToUpper(tableName)))
Review Comment:
Should `string.ToUpper` conversion be avoided for catalog and schema name
too?
##########
csharp/test/Drivers/Snowflake/DriverTests.cs:
##########
@@ -279,17 +279,51 @@ public void CanGetObjectsAll()
{
IEnumerable<AdbcColumn> highPrecisionColumns = columns.Where(c
=> c.XdbcTypeName == "NUMBER");
- if(highPrecisionColumns.Count() > 0)
+ if (highPrecisionColumns.Count() > 0)
{
// ensure they all are coming back as
XdbcDataType_XDBC_DECIMAL because they are Decimal128
short XdbcDataType_XDBC_DECIMAL = 3;
- IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
+ IEnumerable<AdbcColumn> invalidHighPrecisionColumns =
highPrecisionColumns.Where(c => c.XdbcSqlDataType != XdbcDataType_XDBC_DECIMAL);
int count = invalidHighPrecisionColumns.Count();
Assert.True(count == 0, $"There are {count} columns that
do not map to the correct XdbcSqlDataType when UseHighPrecision=true");
}
}
}
+ /// <summary>
+ /// Validates if the driver can call GetObjects with GetObjectsDepth
as Tables with TableName as a Special Character.
+ /// </summary>
+ [SkippableFact, Order(3)]
Review Comment:
Current tests seems to rely on the names set in the config file. Should we
create temporary tables, dbs, schemas etc. with special characters and tests
for those scenarios for better reliability?
--
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]
