Skip to content

Correct MaxAliasLength for various dialects #1415

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 2 commits into from
Nov 3, 2017
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
24 changes: 17 additions & 7 deletions src/NHibernate.Test/MappingTest/ColumnFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public void StringSqlType()
Assert.AreEqual("NVARCHAR(100)", column.GetSqlType(_dialect, null));
}

// Should be kept synchronized with Column same constant.
private const int _charactersLeftCount = 4;

[TestCase("xxxxyyyyz")]
[TestCase("xxxxyyyyzz")]
Expand All @@ -54,16 +56,15 @@ public void GetAliasRespectsMaxAliasLength(string columnName)
{
var dialect = new GenericDialect();

// Verify test case assumption.
Assert.That(dialect.MaxAliasLength, Is.EqualTo(10));
// Test case is meant for a max length of 10, adjusts name if it is more.
columnName = AdjustColumnNameToMaxLength(columnName, dialect, 10);

var column = new Column(columnName);
string generatedAlias = column.GetAlias(dialect);

Assert.That(generatedAlias, Has.Length.LessThanOrEqualTo(dialect.MaxAliasLength));
Assert.That(generatedAlias, Has.Length.LessThanOrEqualTo(dialect.MaxAliasLength - _charactersLeftCount));
}


[TestCase("xxxxyyyyz")]
[TestCase("xxxxyyyyzz")]
[TestCase("xxxxyyyyzzz")]
Expand All @@ -77,15 +78,24 @@ public void GetAliasWithTableSuffixRespectsMaxAliasLength(string columnName)
{
var dialect = new GenericDialect();

// Verify test case assumption.
Assert.That(dialect.MaxAliasLength, Is.EqualTo(10));
// Test case is meant for a max length of 10, adjusts name if it is more.
columnName = AdjustColumnNameToMaxLength(columnName, dialect, 10);

var table = new Table();
var column = new Column(columnName);

string generatedAlias = column.GetAlias(dialect, table);

Assert.That(generatedAlias, Has.Length.LessThanOrEqualTo(dialect.MaxAliasLength));
Assert.That(generatedAlias, Has.Length.LessThanOrEqualTo(dialect.MaxAliasLength - _charactersLeftCount));
}

private static string AdjustColumnNameToMaxLength(string columnName, GenericDialect dialect, int referenceMaxLength)
{
if (dialect.MaxAliasLength > referenceMaxLength)
columnName = new string('w', dialect.MaxAliasLength - referenceMaxLength) + columnName;
else if (dialect.MaxAliasLength < referenceMaxLength)
Assert.Fail("Dialect max alias length is too short for the test.");
return columnName;
}
}
}
1 change: 1 addition & 0 deletions src/NHibernate.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
Expand Down
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/DB2Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ public override string ForUpdateString
get { return " for read only with rs"; }
}

// As of DB2 9.5 documentation, limit is 128 bytes which with Unicode names could mean only 32 characters.
/// <inheritdoc />
public override int MaxAliasLength => 32;

#region Overridden informational metadata

public override bool SupportsEmptyInList => false;
Expand Down
9 changes: 5 additions & 4 deletions src/NHibernate/Dialect/Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2282,10 +2282,11 @@ public virtual string LowercaseFunction
get { return "lower"; }
}

public virtual int MaxAliasLength
{
get { return 10; }
}
// 18 is the smallest of all dialects we handle.
/// <summary>
/// The maximum length a SQL alias can have.
/// </summary>
public virtual int MaxAliasLength => 18;

/// <summary>
/// The syntax used to add a column to a table. Note this is deprecated
Expand Down
6 changes: 6 additions & 0 deletions src/NHibernate/Dialect/FirebirdDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,12 @@ private void RegisterTrigonometricFunctions()
RegisterFunction("tanh", new StandardSQLFunction("tanh", NHibernateUtil.Double));
}

// As of Firebird 2.5 documentation, limit is 30/31 (not all source are concordant), with some
// cases supporting more but considered as bugs and no more tolerated in v3.
// It seems it may be extended to 63 for Firebird v4.
/// <inheritdoc />
public override int MaxAliasLength => 30;
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems we still have a bug. A test has generated the alias requestcompletionstatus3_8691_0_ for a query, that is 32 characters long.

Copy link
Member

Choose a reason for hiding this comment

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

It's because the identifier has 2 "unique" parts for some reasons.

Copy link
Member

Choose a reason for hiding this comment

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

It's in "Column.cs", line 173.

Copy link
Member

Choose a reason for hiding this comment

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

Or not. I'm confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am going to have a look. I am currently adjusting lengths and sources for those lengths.

Copy link
Member

Choose a reason for hiding this comment

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

It is from SelectFragment.cs suffix field. Which is a "collectionSuffix" which is a table number + _.

Copy link
Member Author

Choose a reason for hiding this comment

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

