This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new d6255ac0a fix(c/driver/postgresql): Ensure schema ordering is
consisent and respects case sensitivity of table names (#2028)
d6255ac0a is described below
commit d6255ac0a10825f98846f112f13ffd0dc013e3f5
Author: Dewey Dunnington <[email protected]>
AuthorDate: Tue Jul 23 22:27:06 2024 -0300
fix(c/driver/postgresql): Ensure schema ordering is consisent and respects
case sensitivity of table names (#2028)
Fixes #2006.
---
c/driver/postgresql/connection.cc | 20 +++++++++------
c/driver/postgresql/postgresql_test.cc | 41 +++++++++++++++++++++++++++++++
c/validation/adbc_validation_statement.cc | 6 ++---
3 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/c/driver/postgresql/connection.cc
b/c/driver/postgresql/connection.cc
index e5e524beb..443e37aad 100644
--- a/c/driver/postgresql/connection.cc
+++ b/c/driver/postgresql/connection.cc
@@ -1146,19 +1146,25 @@ AdbcStatusCode PostgresConnection::GetTableSchema(const
char* catalog,
struct AdbcError* error) {
AdbcStatusCode final_status = ADBC_STATUS_OK;
+ char* quoted = PQescapeIdentifier(conn_, table_name, strlen(table_name));
+ std::string table_name_str(quoted);
+ PQfreemem(quoted);
+
+ if (db_schema != nullptr) {
+ quoted = PQescapeIdentifier(conn_, db_schema, strlen(db_schema));
+ table_name_str = std::string(quoted) + "." + table_name_str;
+ PQfreemem(quoted);
+ }
+
std::string query =
"SELECT attname, atttypid "
"FROM pg_catalog.pg_class AS cls "
"INNER JOIN pg_catalog.pg_attribute AS attr ON cls.oid = attr.attrelid "
"INNER JOIN pg_catalog.pg_type AS typ ON attr.atttypid = typ.oid "
- "WHERE attr.attnum >= 0 AND cls.oid = $1::regclass::oid";
+ "WHERE attr.attnum >= 0 AND cls.oid = $1::regclass::oid "
+ "ORDER BY attr.attnum";
- std::vector<std::string> params;
- if (db_schema != nullptr) {
- params.push_back(std::string(db_schema) + "." + table_name);
- } else {
- params.push_back(table_name);
- }
+ std::vector<std::string> params = {table_name_str};
PqResultHelper result_helper =
PqResultHelper{conn_, std::string(query.c_str()), params, error};
diff --git a/c/driver/postgresql/postgresql_test.cc
b/c/driver/postgresql/postgresql_test.cc
index 48a15d693..ab95d3e65 100644
--- a/c/driver/postgresql/postgresql_test.cc
+++ b/c/driver/postgresql/postgresql_test.cc
@@ -673,6 +673,47 @@ TEST_F(PostgresConnectionTest, MetadataSetCurrentDbSchema)
{
ASSERT_THAT(AdbcStatementRelease(&statement.value, &error),
IsOkStatus(&error));
}
+TEST_F(PostgresConnectionTest, MetadataGetSchemaCaseSensitiveTable) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error),
IsOkStatus(&error));
+
+ // Create sample table
+ {
+ adbc_validation::Handle<struct AdbcStatement> statement;
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement.value, &error),
+ IsOkStatus(&error));
+
+ ASSERT_THAT(AdbcStatementSetSqlQuery(&statement.value,
+ "DROP TABLE IF EXISTS \"Uppercase\"",
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr,
&error),
+ IsOkStatus(&error));
+
+ ASSERT_THAT(
+ AdbcStatementSetSqlQuery(
+ &statement.value, "CREATE TABLE \"Uppercase\" (ints INT, strs
TEXT)", &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr,
&error),
+ IsOkStatus(&error));
+ }
+
+ // Check its schema
+ nanoarrow::UniqueSchema schema;
+ ASSERT_THAT(AdbcConnectionGetTableSchema(&connection, nullptr, nullptr,
"Uppercase",
+ schema.get(), &error),
+ IsOkStatus(&error));
+
+ ASSERT_NE(schema->release, nullptr);
+ ASSERT_STREQ(schema->format, "+s");
+ ASSERT_EQ(schema->n_children, 2);
+ ASSERT_STREQ(schema->children[0]->format, "i");
+ ASSERT_STREQ(schema->children[1]->format, "u");
+ ASSERT_STREQ(schema->children[0]->name, "ints");
+ ASSERT_STREQ(schema->children[1]->name, "strs");
+
+ // Do we have to release the connection here?
+}
+
TEST_F(PostgresConnectionTest, MetadataGetStatistics) {
if (!quirks()->supports_statistics()) {
GTEST_SKIP();
diff --git a/c/validation/adbc_validation_statement.cc
b/c/validation/adbc_validation_statement.cc
index 333baf141..8965e95a0 100644
--- a/c/validation/adbc_validation_statement.cc
+++ b/c/validation/adbc_validation_statement.cc
@@ -2065,15 +2065,15 @@ void
StatementTest::TestSqlPrepareErrorParamCountMismatch() {
void StatementTest::TestSqlQueryEmpty() {
ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
IsOkStatus(&error));
- ASSERT_THAT(quirks()->DropTable(&connection, "QUERYEMPTY", &error),
IsOkStatus(&error));
+ ASSERT_THAT(quirks()->DropTable(&connection, "queryempty", &error),
IsOkStatus(&error));
ASSERT_THAT(
- AdbcStatementSetSqlQuery(&statement, "CREATE TABLE QUERYEMPTY (FOO
INT)", &error),
+ AdbcStatementSetSqlQuery(&statement, "CREATE TABLE queryempty (FOO
INT)", &error),
IsOkStatus(&error));
ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error),
IsOkStatus(&error));
ASSERT_THAT(
- AdbcStatementSetSqlQuery(&statement, "SELECT * FROM QUERYEMPTY WHERE
1=0", &error),
+ AdbcStatementSetSqlQuery(&statement, "SELECT * FROM queryempty WHERE
1=0", &error),
IsOkStatus(&error));
{
StreamReader reader;
