Install-DbaMaintenanceSolution: defaults reported by documentation are incorrect

[ReeceGoding]

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

No error. The wrong thing is done silently.

Steps to Reproduce

Firstly, read the Install-DbaMaintenanceSolution documentation and observe our claims that:

• When -Compress is not specified, we use "the SQL Server instance’s default compression setting".
• Both -Verify and -CheckSum "Defaults to enabled (Y) if not specified".

Secondly, spin up an instance with backup compression defaulted to off

# Run after creating the instance
Set-DbaSpConfigure -SqlInstance $YourMachine -ConfigName 'DefaultBackupCompression' -Value 0

and run

$splatInstall = @{
	SqlInstance      = $YourMachine
	InstallJobs      = $true
	ReplaceExisting  = $true
}
Install-DbaMaintenanceSolution @splatInstall

Thirdly, note that the above definitely did not set -Compress, -Verify, or -CheckSum.

Fourthly, observe that the produced jobs do not turn on what they say they should. This

(Get-DbaAgentJobStep -SqlInstance $YourMachine -Job "DatabaseBackup - USER_DATABASES - FULL").Command

returns

EXECUTE [dbo].[DatabaseBackup]
@Databases = 'USER_DATABASES',
@Directory = N'/var/opt/mssql/data',
@BackupType = 'FULL',
@Verify = 'N',
@CleanupTime = NULL,
@CheckSum = 'N',
@LogToTable = 'N',
@Compress = 'N'

which explicitly does the right thing for @Compress, but the wrong thing for -Verify and -CheckSum.

On a version from last week, I get

EXECUTE [dbo].[DatabaseBackup]
@Databases = 'USER_DATABASES',
@Directory = N'/var/opt/mssql/data',
@BackupType = 'FULL',
@Verify = 'Y',
@CleanupTime = NULL,
@Checksum = 'Y',
@LogToTable = 'N'

which does the right thing for -Verify and -CheckSum, but is not explicit about -Compress.

From this, it is quite clear that my #10172 has a lot of the blame. By making the off switches work, I made the documentation a liar.

Please confirm that you are running the most recent version of dbatools

I'm using the current Dev and a version of it from a week ago.

Other details or mentions

Tomorrow, I intend to put in a PR adding tests for these cases as well as our claimed defaults for -BackupLocation and -StartTime. However, it is not clear how the code should be fixed. We cannot make -Verify default to $true because that would be a very naughty way to code a switch and it would break calls that do not attempt to create jobs. Equally, we cannot make -Verify default to $false without changing documented behaviour. So what should we do?

My inclination is that we should change the documentation and make both -Verify and -CheckSum opt in.

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

N/A

SQL Server Edition and Build number

N/A

.NET Framework Version

N/A

[andreasjordan]

let me copy my comment from the pr to this issue in case we what to discuss here:

I think:

• "Verify" and "Checksum" are switch parameters, so have a default of $false.
• "Verify" is an extra to the backup job that should not be enabled by default as it adds time and maybe costs.
• "Checksum" should be set to $true at the instance level with just one setting an not be included in every backup job.
• So they both should not be included in the command by default.
• "Checksum" should not be included based on the instance setting.
• They should only be included and set to "Y" if the switch parameter is used.

I am not a fan of using three states of a switch "$true / $false / not used" as it makes things like Install-DbaMaintenanceSolution -Verify:$verify -Checksum:$checksum impossible.

[ReeceGoding]

I largely agree with @andreasjordan; switches should be for off-by-default behaviour. My only disagreement is that I don't have any strong objection to "Checksum" appearing in the job texts. The commit that introduced these parameters looks to have been this one, so perhaps @potatoqualitee can give us some context?