Fix conn string parsing with unsupported options

The ODBC specification does not mandate how the driver should deal with
unsupported options specified by client in the connection string.

When encountering an unsupported option, DuckDB driver registers a
diagnostic message in a format: 'Invalid keyword: <option_name>', that
can be fetched by client using SQLGetDiagRec function, and returns
SQL_SUCCESS_WITH_INFO instead of SQL_SUCCESS.

This is a reasonable approach, but there is inconsistent error handling
in its implementation in Connect::ParseInputStr. It only checks for
SQL_SUCCESS and treats SQL_SUCCESS_WITH_INFO as an error. This causes
the early exit from this function that prevents subsequent options in
the connection string to be processed. It also prevents the DSN name
to be set on the connection (even when the DSN option comes before the
unsupported one).

To fix this it is proposed to check for both SQL_SUCCESS and
SQL_SUCCESS_WITH_INFO and only do the early exit if some other error
code is returned by FindKeyValPair routine.

Testing: tests are included for a common option and for the DSN option.

Fixes: duckdb#59

src/connect/connect.cpp

@@ -66,31 +66,38 @@ SQLRETURN Connect::FindKeyValPair(const std::string &row) {
 SQLRETURN Connect::ParseInputStr() {
 	size_t row_pos;
 	std::string row;
+	SQLRETURN parse_ret = SQL_SUCCESS;
 
 	if (input_str.empty()) {
-		return SQL_SUCCESS;
+		return parse_ret;
 	}
 
 	while ((row_pos = input_str.find(ROW_DEL)) != std::string::npos) {
 		row = input_str.substr(0, row_pos);
 		SQLRETURN ret = FindKeyValPair(row);
-		if (ret != SQL_SUCCESS) {
+		if (!SQL_SUCCEEDED(ret)) {
 			return ret;
 		}
+		if (SQL_SUCCESS_WITH_INFO == ret && SQL_SUCCESS == parse_ret) {
+			parse_ret = ret;
+		}
 		input_str.erase(0, row_pos + 1);
 	}
 
 	if (!input_str.empty()) {
 		SQLRETURN ret = FindKeyValPair(input_str);
-		if (ret != SQL_SUCCESS) {
+		if (!SQL_SUCCEEDED(ret)) {
 			return ret;
 		}
+		if (SQL_SUCCESS_WITH_INFO == ret && SQL_SUCCESS == parse_ret) {
+			parse_ret = ret;
+		}
 	}
 
 	// Extract the DSN from the config map as it is needed to read from the .odbc.ini file
 	dbc->dsn = seen_config_options["dsn"] ? config_map["dsn"].ToString() : "";
 
-	return SQL_SUCCESS;
+	return parse_ret;
 }
 
 SQLRETURN Connect::ReadFromIniFile() {

test/tests/connect.cpp

@@ -127,6 +127,9 @@ static void TestSettingConfigs() {
 
 	SetConfig("Database=" + GetTesterDirectory() + "test.duckdb;" + "access_mode=READ_WRITE", "access_mode",
 	          "READ_WRITE");
+
+	// Test handling unsupported connection string options
+	SetConfig("unsupported_option_1=value_1;allow_unsigned_extensions=true;", "allow_unsigned_extensions", "true");
 }
 
 TEST_CASE("Test SQLConnect and SQLDriverConnect", "[odbc]") {

test/tests/connect_with_ini.cpp

@@ -46,3 +46,20 @@ TEST_CASE("Test SQLConnect with Ini File with extra options", "[odbc]") {
 	DISCONNECT_FROM_DATABASE(env, dbc);
 #endif
 }
+
+// Connect to the database using the ini file checking that DSN is set correctly
+// when unsupported options are present in the connection string
+TEST_CASE("Test SQLConnect with Ini File with unsupported options", "[odbc]") {
+#if defined ODBC_LINK_ODBCINST || defined WIN32
+	// Connect to the database using the ini file
+	SQLHANDLE env;
+	SQLHANDLE dbc;
+	DRIVER_CONNECT_TO_DATABASE(env, dbc, "DSN=DuckDB;unsupported_option_1=value_1;");
+
+	// Check that the database is set
+	CheckDatabase(dbc);
+
+	// Disconnect from the database
+	DISCONNECT_FROM_DATABASE(env, dbc);
+#endif
+}