Run one by one, the tests succeed. It seems there is some coupling between tests.

Copy link
Member Author

@fredericDelaporte fredericDelaporte Nov 2, 2017

Choose a reason for hiding this comment

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

The coupling is very likely coming from the thread static tableCounter in Table class. It seems it endlessly grows as we build/discard session factories configuration.
Either this field should be reset when starting to build a session factory (if it has no more usages after build) or it should be bound to the session factory configuration instead of the thread on which it was built. (Moved to #1417.)

The SelectFragment suffix is an additional suffix which is meant (at least in the cases I have checked) to differentiate columns of the same table in case it is auto-joined. It does not detect auto-join but simply always add the suffix according to query joins count.

It then means the Column.GetAlias should account for that potential additional suffix by not exhausting the full maximum length. At least 3 characters should be left in my opinion, for supporting more than 9 joins (up to 99) and the additional _ of the suffix. (Moved to #1418.)

If we want want to support more than 99 joins (up to 999), we should keep 4 characters left, but I am not used to have such monstrous queries with NHibernate.


#region Informational metadata

/// <summary>
Expand Down
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/InformixDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,10 @@ public override string GetAddForeignKeyConstraintString(string constraintName, s

return res.ToString();
}

// Informix 7 is said on Internet to be limited to 18. (http://www.justskins.com/forums/length-of-columns-names-143294.html)
/// <inheritdoc />
public override int MaxAliasLength => 18;
}

public class IfxViolatedConstraintExtracter : TemplatedViolatedConstraintNameExtracter
Expand Down
5 changes: 4 additions & 1 deletion src/NHibernate/Dialect/InformixDialect0940.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,5 +146,8 @@ public override bool SupportsLimitOffset
get { return false; }
}

// Informix 9 is said on Internet to be limited to 128. (http://www.justskins.com/forums/length-of-columns-names-143294.html)
/// <inheritdoc />
public override int MaxAliasLength => 128;
};
}
}
9 changes: 9 additions & 0 deletions src/NHibernate/Dialect/IngresDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ public IngresDialect()
DefaultProperties[Environment.ConnectionDriver] = "NHibernate.Driver.IngresDriver";
}

// Ingres 10.2 supports 256 bytes (so worst unicode case would mean 64 characters), but I am unable to find
// the limit for older versions, excepted many various sites mention a 32 length limit.
// https://unifaceinfo.com/docs/0906/Uniface_Library_HTML/ulibrary/INS_NAMING_RULES_8EEFC1A489331BF969D2A8AA36AF2832.html
// There are traces of a ticket for increasing this in version 10: http://lists.ingres.com/pipermail/bugs/2010-May/000052.html
// This dialect seems to target version below 9, since there is Ingres9Dialect deriving from it.
// So sticking to 32.
/// <inheritdoc />
public override int MaxAliasLength => 32;

#region Overridden informational metadata

public override bool SupportsEmptyInList => false;
Expand Down
5 changes: 5 additions & 0 deletions src/NHibernate/Dialect/MsSql2000Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,11 @@ public override bool SupportsSqlBatches
get { return true; }
}

// Was 30 in "earlier version", without telling to which version the document apply.
// https://msdn.microsoft.com/en-us/library/ms191240.aspx#Anchor_3
/// <inheritdoc />
public override int MaxAliasLength => 30;

#region Overridden informational metadata

public override bool SupportsEmptyInList => false;
Expand Down
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/MsSql2005Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ public override string AppendLockHint(LockMode lockMode, string tableName)
return tableName;
}

// SQL Server 2005 supports 128.
/// <inheritdoc />
public override int MaxAliasLength => 128;

#region Overridden informational metadata

/// <summary>
Expand Down
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/MsSqlCeDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@ public override long TimestampResolutionInTicks
}
}

// SQL Server 3.5 supports 128.
/// <inheritdoc />
public override int MaxAliasLength => 128;

#region Informational metadata

/// <summary>
Expand Down
5 changes: 5 additions & 0 deletions src/NHibernate/Dialect/MySQL5Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,10 @@ public override bool SupportsInsertSelectIdentity
{
get { return true; }
}

// At least MySQL 5 is said to support 64 characters for columns, but 5.7 supports 256 for aliases.
// 64 seems quite good enough, being conservative.
/// <inheritdoc />
public override int MaxAliasLength => 64;
}
}
9 changes: 7 additions & 2 deletions src/NHibernate/Dialect/Oracle12cDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace NHibernate.Dialect
{
/// <summary>
/// A dialect specifically for use with Oracle 10g.
/// A dialect specifically for use with Oracle 12c.
/// </summary>
/// <remarks>
/// The main difference between this dialect and <see cref="Oracle12cDialect"/>
Expand Down Expand Up @@ -38,5 +38,10 @@ public override SqlString GetLimitString(SqlString querySqlString, SqlString off

return result.ToSqlString();
}

// 128 since 12.2. https://stackoverflow.com/a/756569/1178314, will
// have to do a 12-2c dialect for exploiting it, or wait for 13.
// / <inheritdoc />
//public override int MaxAliasLength => 128;
}
}
}
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/Oracle8iDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,10 @@ public override long TimestampResolutionInTicks
}
}

