Invoke-Sqlcmd: -Variable parameter as string parsing changed in v22

[AndreaCuneo]

In v21 we used -Variable in Invoke-Sqlcmd as follows

Invoke-Sqlcmd ... -Variable "MyPath = D:\temp\drop\sql\"

This stopped working in v22.

We've mitigated removing the spaces around '='

Invoke-Sqlcmd ... -Variable "MyPath=D:\temp\drop\sql\"

Any chance to have this fixed and released soon?

The problem is that we use the original format in hundreds of Azure DevOps Pipelines which means it takes days of effort to change them all across all customers we manage.

The issue started a couple of days ago when a new version (v20230407.1) of the Agent Image of Azure DevOps has been published which updated this module to v22.

The issue is that pinning the older Image or fixing this takes the same amount of time as require changes to all pipelines.

[kripa-2019]

I have seen 2 other customers report issues with variable resolution after the upgrade to SqlServer: 22.0.59

The error as encountered in the pipeline tasks using this power shell version:

[debug]Caught exception from task script. 2023-04-13T20:19:46.7808116Z ##[debug]Error record: 2023-04-13T20:19:46.8400992Z ##[debug]Variable FooBar is not defined.Check out how to troubleshoot failures at https://aka.ms/sqlazuredeployreadme#troubleshooting- 2023-04-13T20:19:46.8414841Z ##[debug]At D:\a_tasks\SqlAzureDacpacDeployment_XXXXXXXXXXX970f\1.219.0\DeploySqlAzure.ps1:220 char:5 2023-04-13T20:19:46.8428789Z ##[debug]+ throw $errorMessage 2023-04-13T20:19:46.8442848Z ##[debug]+ ~~~~~~~ 2023-04-13T20:19:46.8457217Z ##[debug] + CategoryInfo : OperationStopped: (Variable UserAc...roubleshooting-:String) [], RuntimeException 2023-04-13T20:19:46.8471694Z ##[debug] + FullyQualifiedErrorId : Variable UserAccessGroup is not defined.Check out how to troubleshoot failures at https: //aka.ms/sqlazuredeployreadme#troubleshooting- 2023-04-13T20:19:46.8491306Z ##[debug] 2023-04-13T20:19:46.8512307Z ##[debug]Script stack trace: 2023-04-13T20:19:46.8558060Z ##[debug]at , D:\a_tasks\SqlAzureDacpacDeployment_XXXX1d37a9ab970f\1.219.0\DeploySqlAzure.ps1: line 220 2023-04-13T20:19:46.8576344Z ##[debug]at , : line 1 2023-04-13T20:19:46.8591070Z ##[debug]at , : line 22 2023-04-13T20:19:46.8605396Z ##[debug]at , : line 18 2023-04-13T20:19:46.8625580Z ##[debug]at , : line 1 2023-04-13T20:19:46.8641985Z ##[debug]Exception: 2023-04-13T20:19:46.8688669Z ##[debug]System.Management.Automation.RuntimeException: Variable UserAccessGroup is not defined.Check out how to troubleshoot failures at https://aka.ms/sqlazuredeployreadme#troubleshooting- 2023-04-13T20:19:46.8959904Z ##[error]Variable FooBar is not defined.Check out how to troubleshoot failures at https://aka.ms/sqlazuredeployreadme#troubleshooting- 2023-04-13T20:19:46.8969489Z ##[debug]Processed: ##vso[task.logissue type=error]Variable FooBar is not defined.Check out how to troubleshoot failures at https://aka.ms/sqlazuredeployreadme#troubleshooting- 2023-04-13T20:19:46.8971280Z ##[debug]Processed: ##vso[task.complete result=Failed] 2023-04-13T20:19:48.5667882Z ##[section]Finishing: Deploy Pre Environment Script

[Matteo-T]

In v21 we used -Variable in Invoke-Sqlcmd as follows

Invoke-Sqlcmd ... -Variable "MyPath = D:\temp\drop\sql\"

This stopped working in v22.

We've mitigated removing the spaces around '='

Awww... the change was to make it a little more predicatable what the intention is and to allow passing values that are indeed spaces. Would it help if we assume:

• The variable name is anything that is before the first '=' sign
• Spaced before/after the variable name are ignored/trimmed
• The value is not trimmed at all, thus allowing a value that happened to have a leading space

