Optimize usages of SqlString.Append (#2166)

bahusoid · hazzik · commit 3041c49b7548 · 2019-07-12T04:33:31.000Z
diff --git a/src/NHibernate/Criterion/Distinct.cs b/src/NHibernate/Criterion/Distinct.cs
@@ -17,8 +17,7 @@ public Distinct(IProjection proj)
 
 		public virtual SqlString ToSqlString(ICriteria criteria, int position, ICriteriaQuery criteriaQuery)
 		{
-			return new SqlString("distinct ")
-				.Append(projection.ToSqlString(criteria, position, criteriaQuery));
+			return new SqlString("distinct ", projection.ToSqlString(criteria, position, criteriaQuery));
 		}
 
 		public virtual SqlString ToGroupSqlString(ICriteria criteria, ICriteriaQuery criteriaQuery)
@@ -77,4 +76,4 @@ public override string ToString()
 			return "distinct " + projection.ToString();
 		}
 	}
-}
+}
diff --git a/src/NHibernate/Criterion/Order.cs b/src/NHibernate/Criterion/Order.cs
@@ -40,12 +40,9 @@ public virtual SqlString ToSqlString(ICriteria criteria, ICriteriaQuery criteria
 		{
 			if (projection != null)
 			{
-				SqlString sb = SqlString.Empty;
-				SqlString produced = this.projection.ToSqlString(criteria, 0, criteriaQuery);
+				SqlString produced = projection.ToSqlString(criteria, 0, criteriaQuery);
 				SqlString truncated = SqlStringHelper.RemoveAsAliasesFromSql(produced);
-				sb = sb.Append(truncated);
-				sb = sb.Append(ascending ? " asc" : " desc");
-				return sb;
+				return new SqlString(truncated, ascending ? " asc" : " desc");
 			}
 
 			string[] columns = criteriaQuery.GetColumnAliasesUsingProjection(criteria, propertyName);
diff --git a/src/NHibernate/Criterion/SubqueryProjection.cs b/src/NHibernate/Criterion/SubqueryProjection.cs
@@ -42,7 +42,7 @@ public override IType[] GetTypes(ICriteria criteria, ICriteriaQuery criteriaQuer
 		public override SqlString ToSqlString(ICriteria criteria, int loc, ICriteriaQuery criteriaQuery)
 		{
 			return _subQuery.ToSqlString(criteria, criteriaQuery)
-				.Append(new SqlString(" as y", loc.ToString(), "_"));
+				.Append(" as y", loc.ToString(), "_");
 		}
 
 		public override SqlString ToGroupSqlString(ICriteria criteria, ICriteriaQuery criteriaQuery)
diff --git a/src/NHibernate/Dialect/PostgreSQL81Dialect.cs b/src/NHibernate/Dialect/PostgreSQL81Dialect.cs
@@ -111,7 +111,7 @@ public override SqlString AppendIdentitySelectToInsert(SqlString insertSql)
 
 		public override SqlString AppendIdentitySelectToInsert(SqlString insertString, string identifierColumnName)
 		{
-			return insertString.Append(" returning ").Append(identifierColumnName);
+			return insertString.Append(" returning ", identifierColumnName);
 		}
 
 		public override bool SupportsInsertSelectIdentity
diff --git a/src/NHibernate/Dialect/PostgreSQLDialect.cs b/src/NHibernate/Dialect/PostgreSQLDialect.cs
@@ -177,7 +177,7 @@ public override string GetDropSequenceString(string sequenceName)
 
 		public override SqlString AddIdentifierOutParameterToInsert(SqlString insertString, string identifierColumnName, string parameterName)
 		{
-			return insertString.Append(" returning ").Append(identifierColumnName);
+			return insertString.Append(" returning ",  identifierColumnName);
 		}
 
 		public override InsertGeneratedIdentifierRetrievalMethod InsertGeneratedIdentifierRetrievalMethod
diff --git a/src/NHibernate/Driver/BasicResultSetsCommand.cs b/src/NHibernate/Driver/BasicResultSetsCommand.cs
@@ -22,7 +22,7 @@ public BasicResultSetsCommand(ISessionImplementor session)
 		{
 			Commands = new List<ISqlCommand>();
 			Session = session;
-			_statementTerminator = session.Factory.Dialect.StatementTerminator.ToString();
+			_statementTerminator = session.Factory.Dialect.StatementTerminator.ToString() + Environment.NewLine;
 		}
 
 		protected List<ISqlCommand> Commands { get; private set; }
@@ -32,7 +32,7 @@ public BasicResultSetsCommand(ISessionImplementor session)
 		public virtual void Append(ISqlCommand command)
 		{
 			Commands.Add(command);
-			sqlString = sqlString.Append(command.Query).Append(_statementTerminator).Append(Environment.NewLine);
+			sqlString = sqlString.Append(command.Query, _statementTerminator);
 		}
 
 		public bool HasQueries
diff --git a/src/NHibernate/Engine/JoinSequence.cs b/src/NHibernate/Engine/JoinSequence.cs
@@ -204,7 +204,7 @@ internal virtual JoinFragment ToJoinFragment(
 				{
 					if (join.Alias.Equals(withClauseJoinAlias))
 					{
-						condition = condition.Append(" and ").Append(withClauseFragment);
+						condition = condition.Append(" and ", withClauseFragment);
 					}
 				}
 
diff --git a/src/NHibernate/Hql/Ast/ANTLR/Tree/EntityJoinJoinSequenceImpl.cs b/src/NHibernate/Hql/Ast/ANTLR/Tree/EntityJoinJoinSequenceImpl.cs
@@ -30,12 +30,16 @@ internal override JoinFragment ToJoinFragment(
 		{
 			var joinFragment = new ANSIJoinFragment();
 
-			var on = withClauseFragment ?? new SqlString();
-			var filters = _entityType.GetOnCondition(_tableAlias, Factory, enabledFilters);
-			if (!string.IsNullOrEmpty(filters))
-			{
-				on.Append(" and ").Append(filters);
-			}
+			var on = withClauseFragment ?? SqlString.Empty;
+			//Note: filters logic commented due to following issues
+			//1) Original code is non functional as SqlString is immutable and so all Append results are lost. Correct code would look like: on = on.Append(filters);
+			//2) Also it seems GetOnCondition always returns empty string for entity join (as IsReferenceToPrimaryKey is always true).
+			//   So if filters for entity join really make sense we need to inline GetOnCondition part that retrieves filters
+//			var filters = _entityType.GetOnCondition(_tableAlias, Factory, enabledFilters);
+//			if (!string.IsNullOrEmpty(filters))
+//			{
+//				on.Append(" and ").Append(filters);
+//			}
 			joinFragment.AddJoin(_tableName, _tableAlias, Array.Empty<string>(), Array.Empty<string>(), _joinType, on);
 			return joinFragment;
 		}
diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs
@@ -805,7 +805,7 @@ protected SqlString MergeOrderings(SqlString ass, SqlString orderBy)
 			else if (orderBy.Length == 0)
 				return ass;
 			else
-				return ass.Append(StringHelper.CommaSpace).Append(orderBy);
+				return ass.Append(StringHelper.CommaSpace, orderBy);
 		}
 
 		protected SqlString MergeOrderings(string ass, SqlString orderBy) {
diff --git a/src/NHibernate/Loader/OuterJoinableAssociation.cs b/src/NHibernate/Loader/OuterJoinableAssociation.cs
@@ -49,7 +49,7 @@ public OuterJoinableAssociation(IAssociationType joinableType, String lhsAlias,
 			rhsColumns = JoinHelper.GetRHSColumnNames(joinableType, factory);
 			on = new SqlString(joinableType.GetOnCondition(rhsAlias, factory, enabledFilters));
 			if (SqlStringHelper.IsNotEmpty(withClause))
-				on = on.Append(" and ( ").Append(withClause).Append(" )");
+				on = on.Append(" and ( ", withClause, " )");
 			this.enabledFilters = enabledFilters; // needed later for many-to-many/filter application
 		}
 
@@ -166,7 +166,7 @@ public void AddManyToManyJoin(JoinFragment outerjoin, IQueryableCollection colle
 			SqlString condition = string.Empty.Equals(manyToManyFilter)
 								? on
 								: SqlStringHelper.IsEmpty(on) ? new SqlString(manyToManyFilter) : 
-									on.Append(" and ").Append(manyToManyFilter);
+									on.Append(" and ", manyToManyFilter);
 
 			outerjoin.AddJoin(joinable.TableName, rhsAlias, lhsColumns, rhsColumns, joinType, condition);
 			outerjoin.AddJoins(joinable.FromJoinFragment(rhsAlias, false, true),
diff --git a/src/NHibernate/SqlCommand/SqlString.cs b/src/NHibernate/SqlCommand/SqlString.cs
@@ -347,7 +347,23 @@ public SqlString Append(string text)
 		{
 			if (string.IsNullOrEmpty(text)) return this;
 			if (_length == 0) return new SqlString(text);
-			return new SqlString(new object[] { this, text });
+			return new SqlString(this, text);
+		}
+
+		public SqlString Append(params object[] parts)
+		{
+			return _length == 0
+				? new SqlString(parts)
+				: new SqlString(GetAppendParts(parts));
+		}
+
+		private IEnumerable<object> GetAppendParts(object[] parts)
+		{
+			yield return this;
+			foreach (var part in parts)
+			{
+				yield return part;
+			}
 		}
 
 		/// <summary>
diff --git a/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs b/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs
@@ -134,7 +134,7 @@ private void AddColumnWithValueOrType(string columnName, object valueOrType)
 		public SqlUpdateBuilder AppendAssignmentFragment(SqlString fragment)
 		{
 			// SqlString is immutable
-			assignments = assignments == null ? fragment : assignments.Append(", ").Append(fragment);
+			assignments = assignments == null ? fragment : assignments.Append(", ", fragment);
 			return this;
 		}
 

Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,7 @@ public Distinct(IProjection proj)`
`17`	`17`
`18`	`18`	`public virtual SqlString ToSqlString(ICriteria criteria, int position, ICriteriaQuery criteriaQuery)`
`19`	`19`	`{`
`20`		`- return new SqlString("distinct ")`
`21`		`- .Append(projection.ToSqlString(criteria, position, criteriaQuery));`
	`20`	`+ return new SqlString("distinct ", projection.ToSqlString(criteria, position, criteriaQuery));`
`22`	`21`	`}`
`23`	`22`
`24`	`23`	`public virtual SqlString ToGroupSqlString(ICriteria criteria, ICriteriaQuery criteriaQuery)`
`@@ -77,4 +76,4 @@ public override string ToString()`
`77`	`76`	`return "distinct " + projection.ToString();`
`78`	`77`	`}`
`79`	`78`	`}`
`80`		`-}`
	`79`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -40,12 +40,9 @@ public virtual SqlString ToSqlString(ICriteria criteria, ICriteriaQuery criteria`
`40`	`40`	`{`
`41`	`41`	`if (projection != null)`
`42`	`42`	`{`
`43`		`- SqlString sb = SqlString.Empty;`
`44`		`- SqlString produced = this.projection.ToSqlString(criteria, 0, criteriaQuery);`
	`43`	`+ SqlString produced = projection.ToSqlString(criteria, 0, criteriaQuery);`
`45`	`44`	`SqlString truncated = SqlStringHelper.RemoveAsAliasesFromSql(produced);`
`46`		`- sb = sb.Append(truncated);`
`47`		`- sb = sb.Append(ascending ? " asc" : " desc");`
`48`		`- return sb;`
	`45`	`+ return new SqlString(truncated, ascending ? " asc" : " desc");`
`49`	`46`	`}`
`50`	`47`
`51`	`48`	`string[] columns = criteriaQuery.GetColumnAliasesUsingProjection(criteria, propertyName);`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ public override IType[] GetTypes(ICriteria criteria, ICriteriaQuery criteriaQuer`
`42`	`42`	`public override SqlString ToSqlString(ICriteria criteria, int loc, ICriteriaQuery criteriaQuery)`
`43`	`43`	`{`
`44`	`44`	`return _subQuery.ToSqlString(criteria, criteriaQuery)`
`45`		`- .Append(new SqlString(" as y", loc.ToString(), "_"));`
	`45`	`+ .Append(" as y", loc.ToString(), "_");`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`public override SqlString ToGroupSqlString(ICriteria criteria, ICriteriaQuery criteriaQuery)`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,7 @@ public override SqlString AppendIdentitySelectToInsert(SqlString insertSql)`
`111`	`111`
`112`	`112`	`public override SqlString AppendIdentitySelectToInsert(SqlString insertString, string identifierColumnName)`
`113`	`113`	`{`
`114`		`- return insertString.Append(" returning ").Append(identifierColumnName);`
	`114`	`+ return insertString.Append(" returning ", identifierColumnName);`
`115`	`115`	`}`
`116`	`116`
`117`	`117`	`public override bool SupportsInsertSelectIdentity`
Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,7 @@ public override string GetDropSequenceString(string sequenceName)`
`177`	`177`
`178`	`178`	`public override SqlString AddIdentifierOutParameterToInsert(SqlString insertString, string identifierColumnName, string parameterName)`
`179`	`179`	`{`
`180`		`- return insertString.Append(" returning ").Append(identifierColumnName);`
	`180`	`+ return insertString.Append(" returning ", identifierColumnName);`
`181`	`181`	`}`
`182`	`182`
`183`	`183`	`public override InsertGeneratedIdentifierRetrievalMethod InsertGeneratedIdentifierRetrievalMethod`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ public BasicResultSetsCommand(ISessionImplementor session)`
`22`	`22`	`{`
`23`	`23`	`Commands = new List<ISqlCommand>();`
`24`	`24`	`Session = session;`
`25`		`- _statementTerminator = session.Factory.Dialect.StatementTerminator.ToString();`
	`25`	`+ _statementTerminator = session.Factory.Dialect.StatementTerminator.ToString() + Environment.NewLine;`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`protected List<ISqlCommand> Commands { get; private set; }`
`@@ -32,7 +32,7 @@ public BasicResultSetsCommand(ISessionImplementor session)`
`32`	`32`	`public virtual void Append(ISqlCommand command)`
`33`	`33`	`{`
`34`	`34`	`Commands.Add(command);`
`35`		`- sqlString = sqlString.Append(command.Query).Append(_statementTerminator).Append(Environment.NewLine);`
	`35`	`+ sqlString = sqlString.Append(command.Query, _statementTerminator);`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`public bool HasQueries`
Original file line number	Diff line number	Diff line change
`@@ -204,7 +204,7 @@ internal virtual JoinFragment ToJoinFragment(`
`204`	`204`	`{`
`205`	`205`	`if (join.Alias.Equals(withClauseJoinAlias))`
`206`	`206`	`{`
`207`		`- condition = condition.Append(" and ").Append(withClauseFragment);`
	`207`	`+ condition = condition.Append(" and ", withClauseFragment);`
`208`	`208`	`}`
`209`	`209`	`}`
`210`	`210`
Original file line number	Diff line number	Diff line change
`@@ -805,7 +805,7 @@ protected SqlString MergeOrderings(SqlString ass, SqlString orderBy)`
`805`	`805`	`else if (orderBy.Length == 0)`
`806`	`806`	`return ass;`
`807`	`807`	`else`
`808`		`- return ass.Append(StringHelper.CommaSpace).Append(orderBy);`
	`808`	`+ return ass.Append(StringHelper.CommaSpace, orderBy);`
`809`	`809`	`}`
`810`	`810`
`811`	`811`	`protected SqlString MergeOrderings(string ass, SqlString orderBy) {`