// 30 before 12.1. https://stackoverflow.com/a/756569/1178314
/// <inheritdoc />
public override int MaxAliasLength => 30;

#region Overridden informational metadata

public override bool SupportsEmptyInList
Expand Down
4 changes: 4 additions & 0 deletions src/NHibernate/Dialect/PostgreSQL81Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,9 @@ public override bool SupportsInsertSelectIdentity

/// <inheritdoc />
public override bool SupportsDateTimeScale => true;

// Said to be 63 bytes at least since v8.
/// <inheritdoc />
public override int MaxAliasLength => 63;
}
}
6 changes: 5 additions & 1 deletion src/NHibernate/Dialect/SQLiteDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ public override bool SupportsForeignKeyConstraintInAlterTable
/// </remarks>
public override bool SupportsConcurrentWritingConnections => false;

// Said to be unlimited. http://sqlite.1065341.n5.nabble.com/Max-limits-on-the-following-td37859.html
/// <inheritdoc />
public override int MaxAliasLength => 128;

[Serializable]
protected class SQLiteCastFunction : CastFunction
{
Expand All @@ -407,4 +411,4 @@ protected override bool CastingIsRequired(string sqlType)
}
}
}
}
}
23 changes: 15 additions & 8 deletions src/NHibernate/Mapping/Column.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,14 @@ public string GetQuotedName(Dialect.Dialect d)
return IsQuoted ? d.QuoteForColumnName(_name) : _name;
}

// Accommodate the one character suffix appended in AbstractCollectionPersister and
// the SelectFragment suffix up to 99 joins.
private const int _charactersLeftCount = 4;

/// <summary>
/// For any column name, generate an alias that is unique to that
/// column name, and also take Dialect.MaxAliasLength into account.
/// It keeps four characters left for accommodating additional suffixes.
/// </summary>
public string GetAlias(Dialect.Dialect dialect)
{
Expand All @@ -127,6 +131,7 @@ public string GetAlias(Dialect.Dialect dialect)

private string GetAlias(int maxAliasLength)
{
var usableLength = maxAliasLength - _charactersLeftCount;
var name = CanonicalName;
string alias = name;
string suffix = UniqueInteger.ToString() + StringHelper.Underscore;
Expand All @@ -148,25 +153,27 @@ private string GetAlias(int maxAliasLength)
// reason, the checks for "_quoted" and "rowid" looks redundant. If you remove
// those checks, then the double checks for total length can be reduced to one.
// But I will leave it like this for now to make it look similar. /Oskar 2016-08-20
bool useRawName = name.Length + suffix.Length <= maxAliasLength &&
bool useRawName = name.Length + suffix.Length <= usableLength &&
!_quoted &&
!StringHelper.EqualsCaseInsensitive(name, "rowid");
if (!useRawName)
{
if (suffix.Length >= maxAliasLength)
if (suffix.Length >= usableLength)
{
throw new MappingException(
string.Format(
"Unique suffix {0} length must be less than maximum {1} characters.",
suffix,
maxAliasLength));
$"Unique suffix {suffix} length must be less than maximum {usableLength} characters.");
}
if (alias.Length + suffix.Length > maxAliasLength)
alias = alias.Substring(0, maxAliasLength - suffix.Length);
if (alias.Length + suffix.Length > usableLength)
alias = alias.Substring(0, usableLength - suffix.Length);
}
return alias + suffix;
}

/// <summary>
/// For any column name, generate an alias that is unique to that
/// column name and table, and also take Dialect.MaxAliasLength into account.
/// It keeps four characters left for accommodating additional suffixes.
/// </summary>
public string GetAlias(Dialect.Dialect dialect, Table table)
{
string suffix = table.UniqueInteger.ToString() + StringHelper.Underscore;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,11 @@ public AbstractCollectionPersister(Mapping.Collection collection, ICacheConcurre
foreach (Column col in collection.Owner.Key.ColumnIterator)
{
keyColumnNames[k] = col.GetQuotedName(dialect);
keyColumnAliases[k] = col.GetAlias(dialect) + "_owner_"; // Force the alias to be unique in case it conflicts with an alias in the entity
// Force the alias to be unique in case it conflicts with an alias in the entity
// As per Column.GetAlias, we have 3 characters left for SelectFragment suffix and one for here.
// Since suffixes are composed of digits and '_', and GetAlias is already suffixed, adding any other
// letter will avoid collision.
keyColumnAliases[k] = col.GetAlias(dialect) + "o";
k++;
}
joinColumnNames = new string[collection.Key.ColumnSpan];
Expand Down