In other word, would you scripts work if I change the logic to accept both Invoke-Sqlcmd ... -Variable "MyPath = D:\temp\drop\sql\" Invoke-Sqlcmd ... -Variable "MyPath= D:\temp\drop\sql\" and consider them equivalent?

IOW, is the issue with your scripts on (1) the variable name (2) the value (3) both?

If that does not work, I suppose we could go back to the old (weird) behavior and, for scenarios that are impossible to express using the old "var=value" syntax, have the users use the new @{ Var = Value } syntax.

What do you think?

[Matteo-T]

@kripa-2019 - I'm sorry, the stuff you pasted here are not helpful for me to understand what the issue is. Any chance you could provide examples of what those .ps1 are actually doing? It would help in making sure that whatever change I made to accommodate what @AndreaCuneo reported would also help you.

[potatoqualitee]

If that does not work, I suppose we could go back to the old (weird) behavior and, for scenarios that are impossible to express using the old "var=value" syntax, have the users use the new @{ Var = Value } syntax.

We use hashtables as well and it's a ton easier, IMO. Changing it would be going a step back, though the current way does break compatibility. Perhaps what you could do is support both by making Variable a PSObject, detecting if it's an array or hashtable, and handle it that way? I imagine I'd just build the hashtable for them based off of their array to keep the implementation consistent.

[Matteo-T]

It is already taking a PSObject and handles both the array (legacy) and the hashtable (more modern). The hashtable won't go away for sure: I had to introduce it to overcome a bunch of problems with the array / string like the spaces and equal signs (quite common in tokens and things like that).

v22 is a major version, so breaking changes are my moment to finally fix things (and I was too gentle, IMHO).

I just don't like that " Var1 = Value1" and " Var1=Value1" would be the same thing: it seemed plain wrong. I could make an exception in forgiving the use of spaces in the variable name (in the case of the (array of) string(s) under the assumption that is is innatural (or impossible - I'm not sure, I'd have to check the specs for sqlcmd.exe) to have a space in a variable name.

[potatoqualitee]

ahhh I see, got it. i do forget that it's intended to mostly align with sqlcmd

[AndreaCuneo]

In v21 we used -Variable in Invoke-Sqlcmd as follows Invoke-Sqlcmd ... -Variable "MyPath = D:\temp\drop\sql\" This stopped working in v22. We've mitigated removing the spaces around '='

Awww... the change was to make it a little more predicatable what the intention is and to allow passing values that are indeed spaces. Would it help if we assume:

• The variable name is anything that is before the first '=' sign
• Spaced before/after the variable name are ignored/trimmed
• The value is not trimmed at all, thus allowing a value that happened to have a leading space

In other word, would you scripts work if I change the logic to accept both Invoke-Sqlcmd ... -Variable "MyPath = D:\temp\drop\sql" Invoke-Sqlcmd ... -Variable "MyPath= D:\temp\drop\sql" and consider them equivalent?

IOW, is the issue with your scripts on (1) the variable name (2) the value (3) both?

If that does not work, I suppose we could go back to the old (weird) behavior and, for scenarios that are impossible to express using the old "var=value" syntax, have the users use the new @{ Var = Value } syntax.

What do you think?

Unfortunatly the error would persist. I've checked using -Variable "MyPath= D:\temp\drop\sql" with the current version and it still fails as where is used the ' ' in front cause a relative vs absolute resolution of the path.

As far as I can tell from the error message, the space after the variable name is already handled as you-re suggesting as the Variable itself is found but the replacement is different from v21. When I use MyPath = D:.... or MyPath= D:... I have the same error message with v22 and is due to the Path being interpreted as relative instead of absolute while still containing the MyPath contents.

As far as I can tell, the required resolution is for trimming the whitespaces around =.

I think that the old parser should be restored and if someone need the more stable parsing can use the Hash/Dictionary which would not mangle the Value at all.

[Matteo-T]

@AndreaCuneo, Weird. I'll take a look when I get a chance.

In the meanwhile, could you just share the script where that variable is used or something that shows the actual issue? I don't see an obvious correlation between the presence/absence of a space and the path being interpreted as a relative/absolute path. If you could share something, that would be very useful.

[AndreaCuneo]

