Refactor & fix unsafe pg and fb identifiers

src/convert.c

@@ -122,7 +122,10 @@ static bool foreign_expr_walker(Node *node,
 static bool canConvertOp(OpExpr *oe, int firebird_version);
 static bool is_builtin(Oid procid);
 
-static const char *quote_fb_identifier_for_import(const char *ident);
+static const char *quote_identifier_for_import(const char *ident, bool unsafe);
+
+bool unSafeFirebirdIdentifier (const char *identifier);
+bool unSafePostgresIdentifier (const char *identifier);
 
 /**
  * buildSelectSql()
@@ -467,6 +470,58 @@ generateColumnMetadataQuery(StringInfoData *data_type_sql, char *fb_table_name)
 	return;
 }
 
+/**
+ * unSafeFirebirdIdentifier()
+ *
+ * checks if identifier contains space or identifier is not uppercase
+ * SIMPLIFICATION, not real UTF, POC
+ */
+bool unSafeFirebirdIdentifier (const char *identifier)
+{
+	const char *ptr;
+	if (!((identifier[0] >= 'A' && identifier[0] <= 'Z') || identifier[0] == '_'))
+		return true;
+
+	for (ptr = identifier; *ptr; ptr++)
+	{
+		char		ch = *ptr;
+
+		if (!(ch >= 'A' && ch <= 'Z') &&
+			!(ch >= '0' && ch <= '9') &&
+			(ch != '_'))
+		{
+			return true;
+		}
+	}
+	return false;
+}
+
+/**
+ * unSafePostgresIdentifier()
+ *
+ * checks if identifier contains space or identifier is not lowercase
+ * SIMPLIFICATION, not real UTF, POC
+ */
+bool unSafePostgresIdentifier (const char *identifier)
+{
+	const char *ptr;
+	if (!((identifier[0] >= 'a' && identifier[0] <= 'z') || identifier[0] == '_'))
+		return true;
+
+	for (ptr = identifier; *ptr; ptr++)
+	{
+		char		ch = *ptr;
+
+		if (!(ch >= 'a' && ch <= 'z') &&
+			!(ch >= '0' && ch <= '9') &&
+			(ch != '_'))
+		{
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * convertFirebirdObject()
  *
@@ -475,64 +530,83 @@ generateColumnMetadataQuery(StringInfoData *data_type_sql, char *fb_table_name)
 void
 convertFirebirdObject(char *server_name, char *schema, char *object_name, char object_type, char *pg_name, bool import_not_null, bool updatable, FBresult *colres, StringInfoData *create_table)
 {
-	const char *table_name;
+	const char *safe_pg_server_name;
+	bool unsafe_pg_server_name;
+	const char *safe_pg_schema_name;
+	bool unsafe_pg_schema_name;
+	const char *safe_pg_table_name;
+	bool unsafe_pg_table_name;
+	const char *safe_fb_table_name;
+	bool unsafe_fb_table_name;
+
+	const char *safe_fb_column_name;
+   	bool unsafe_fb_column_name = false;
+	const char *safe_pg_column_name;
+   	bool unsafe_pg_column_name = false;
+
 	bool use_pg_name = false;
 	int colnr, coltotal;
 	List	   *table_options = NIL;
+
+	size_t identifierL = 0;
+	size_t cnL = 0;
+	char *cnOption;
 
 	/* Initialise table options list */
 	if (updatable == false)
 		table_options = lappend(table_options, "updatable 'false'");
 
-	/*
-	 * If the Firebird identifier is all lower-case, force "quote_identifier 'true'"
-	 * as PostgreSQL won't know to quote it.
-	 * XXX Currently we just check if the first character is lower case.
-	 */
-	table_name = quote_fb_identifier_for_import(object_name);
-
-	elog(DEBUG3, "object_name: %s; table_name: %s; pg_name: %s",
+	unsafe_pg_server_name = unSafePostgresIdentifier(server_name);
+    safe_pg_server_name = quote_identifier_for_import(server_name, unsafe_pg_server_name);
+
+	unsafe_pg_schema_name = unSafePostgresIdentifier(schema);
+    safe_pg_schema_name = quote_identifier_for_import(schema, unsafe_pg_schema_name);		
+
+	unsafe_pg_table_name = unSafePostgresIdentifier(object_name);
+	safe_pg_table_name = quote_identifier_for_import(object_name, unsafe_pg_table_name);
+	unsafe_fb_table_name = unSafeFirebirdIdentifier(object_name);
+	safe_fb_table_name = quote_identifier_for_import(object_name, unsafe_fb_table_name);
+
+	elog(DEBUG3, "safe_server_name: %s; safe_schema_name: %s\n          object_name: %s; pg_name: %s\n          safe_pg_table_name: %s\n          safe_fb_table_name: %s",
+   	     safe_pg_server_name,
+   	     safe_pg_schema_name,
 		 object_name,
-		 table_name,
-		 pg_name ? pg_name : "NULL");
+ 		 pg_name ? pg_name : "NULL",
+		 safe_pg_table_name,
+ 		 safe_fb_table_name);
 
-	if (table_name[0] == '"')
-	{
-		if (table_name[1] >= 'a' && table_name[1] <= 'z')
+	if (unsafe_fb_table_name)
+		{
 			table_options = lappend(table_options, "quote_identifier 'true'");
-	}
+			if (unsafe_pg_table_name)
+			{
+				identifierL = strlen(object_name);
+				cnL = strlen("table_name '");			
+				cnOption = malloc(identifierL + cnL + 2);
+				cnOption[0] = '\0';
+			    strcat(cnOption, "table_name '");
+			    strcat(cnOption, object_name);
+   			    strcat(cnOption, "'");
+   			    table_options = lappend(table_options, (void*)cnOption);
+   		    }
+		}
 	else if (pg_name != NULL)
 	{
 		/*
-		 * If "pg_name" == "table_name", i.e. the non-quoted folder-to-upper-case
-		 * version used in the Firebird metadata query, then that implies
-		 * the_name was quoted in the "LIMIT TO" clause, so we must
-		 * quote it here.
+		 * Use the name provided in the "LIMIT TO" clause
+		 * as-is, as the FDW API will reject the provided table definition.
 		 *
-		 * E.g. LIMIT TO ("BAR")
+		 * E.g. LIMIT TO (bar) -> must be "CREATE FOREIGN TABLE schema.bar",
+		 * not "CREATE FOREIGN TABLE schema.BAR".
 		 */
-		if (strcmp(table_name, pg_name) == 0)
-		{
-			table_name = quote_identifier(table_name);
-		}
-		else
-		{
-			/*
-			 * Otherwise use the name provided in the "LIMIT TO" clause
-			 * as-is, as the FDW API will reject the provided table definition.
-			 *
-			 * E.g. LIMIT TO (bar) -> must be "CREATE FOREIGN TABLE schema.bar",
-			 * not "CREATE FOREIGN TABLE schema.BAR".
-			 */
-			use_pg_name = true;
-		}
+		use_pg_name = true;
 	}
 
 	/* Generate SQL */
 	appendStringInfo(create_table,
 					 "CREATE FOREIGN TABLE %s.%s (\n",
-					 schema,
-					 use_pg_name ? pg_name : table_name);
+					 safe_pg_schema_name,
+					 use_pg_name ? pg_name : safe_pg_table_name);
 
 	coltotal = FQntuples(colres);
 
@@ -546,26 +620,37 @@ convertFirebirdObject(char *server_name, char *schema, char *object_name, char o
 		List	   *column_options = NIL;
 
 		char *datatype;
-		char *colname = pstrdup(FQgetvalue(colres, colnr, 0));
-
-		const char *col_identifier = quote_fb_identifier_for_import(colname);
+		char *fb_metadata_column_name = pstrdup(FQgetvalue(colres, colnr, 0));
+
+		unsafe_fb_column_name = unSafeFirebirdIdentifier(fb_metadata_column_name);
+		safe_fb_column_name = quote_identifier_for_import(fb_metadata_column_name, unsafe_fb_column_name);
+
+		unsafe_pg_column_name = unSafePostgresIdentifier(fb_metadata_column_name);
+		safe_pg_column_name = quote_identifier_for_import(fb_metadata_column_name, unsafe_pg_column_name);
+
+		elog(DEBUG3, "column name %s; unsafe pg %d: %s; unsafe fb %d: %s",
+		     fb_metadata_column_name,
+		     unsafe_pg_column_name,
+		     safe_pg_column_name,
+		     unsafe_fb_column_name,
+   		     safe_fb_column_name
+		     );
 
 		/*
-		 * If the Firebird identifier is all lower-case, force "quote_identifier 'true'"
+		 * If the Firebird identifier is unsafe, force "quote_identifier 'true'"
 		 * as PostgreSQL won't know to quote it.
-		 * XXX Currently we just check if the first character is lower case.
 		 */
-		if (col_identifier[0] == '"' && (col_identifier[1] >= 'a' && col_identifier[1] <= 'z'))
+		if (unsafe_fb_column_name)
 			column_options = lappend(column_options, "quote_identifier 'true'");
 
 		/* Column name and datatype */
-		datatype = FQgetvalue(colres, colnr, 2);
+		datatype = FQgetvalue(colres, colnr, 2);		
+
 		appendStringInfo(create_table,
 						 "	%s %s",
-						 col_identifier,
+						 safe_pg_column_name,
 						 datatype);
 
-
 		/* add OPTIONS if required */
 		if (column_options != NIL)
 		{
@@ -621,7 +706,7 @@ convertFirebirdObject(char *server_name, char *schema, char *object_name, char o
 
 	appendStringInfo(create_table,
 					 ") SERVER %s",
-					 server_name);
+					 safe_pg_server_name);
 
 	if (table_options != NIL)
 	{
@@ -748,12 +833,12 @@ convertDatum(Datum datum, Oid type)
 void
 convertColumnRef(StringInfo buf, Oid relid, int varattno, bool quote_identifier)
 {
-	char	   *colname = NULL;
+	char	   *fb_metadata_column_name = NULL;
 	bool		quote_col_identifier = quote_identifier;
 	fbColumnOptions column_options = fbColumnOptions_init;
 
 	column_options.quote_identifier = &quote_col_identifier;
-	column_options.column_name = &colname;
+	column_options.column_name = &fb_metadata_column_name;
 
 	elog(DEBUG2, "entering function %s", __func__);
 
@@ -763,17 +848,17 @@ convertColumnRef(StringInfo buf, Oid relid, int varattno, bool quote_identifier)
 							 &column_options);
 
 	/* otherwise use Postgres column name */
-	if (colname == NULL)
+	if (fb_metadata_column_name == NULL)
 	{
 #if (PG_VERSION_NUM >= 110000)
-		colname = get_attname(relid, varattno, false);
+		fb_metadata_column_name = get_attname(relid, varattno, false);
 #else
-		colname = get_relid_attribute_name(relid, varattno);
+		fb_metadata_column_name = get_relid_attribute_name(relid, varattno);
 #endif
 	}
 
 	appendStringInfoString(buf,
-						   quote_fb_identifier(colname, quote_col_identifier));
+						   quote_fb_identifier(fb_metadata_column_name, quote_col_identifier));
 }
 
 
@@ -824,43 +909,53 @@ quote_fb_identifier(const char *ident, bool quote_ident)
 	return quoted_ident;
 }
 
-
 /**
- * quote_fb_identifier_for_import()
+ * quote_identifier()
  *
- * Given a Firebird relation name, determine whether it would
- * be quoted in Firebird, i.e. contains characters other than
- * ASCII capital letters, digits and underscores.
+ * Add ""
  */
-static const char *
-quote_fb_identifier_for_import(const char *ident)
+const char *quote_identifier(const char *ident)
 {
 	int			nquotes = 0;
-	bool		safe;
 	const char *ptr;
 	char	   *result;
 	char	   *optr;
+
+	for (ptr = ident; *ptr; ptr++)
+	{
+		if (*ptr == '"')
+			nquotes++;
+	}
+
+	result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
 
-	safe = ((ident[0] >= 'A' && ident[0] <= 'Z') || ident[0] == '_');
-
+	optr = result;
+	*optr++ = '"';
 	for (ptr = ident; *ptr; ptr++)
 	{
 		char		ch = *ptr;
 
-		if ((ch >= 'A' && ch <= 'Z') ||
-			(ch >= '0' && ch <= '9') ||
-			(ch == '_'))
-		{
-			/* okay */
-		}
-		else
-		{
-			safe = false;
-			if (ch == '"')
-				nquotes++;
-		}
+		if (ch == '"')
+			*optr++ = '"';
+		*optr++ = ch;
 	}
+	*optr++ = '"';
+	*optr = '\0';
 
+	return result;	
+}
+
+/**
+ * quote_identifier_for_import()
+ *
+ * Given a object name, determine whether it would be quoted
+ * both for Postgres or Firebird depends on source of safe flague
+ */
+static const char *
+quote_identifier_for_import(const char *ident, bool unsafe)
+{
+	bool		safe;
+	safe = !unsafe;
 
 	if (safe)
 	{
@@ -890,24 +985,7 @@ quote_fb_identifier_for_import(const char *ident)
 
 	if (safe)
 		return ident;			/* no change needed */
-
-
-	result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
-
-	optr = result;
-	*optr++ = '"';
-	for (ptr = ident; *ptr; ptr++)
-	{
-		char		ch = *ptr;
-
-		if (ch == '"')
-			*optr++ = '"';
-		*optr++ = ch;
-	}
-	*optr++ = '"';
-	*optr = '\0';
-
-	return result;
+    return quote_identifier(ident);
 }
 
 /**