NH-3749 - Remove unnecessary comma in CREATE TABLE statement#393
NH-3749 - Remove unnecessary comma in CREATE TABLE statement#393lesnyh wants to merge 4 commits intonhibernate:masterfrom
Conversation
|
@lesnyh can you add some tests? |
|
Yes, I will try to add some tests. But to reproduce this problem we need DBMS that doesn't support not null unique constraints (dialect.SupportsNotNullUnique == false). I use DBMS Linter SQL Server and have problems in 21 tests: |
|
I think it may be a little bit hard to write a test, because now all NHibernate test dialects support not null unique constraints. I have found interesting comment in file NHibernate\Mapping\Index.cs: |
hazzik
left a comment
hazzik
left a comment
There was a problem hiding this comment.
Please add some tests anyway. You can implement own TestDialect class to demonstrate issue.
src/NHibernate/Mapping/Table.cs
Outdated
| } | ||
|
|
||
| foreach (UniqueKey uk in UniqueKeyIterator) | ||
| if (dialect.SupportsNotNullUnique) |
There was a problem hiding this comment.
I think it's more accurate way is to check that uk.SqlConstraintString(dialect) returns not empty string before adding to buf
There was a problem hiding this comment.
I have corrected code.
|
Can I add our LinterDialect class for DBMS Linter SQL? |
|
That would be unnecessary and make the test bloated. Create a small dialect just for the test instead., |
|
I have corrected code and added small test. |
|
BTW, the property name SupportsNotNullUnique is a bit confusing because according to the usage of this property it means: "supports nullable columns with unique constraints". You'd better add a comment to clarify what this property mean. |
|
Can you please enable Allow edits from maintainers option on this PR? |
|
Replaced by #1651 |
3 participants
I develop NHibernate Driver for DBMS Linter SQL and found a problem in file Nhibernate\Mapping\Table.cs. The problem is in the following code:
foreach (UniqueKey uk in UniqueKeyIterator)
{
buf.Append(',').Append(uk.SqlConstraintString(dialect));
}
This code produces unnecessary comma in CREATE TABLE statement if dialect doesn't support not null unique constraints. For example, in test Nhibernate.Test.Component.Basic.ComponentWithUniqueConstraintTests.CanBePersistedWithUniqueValues we have the following query:
create table Employee (Id BIGINT not null, Name VARCHAR(255) null, Dob DATE null, HireDate DATE null, primary key (Id),);
I suggest that we check dialect.SupportsNotNullUnique before running foreach loop. Something like this:
if (dialect.SupportsNotNullUnique)
{
foreach (UniqueKey uk in UniqueKeyIterator)
{
buf.Append(',').Append(uk.SqlConstraintString(dialect));
}
}