@Matteo-T the script is a SQLCMD script (contents below) and the Variable is used as working directory. The 0001_MigrateX.sql can be anything.

:r $(MyPath)"\0001_MigrateX.sql"
GO

The error message is

Cannot find path 'D:\a\_tasks\SqlAzureDacpacDeployment_ce85a08b-a538-4d2b-8589-1d37a9ab970f\1.219.0\ D:\a\r1\a\XXXX\drop\sql\\0001_MigrateX.sql' because it does not exist.

The D:\a\_tasks\SqlAzureDacpacDeployment_ce85a08b-a538-4d2b-8589-1d37a9ab970f\1.219.0\ is the 'current working directory' while D:\a\r1\a\XXXX\drop\sql\ is the content of the MyPath variable (space in front under debate).

In v22 fails with the same error either with -Variable "MyPath= D:\a\r1\a\XXXX\drop\sql\" or -Variable "MyPath = D:\a\r1\a\XXXX\drop\sql\" but works with -Variable "MyPath=D:\a\r1\a\XXXX\drop\sql\".

In v21 -Variable "MyPath = D:\a\r1\a\XXXX\drop\sql\" was working.

[Matteo-T]

@AndreaCuneo I may have found a solution to the problem that would not sacrifice the new semantic and essentially make the :r logic a little more forgiving and re-enable your scenario. Essentially, an LTRIM only in the :r case when the path "looks quasi-absolute". And, I'll make a note of this scenario in the documentation.

I'll need a little time to think about it and make sure nothing else breaks....

[kripa-2019]

@kripa-2019 - I'm sorry, the stuff you pasted here are not helpful for me to understand what the issue is. Any chance you could provide examples of what those .ps1 are actually doing? It would help in making sure that whatever change I made to accommodate what @AndreaCuneo reported would also help you.

Cust is trying to publish a Azure SQL Database via azure pipelines, using "Azure SQL Database Deployment" task - (https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks/SqlAzureDacpacDeploymentV1/README.md) -- which internally uses this PowerShell module to publish the script.

A gist of their script for the variable FooBar is:

SET @Msg = NULL IF (ISNULL(LTRIM(RTRIM(N'$(FooBar)')), '') = '') BEGIN SET @Msg = 'Required variable FooBar is not set'; THROW 51024, @Msg, 1; END ELSE BEGIN PRINT CHAR(9) + 'Variable - OK'; END;

---

--Create Group Login for users that have direct SQL access

USE [master]; PRINT 'Create Group Login for users that have direct SQL access';

IF NOT EXISTS (SELECT * FROM sys.server_principals WHERE name = N'$(FooBar)') BEGIN PRINT CHAR(9) + 'Creating Login for Group ' + N'$(FooBar)'; CREATE LOGIN [$(FooBar)] FROM EXTERNAL PROVIDER; END ELSE BEGIN PRINT CHAR(9) + 'Login for Group ' + N'$(FooBar)' + ' already exists'; END; PRINT CHAR(13);

[AndreaCuneo]

@Matteo-T any update on this?

[Matteo-T]

Sorry, I've been out sick for a few days and I'm currently busy with other stuff. I won't be able to look into this any time soon.

[Matteo-T]

@AndreaCuneo / @kripa-2019 as much as I hate it, I decided to revert the behavior to what v21 was doing, i.e. when passing the variables using the -Variables <array | string>, the value is trimmed.

My recommendation would be to stop relying on this buggy behavior. The reason I'm saying buggy is because it deviates from the behavior of SqlCmd.exe.

The upcoming v22.1 (which is expected to roll out by the end of this week) should have this issue addressed.

[AndreaCuneo]

@Matteo-T thanks for the update. Internally I'm promoting migration to hash-dictionary approach but having this solved is of great help for all legacy projects so that they do not break at first change.

Not sure if possible, but I'd welcome a WARN message promoting the use of safe Variable passing (i.e. hash) instead of plain string to avoid further use of the 'buggy' mode.

[Matteo-T]

I can't promise the warning, but the rollback just got merged. I am looking at one more issue to see if I can squeeze it in at minimal risk. Either way, I expect to push this one out in the next 24h or so (unless the unthinkable happens...)

[Matteo-T]

This was fixed in v22.1.1