New-DbaDbTable a varchar column default should not always be quoted (e.g., newid())

[RandyInMarin]

Verified issue does not already exist?

Yes

What error did you receive?

No error.

Steps to Reproduce

# provide your command(s) executed pertaining to dbatools
# please include variable values (redacted or fake if needed) for reference

$columnMap = @(
@{ 'Name' = "Id"; 'Type' = 'int'; 'Identity' = $true }
@{ 'Name' = 'Value'; 'Type' = 'varchar'; 'MaxLength' = 36; 'Nullable' = $true; 'Default' = 'NEWID()' }
@{ 'Name' = 'Note'; 'Type' = 'varchar'; 'MaxLength' = 50; 'Nullable' = $true }
@{ 'Name' = 'CreatedAt'; 'Type' = 'datetime'; 'MaxLength' = 20; 'Nullable' = $false; 'Default' = 'GETDATE()' }
)
$tb = New-DbaDbTable -SqlInstance $sqlInstance -Database $databaseName -Schema $schema -Name $tableName -ColumnMap $columnMap

Are you running the latest release?

Yes

Other details or mentions

The GETDATE() default works correctly. The type is datetime. The issues is with varchar or related types.

This the related code.

if ($sqlDbType -in @('NVarchar', 'NChar', 'NVarcharMax', 'NCharMax')) {
$sqlColumn.AddDefaultConstraint($dfName).Text = "N'$($column.Default)'"
} elseif ($sqlDbType -in @('Varchar', 'Char', 'VarcharMax', 'CharMax')) {
$sqlColumn.AddDefaultConstraint($dfName).Text = "'$($column.Default)'"
} else {
$sqlColumn.AddDefaultConstraint($dfName).Text = $column.Default
}

Is it a string to convert or an expression to evaluate? For example, if I provided a date as a string to a datetime column, it would need quotes. However, if it an expression like GETDATE(), it can't have quotes. This applies to any type. Not sure how you can handle this column by column. Perhaps a new Boolean keyword such as DefaultEvaluated or AutoQuoteDefault to allow us to control quoting for each column? The default of the setting should not change by type because that would be too confusing.

Auto quoting does have value...don't want to be double quoting to get the quotes if they are needed. If my default is "There's no value", perhaps it would take more than adding just outer quotes. A quote_name of some sort?

What PowerShell host was used when producing this error

VS Code (integrated terminal)

PowerShell Host Version

Name Value

---

PSVersion 7.2.1
PSEdition Core
GitCommitId 7.2.1
OS Microsoft Windows 10.0.19043
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

SQL Server Edition and Build number

Microsoft SQL Server 2019 (RTM-CU12) (KB5004524) - 15.0.4153.1 (X64) Jul 19 2021 15:37:34 Copyright (C) 2019 Microsoft Corporation Developer Edition (64-bit) on Windows 10 Enterprise 10.0 (Build 19043: ) (Hypervisor)

.NET Framework Version

.NET 6.0.0-rtm.21522.10

[RandyInMarin]

Hi, I did not want to wait for a fix, so I added code for a DefaultEx property in my local copy of allcommands.ps1. I'll adapt to the new version when it's available.

<#
if ($column.Default) {
    if ($column.DefaultName) {
        $dfName = $column.DefaultName
    } else {
        $dfName = "DF_$name`_$($column.Name)"
    }
    if ($sqlDbType -in @('NVarchar', 'NChar', 'NVarcharMax', 'NCharMax')) {
        $sqlColumn.AddDefaultConstraint($dfName).Text = "N'$($column.Default)'"
    } elseif ($sqlDbType -in @('Varchar', 'Char', 'VarcharMax', 'CharMax')) {
        $sqlColumn.AddDefaultConstraint($dfName).Text = "'$($column.Default)'"
    } else {
        $sqlColumn.AddDefaultConstraint($dfName).Text = $column.Default
    }
}
#>
if ($column.DefaultEx) { # override the default that would add quotes to an expression
    if ($column.DefaultName) {
        $dfName = $column.DefaultName
    } else {
        $dfName = "DF_$name`_$($column.Name)"
    }
    $sqlColumn.AddDefaultConstraint($dfName).Text = $column.DefaultEx
} elseif ($column.Default) { # string - for now, use DefaultEx for types requiring an explicit conversion
    if ($column.DefaultName) {
        $dfName = $column.DefaultName
    } else {
        $dfName = "DF_$name`_$($column.Name)"
    }
    if ($sqlDbType -in @('Varchar', 'Char', 'VarcharMax', 'CharMax')) {
        $sqlColumn.AddDefaultConstraint($dfName).Text = "'$($column.Default)'"
    } elseif ($sqlDbType -in @('NVarchar', 'NChar', 'NVarcharMax', 'NCharMax')) {
        $sqlColumn.AddDefaultConstraint($dfName).Text = "N'$($column.Default)'"
    } else { # For a general case, do we use the more general unicode?
        # The conversion is a problem for millions of inserts?
        # It's already a conversion, so it does not matter?
        $sqlColumn.AddDefaultConstraint($dfName).Text = "N'$($column.Default)'"
    }
}

