Merge pull request #1874 from wheels-dev/feat/quote-identifiers

feat(sql): quote identifiers to prevent reserved word conflicts

Author: bpamiri

core/src/wheels/databaseAdapters/Base.cfc

@@ -161,6 +161,8 @@ component output=false extends="wheels.Global"{
 				"#Chr(10)#,#Chr(13)#, ",
 				",,"
 			);
+			// Strip identifier quotes from column list for comparison
+			local.columnList = $stripIdentifierQuotes(local.columnList);
 			if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
 				local.rv = {};
 				query = $query(sql = "SELECT LAST_INSERT_ID() AS lastId", argumentCollection = arguments.queryAttributes);
@@ -186,6 +188,23 @@ component output=false extends="wheels.Global"{
 		return " DEFAULT VALUES";
 	}
 
+	/**
+	 * Quote a database identifier (table or column name) using the adapter's quoting character.
+	 * Base implementation is a no-op; individual adapters override with their specific quoting.
+	 * This prevents reserved word conflicts across all supported databases.
+	 */
+	public string function $quoteIdentifier(required string name) {
+		return arguments.name;
+	}
+
+	/**
+	 * Strip all identifier quote characters from a string.
+	 * Used when parsing rendered SQL to compare column names without quoting artifacts.
+	 */
+	public string function $stripIdentifierQuotes(required string str) {
+		return ReReplace(arguments.str, '`|\[|\]|"', "", "all");
+	}
+
 	/**
 	 * Set a default for the table alias string (e.g. "users AS users2").
 	 * Individual database adapters will override when necessary.

core/src/wheels/databaseAdapters/MicrosoftSQLServer/MicrosoftSQLServerModel.cfc

@@ -140,7 +140,10 @@ component extends="wheels.databaseAdapters.Base" output=false {
 			local.iEnd = ListLen(local.thirdOrder);
 			for (local.i = 1; local.i <= local.iEnd; local.i++) {
 				local.item = ReReplace(ReReplace(ListGetAt(local.thirdOrder, local.i), " ASC\b", ""), " DESC\b", "");
-				if (!ListFindNoCase(local.thirdSelect, local.item)) {
+				// Strip identifier quotes for comparison since SELECT may have different quoting than ORDER BY
+				local.itemStripped = $stripIdentifierQuotes(local.item);
+				local.thirdSelectStripped = $stripIdentifierQuotes(local.thirdSelect);
+				if (!ListFindNoCase(local.thirdSelectStripped, local.itemStripped) && !ListFindNoCase(local.thirdSelect, local.item)) {
 					// The test "order_clause_with_paginated_include_and_ambiguous_columns" passes in a complex order (CASE WHEN registration IN ('foo') THEN 0 ELSE 1 END DESC).
 					// This gets moved up to the SELECT clause to support pagination.
 					// However, we need to add "AS" to it otherwise we get a "No column name was specified" error.
@@ -233,9 +236,11 @@ component extends="wheels.databaseAdapters.Base" output=false {
 				"#Chr(10)#,#Chr(13)#, ",
 				",,"
 			);
+			// Strip identifier quotes from column list for comparison
+			local.columnList = $stripIdentifierQuotes(local.columnList);
 			if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
 				local.rv = {};
-				
+
 				// Use @@IDENTITY instead of SCOPE_IDENTITY() for BoxLang compatibility
 				// SCOPE_IDENTITY() returns empty values in BoxLang with SQL Server
 				query = $query(sql = "SELECT @@IDENTITY AS lastId", argumentCollection = arguments.queryAttributes);
@@ -258,5 +263,12 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		return "NEWID()";
 	}
 
+	/**
+	 * Override Base adapter's function.
+	 * SQL Server uses square brackets to quote identifiers.
+	 */
+	public string function $quoteIdentifier(required string name) {
+		return "[#arguments.name#]";
+	}
 
 }

core/src/wheels/databaseAdapters/MySQL/MySQLModel.cfc

@@ -115,5 +115,12 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		return "() VALUES()";
 	}
 
+	/**
+	 * Override Base adapter's function.
+	 * MySQL uses backticks to quote identifiers.
+	 */
+	public string function $quoteIdentifier(required string name) {
+		return "`#arguments.name#`";
+	}
 
 }

core/src/wheels/databaseAdapters/Oracle/OracleModel.cfc

@@ -106,6 +106,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
 				"#Chr(10)#,#Chr(13)#, ",
 				",,"
 			);
+			// Strip identifier quotes from column list for comparison
+			local.columnList = $stripIdentifierQuotes(local.columnList);
 			if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
 				local.rv = {};
 				local.tbl = SpanExcluding(Right(local.sql, Len(local.sql) - 12), " ");
@@ -141,5 +143,14 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		return arguments.table & " " & arguments.alias;
 	}
 
+	/**
+	 * Override Base adapter's function.
+	 * Oracle uses double-quotes to quote identifiers.
+	 */
+	public string function $quoteIdentifier(required string name) {
+		// Oracle folds unquoted identifiers to uppercase, so we must uppercase
+		// before quoting to match the actual stored name
+		return """#UCase(arguments.name)#""";
+	}
 
 }

core/src/wheels/databaseAdapters/PostgreSQL/PostgreSQLModel.cfc

@@ -151,10 +151,15 @@ component extends="wheels.databaseAdapters.Base" output=false {
 				}
 			}
 
+			// Strip identifier quotes from column list for comparison
+			local.columnList = $stripIdentifierQuotes(local.columnList);
+
 			// Lucee/ACF doesn't support PostgreSQL natively when it comes to returning the primary key value of the last inserted record so we have to do it manually by using the sequence.
 			if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
 				local.rv = {};
 				local.tbl = SpanExcluding(Right(local.sql, Len(local.sql) - 12), " ");
