Skip to content

Commit

Permalink
fix(c/driver/common): Object name matching handles shared prefix case…
Browse files Browse the repository at this point in the history 
… correctly (#1168)

Fixes #1100 

Test before fix:
```
Expected equality of these values:
  AdbcGetObjectsDataGetTableByName(&mock_data, "mock_catalog", "mock_schema", "table_suffix")
    Which is: 0x16d014ee8
  &mock_table_suffix
    Which is: 0x16d014ea8
arrow-adbc/c/driver/common/utils_test.cc:220: Failure
Expected equality of these values:
  AdbcGetObjectsDataGetColumnByName(&mock_data, "mock_catalog", "mock_schema", "table", "column_suffix")
    Which is: 0x16d014df8
  &mock_column_suffix
    Which is: 0x16d014d48
arrow-adbc/c/driver/common/utils_test.cc:224: Failure
Expected equality of these values:
  AdbcGetObjectsDataGetConstraintByName(&mock_data, "mock_catalog", "mock_schema", "table", "constraint_suffix")
    Which is: 0x16d014d08
  &mock_constraint_suffix
    Which is: 0x16d014cc8
[  FAILED  ] AdbcGetObjectsData.GetObjectsByName (0 ms)
```

Test after fix:
```
$ ctest                                                                           
Test project arrow-adbc/build
    Start 1: adbc-driver-common-test
1/2 Test #1: adbc-driver-common-test ..........   Passed    0.25 sec
    Start 2: adbc-driver-sqlite-test
2/2 Test #2: adbc-driver-sqlite-test ..........   Passed    0.19 sec

100% tests passed, 0 tests failed out of 2

Label Time Summary:
driver-common    =   0.25 sec*proc (1 test)
driver-sqlite    =   0.19 sec*proc (1 test)
unittest         =   0.43 sec*proc (2 tests)

Total Test time (real) =   0.44 sec
```
  • Loading branch information
@ywc88
ywc88 authored Oct 5, 2023
1 parent 059ce57 commit e419d86
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 10 deletions.
21 changes: 11 additions & 10 deletions c/driver/common/utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ struct AdbcErrorDetails {
int capacity;
};

bool StringViewEquals(struct ArrowStringView stringView, const char* str) {
int64_t len = (int64_t)strlen(str);
return len == stringView.size_bytes &&
!strncmp(stringView.data, str, stringView.size_bytes);
}

static void ReleaseErrorWithDetails(struct AdbcError* error) {
struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data;
free(details->message);
Expand Down Expand Up @@ -1006,8 +1012,7 @@ struct AdbcGetObjectsCatalog* AdbcGetObjectsDataGetCatalogByName(
if (catalog_name != NULL) {
for (int64_t i = 0; i < get_objects_data->n_catalogs; i++) {
struct AdbcGetObjectsCatalog* catalog = get_objects_data->catalogs[i];
struct ArrowStringView name = catalog->catalog_name;
if (!strncmp(name.data, catalog_name, name.size_bytes)) {
if (StringViewEquals(catalog->catalog_name, catalog_name)) {
return catalog;
}
}
Expand All @@ -1025,8 +1030,7 @@ struct AdbcGetObjectsSchema* AdbcGetObjectsDataGetSchemaByName(
if (catalog != NULL) {
for (int64_t i = 0; i < catalog->n_db_schemas; i++) {
struct AdbcGetObjectsSchema* schema = catalog->catalog_db_schemas[i];
struct ArrowStringView name = schema->db_schema_name;
if (!strncmp(name.data, schema_name, name.size_bytes)) {
if (StringViewEquals(schema->db_schema_name, schema_name)) {
return schema;
}
}
Expand All @@ -1045,8 +1049,7 @@ struct AdbcGetObjectsTable* AdbcGetObjectsDataGetTableByName(
if (schema != NULL) {
for (int64_t i = 0; i < schema->n_db_schema_tables; i++) {
struct AdbcGetObjectsTable* table = schema->db_schema_tables[i];
struct ArrowStringView name = table->table_name;
if (!strncmp(name.data, table_name, name.size_bytes)) {
if (StringViewEquals(table->table_name, table_name)) {
return table;
}
}
Expand All @@ -1066,8 +1069,7 @@ struct AdbcGetObjectsColumn* AdbcGetObjectsDataGetColumnByName(
if (table != NULL) {
for (int64_t i = 0; i < table->n_table_columns; i++) {
struct AdbcGetObjectsColumn* column = table->table_columns[i];
struct ArrowStringView name = column->column_name;
if (!strncmp(name.data, column_name, name.size_bytes)) {
if (StringViewEquals(column->column_name, column_name)) {
return column;
}
}
Expand All @@ -1087,8 +1089,7 @@ struct AdbcGetObjectsConstraint* AdbcGetObjectsDataGetConstraintByName(
if (table != NULL) {
for (int64_t i = 0; i < table->n_table_constraints; i++) {
struct AdbcGetObjectsConstraint* constraint = table->table_constraints[i];
struct ArrowStringView name = constraint->constraint_name;
if (!strncmp(name.data, constraint_name, name.size_bytes)) {
if (StringViewEquals(constraint->constraint_name, constraint_name)) {
return constraint;
}
}
Expand Down
81 changes: 81 additions & 0 deletions c/driver/common/utils_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,84 @@ TEST(ErrorDetails, RoundTripValues) {

error.release(&error);
}

// https://github.com/apache/arrow-adbc/issues/1100
TEST(AdbcGetObjectsData, GetObjectsByName) {
// Mock objects where the names being compared share the same prefix
struct AdbcGetObjectsData mock_data;
struct AdbcGetObjectsCatalog mock_catalog;
struct AdbcGetObjectsSchema mock_schema;
struct AdbcGetObjectsTable mock_table, mock_table_suffix;
struct AdbcGetObjectsColumn mock_column, mock_column_suffix;
struct AdbcGetObjectsConstraint mock_constraint, mock_constraint_suffix;

mock_catalog.catalog_name = {"mock_catalog", (int64_t)strlen("mock_catalog")};
mock_schema.db_schema_name = {"mock_schema", (int64_t)strlen("mock_schema")};
mock_table.table_name = {"table", (int64_t)strlen("table")};
mock_table_suffix.table_name = {"table_suffix", (int64_t)strlen("table_suffix")};
mock_column.column_name = {"column", (int64_t)strlen("column")};
mock_column_suffix.column_name = {"column_suffix", (int64_t)strlen("column_suffix")};
mock_constraint.constraint_name = {"constraint", (int64_t)strlen("constraint")};
mock_constraint_suffix.constraint_name = {"constraint_suffix",
(int64_t)strlen("constraint_suffix")};

struct AdbcGetObjectsCatalog* catalogs[] = {&mock_catalog};
mock_data.catalogs = catalogs;
mock_data.n_catalogs = 1;

struct AdbcGetObjectsSchema* schemas[] = {&mock_schema};
mock_catalog.catalog_db_schemas = schemas;
mock_catalog.n_db_schemas = 1;

struct AdbcGetObjectsTable* tables[] = {&mock_table, &mock_table_suffix};
mock_schema.db_schema_tables = tables;
mock_schema.n_db_schema_tables = 2;

struct AdbcGetObjectsColumn* columns[] = {&mock_column, &mock_column_suffix};
mock_table.table_columns = columns;
mock_table.n_table_columns = 2;

struct AdbcGetObjectsConstraint* constraints[] = {&mock_constraint,
&mock_constraint_suffix};
mock_table.table_constraints = constraints;
mock_table.n_table_constraints = 2;

EXPECT_EQ(AdbcGetObjectsDataGetTableByName(&mock_data, "mock_catalog", "mock_schema",
"table"),
&mock_table);
EXPECT_EQ(AdbcGetObjectsDataGetTableByName(&mock_data, "mock_catalog", "mock_schema",
"table_suffix"),
&mock_table_suffix);
EXPECT_EQ(AdbcGetObjectsDataGetTableByName(&mock_data, "mock_catalog", "mock_schema",
"nonexistent"),
nullptr);

EXPECT_EQ(AdbcGetObjectsDataGetCatalogByName(&mock_data, "mock_catalog"),
&mock_catalog);
EXPECT_EQ(AdbcGetObjectsDataGetCatalogByName(&mock_data, "nonexistent"), nullptr);

EXPECT_EQ(AdbcGetObjectsDataGetSchemaByName(&mock_data, "mock_catalog", "mock_schema"),
&mock_schema);
EXPECT_EQ(AdbcGetObjectsDataGetSchemaByName(&mock_data, "mock_catalog", "nonexistent"),
nullptr);

EXPECT_EQ(AdbcGetObjectsDataGetColumnByName(&mock_data, "mock_catalog", "mock_schema",
"table", "column"),
&mock_column);
EXPECT_EQ(AdbcGetObjectsDataGetColumnByName(&mock_data, "mock_catalog", "mock_schema",
"table", "column_suffix"),
&mock_column_suffix);
EXPECT_EQ(AdbcGetObjectsDataGetColumnByName(&mock_data, "mock_catalog", "mock_schema",
"table", "nonexistent"),
nullptr);

EXPECT_EQ(AdbcGetObjectsDataGetConstraintByName(&mock_data, "mock_catalog",
"mock_schema", "table", "constraint"),
&mock_constraint);
EXPECT_EQ(AdbcGetObjectsDataGetConstraintByName(
&mock_data, "mock_catalog", "mock_schema", "table", "constraint_suffix"),
&mock_constraint_suffix);
EXPECT_EQ(AdbcGetObjectsDataGetConstraintByName(&mock_data, "mock_catalog",
"mock_schema", "table", "nonexistent"),
nullptr);
}

0 comments on commit e419d86

Please sign in to comment.