Skip to content

Optimize usages of SqlString.Append #2166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/NHibernate/Criterion/Distinct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -77,4 +76,4 @@ public override string ToString()
return "distinct " + projection.ToString();
}
}
}
}
7 changes: 2 additions & 5 deletions src/NHibernate/Criterion/Order.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Criterion/SubqueryProjection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Dialect/PostgreSQL81Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Dialect/PostgreSQLDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/NHibernate/Driver/BasicResultSetsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Engine/JoinSequence.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ internal virtual JoinFragment ToJoinFragment(
{
if (join.Alias.Equals(withClauseJoinAlias))
{
condition = condition.Append(" and ").Append(withClauseFragment);
condition = condition.Append(" and ", withClauseFragment);
}
}

Expand Down
16 changes: 10 additions & 6 deletions src/NHibernate/Hql/Ast/ANTLR/Tree/EntityJoinJoinSequenceImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/Loader/JoinWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/NHibernate/Loader/OuterJoinableAssociation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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),
Expand Down
18 changes: 17 additions & 1 deletion src/NHibernate/SqlCommand/SqlString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider introducing a new private SqlString(SqlString other, params object[] parts) constructor which can optimise the parts copying and append operations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I've considered such ctor. But looking at existing ctors SqlString(SqlString x) and SqlString(params object[] parts) - I didn't find a way to merge them nicely. So IMHO it's not worth it. But if you've come up with implementation - feel free to add it.

{
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>
Expand Down
2 changes: 1 addition & 1 deletion src/NHibernate/SqlCommand/SqlUpdateBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down