+				// Strip identifier quotes that may have been added by $quoteIdentifier
+				local.tbl = ReReplace(local.tbl, '^"|"$', "", "all");
 				query = $query(
 					sql = "SELECT currval(pg_get_serial_sequence('#local.tbl#', '#arguments.primaryKey#')) AS lastId",
 					argumentCollection = arguments.queryAttributes
@@ -172,5 +177,14 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		return "random()";
 	}
 
+	/**
+	 * Override Base adapter's function.
+	 * PostgreSQL uses double-quotes to quote identifiers (ANSI SQL standard).
+	 */
+	public string function $quoteIdentifier(required string name) {
+		// PostgreSQL folds unquoted identifiers to lowercase, so we must lowercase
+		// before quoting to match the actual stored name
+		return """#LCase(arguments.name)#""";
+	}
 
 }

core/src/wheels/databaseAdapters/SQLite/SQLiteModel.cfc

@@ -105,6 +105,8 @@ component extends="wheels.databaseAdapters.Base" output=false {
                 );
             }
 
+            // Strip identifier quotes from column list for comparison
+            local.columnList = $stripIdentifierQuotes(local.columnList);
             // If the primary key column wasn't part of the INSERT, we fetch last inserted ID
             if (!ListFindNoCase(local.columnList, ListFirst(arguments.primaryKey))) {
                 local.rv = {};
@@ -126,4 +128,12 @@ component extends="wheels.databaseAdapters.Base" output=false {
 		return " DEFAULT VALUES";
 	}
 
+	/**
+	 * Override Base adapter's function.
+	 * SQLite uses double-quotes to quote identifiers (ANSI SQL standard).
+	 */
+	public string function $quoteIdentifier(required string name) {
+		return """#arguments.name#""";
+	}
+
 }

core/src/wheels/model/calculations.cfc

@@ -235,7 +235,7 @@ component {
 			if (StructKeyExists(variables.wheels.class.propertyStruct, local.item)) {
 				local.properties = ListAppend(
 					local.properties,
-					tableName() & "." & variables.wheels.class.properties[local.item].column
+					$quotedTableName() & "." & $quoteColumn(variables.wheels.class.properties[local.item].column)
 				);
 			} else if (StructKeyExists(variables.wheels.class.calculatedProperties, local.item)) {
 				local.properties = ListAppend(local.properties, variables.wheels.class.calculatedProperties[local.item].sql);

core/src/wheels/model/create.cfc

@@ -301,7 +301,7 @@ component {
 					)
 				)
 			) {
-				ArrayAppend(local.sql, variables.wheels.class.properties[local.key].column);
+				ArrayAppend(local.sql, $quoteColumn(variables.wheels.class.properties[local.key].column));
 				ArrayAppend(local.sql, ",");
 				ArrayAppend(local.sql2, $buildQueryParamValues(local.key));
 				ArrayAppend(local.sql2, ",");
@@ -310,7 +310,7 @@ component {
 
 		if (ArrayLen(local.sql)) {
 			// Create wrapping SQL code and merge the second array that holds the values with the first one.
-			ArrayPrepend(local.sql, "INSERT INTO #tableName()# (");
+			ArrayPrepend(local.sql, "INSERT INTO #$quotedTableName()# (");
 			ArrayPrepend(local.sql2, " VALUES (");
 			ArrayDeleteAt(local.sql, ArrayLen(local.sql));
 			ArrayDeleteAt(local.sql2, ArrayLen(local.sql2));
@@ -332,7 +332,7 @@ component {
 			local.pks = primaryKey(0);
 			ArrayAppend(
 				local.sql,
-				"INSERT INTO #tableName()#" & variables.wheels.class.adapter.$defaultValues($primaryKey = local.pks)
+				"INSERT INTO #$quotedTableName()#" & variables.wheels.class.adapter.$defaultValues($primaryKey = local.pks)
 			);
 		}
 

core/src/wheels/model/miscellaneous.cfc

@@ -185,6 +185,22 @@ component {
 		}
 	}
 
+	/**
+	 * Returns the table name quoted with the adapter's identifier quoting character.
+	 * Used internally when building SQL to prevent reserved word conflicts.
+	 */
+	public string function $quotedTableName() {
+		return variables.wheels.class.adapter.$quoteIdentifier(tableName());
+	}
+
+	/**
+	 * Quotes a column name using the adapter's identifier quoting character.
+	 * Used internally when building SQL to prevent reserved word conflicts.
+	 */
+	public string function $quoteColumn(required string column) {
+		return variables.wheels.class.adapter.$quoteIdentifier(arguments.column);
+	}
+
 	/**
 	 * Returns the table name prefix set for the table.
 	 *

core/src/wheels/model/read.cfc

@@ -213,7 +213,7 @@ component {
 					list = arguments.select,
 					returnAs = arguments.returnAs
 				);
-				local.columns = ReReplace(local.columns, "\w*?\.([\w\s]*?)(,|$)", "\1\2", "all");
+				local.columns = ReReplace(local.columns, "[`""\[\]\w]*?\.([\w\s]*?)(,|$)", "\1\2", "all");
 				local.columns = ReReplace(local.columns, "\(.*?\)\sAS\s([\w\s]*?)(,|$)", "\1\2", "all");
 				local.columns = ReReplace(local.columns, "\w*?\sAS\s([\w\s]*?)(,|$)", "\1\2", "all");
 				local.rv = QueryNew(local.columns);