This worked as expected. NEWID() was an expression for one column and a quoted string for the other.

$columnMap = @(
	[ordered]@{ 'Name' = "Id"; 'Type' = 'int'; 'Identity' = $true }
	[ordered]@{ 'Name' = 'Value'; 'Type' = 'varchar'; 'MaxLength' = 36; 'Nullable' = $true; 'DefaultEx' = 'NEWID()' }
	[ordered]@{ 'Name' = 'Note'; 'Type' = 'varchar'; 'MaxLength' = 50; 'Nullable' = $true; 'Default' = 'NEWID()' }
	[ordered]@{ 'Name' = 'CreatedAt'; 'Type' = 'datetime'; 'MaxLength' = 20; 'Nullable' = $false; 'DefaultEx' = 'GETDATE()' }
	)
$tb = New-DbaDbTable -SqlInstance $sqlInstance -Database $databaseName -Schema $schema -Name $tableName -ColumnMap $columnMap

[andreasjordan]

I will open a pull request for that. But I will only add DefaultEx and not change the code for Default to avoid a breaking change.

[drstonephd]

This will still leave an issue if a string value for a date is passed as a default to a datetime. We can add extra quotes as a work around. For example, '''2030-01-01'''. It will be confusing because the extra quotes are not needed for string types and needed for non-string types.

There is a more serious issue than just an error. The SMO create call will create the table even if it fails to create the constraint. It will create constrains until it reaches the constraint that fails. It will also skip creating any constraints following.

I was thinking of parsing the text of the constraint. It sort of works. However, 2010-01-01 parses just fine and evaluates to 2008. So, did the user want a date or 2008? I would guess a date. So test for these odd cases using regex before the parse test? A cast to the data type might also be something include in the parse test. However, 2008 would cast to a datetime just fine, but there might be other cases were it helps. For example, it would not cast to a date.

'''
# The constraint content is independent of the column type.
# If the string parses, assume it's an expression; otherwise quote it to avoid a syntax error
if ($column.DefaultName) {
$dfName = $column.DefaultName
} else {
$dfName = "DF_$name`_$($column.Name)"
}
try {
$testConstraintSql = 'SELECT (' + $column.Default + ') as [x]'
$server.ConnectionContext.ExecuteNonQuery($testConstraintSql, [Microsoft.SqlServer.Management.Common.ExecutionTypes]::ParseOnly)
$sqlColumn.AddDefaultConstraint($dfName).Text = $column.Default
} catch {
$sqlColumn.AddDefaultConstraint($dfName).Text = "'$($column.Default)'"
}
'''

Forget it and include a switches to force the default to be an expression or string? It's starts to look odd. Default, DefaultEx/DefaultExp, and DefaultStr/DefaultNonExp? (I'm not really thinking through the names. Just noting how hard naming can be.)

[andreasjordan]

Ok, before I open a new PR, let's talk about what it should include.

First of all, I don't see much use cases for this command. What is the value in building an array of hashtables opposed to just run a "CREATE TABLE ..." with all I need in one query? This said, I don't what to put too much time in there...

To not have a breaking change, I would not touch the code for "Default".

I added "DefaultEx" without much thinking of the name - it was your proposal. It could also be "DefaultExp" or "DefaultExpression". We can also add code like this:

$DefaultExpression = $null
if ($column.DefaultExpression) { $DefaultExpression = $column.DefaultExpression }
if ($column.DefaultExp) { $DefaultExpression = $column.DefaultExp }
if ($column.DefaultEx) { $DefaultExpression = $column.$DefaultExpression }

and then use $DefaultExpression.

Then we can also add similar code for "DefaultSt"/"DefaultStr"/"DefaultString" to always use the values as a string to support fixed dates.

About the partial failure: This is a problem of the used SMO. We only run one .Create() via Invoke-Create. The SMO is not building one statement and thus have one transaction, but running one CREATE and then some ALTERs.
We would have to do a Remove-DbaDbTable or simply a .Drop() on the $object (and the $schemaObject). So we would have to change

Stop-Function -Message "Failure" -ErrorRecord $_ -Continue

to

$exeption = $_
Write-Message -Level Warning -Message "Failed to create table or failure while adding constraints. Will try to remove table."
try {
    $object.Drop()
    if ($schemaObject) {
        $schemaObject.Drop()
    }
} catch {
    Write-Message -Level Warning -Message "Failed to drop table: $_. Maybe table is still there."
}
Stop-Function -Message "Failure" -ErrorRecord $exeption -Continue

(code is not tested, so maybe some changes are needed)

@potatoqualitee What do you think?

[potatoqualitee]

I agree about DefaultExpression and prefer that one, and I like the extra attempt at the end 💯

[andreasjordan]

Neither .Dop() nor .DropIfExists() work as expected. I think this is another problem inside of the SMO...

So I will have to test if table exists at the beginning (to prevent dropping an existing table) and then try to use Remove-DbaDbTable at the end...

So should I just rename "DefaultEx" to "DefaultExpression"? Or add "DefaultExpression"?

[andreasjordan]

Ok, PR is open, please have a look at #8150.