Snowflake engine does not support fulle range of characters in role/warehouse identifiers

[marc-marketparts]

If you use special characters like dot "." in either a role or a warehouse name, SQITCH will fail executing USE ROLE <role_name>, or USE WAREHOUSE <warehouse_name>.

As stated on Snowflake docs, identifiers can include a wide range of characters in object identifiers if they are quoted.

So far, the Sqitch snowflake engine supports unquoted identifiers only.

sqitch/lib/App/Sqitch/Engine/snowflake.pm

Lines 214 to 217 in 8f72ee6

	($role ? ("USE ROLE $role") : ()),
	"ALTER WAREHOUSE $wh RESUME IF SUSPENDED",
	"USE WAREHOUSE $wh",
	'USE SCHEMA ' . $self->registry,

The solution could be :

• quoting the role or warehouse name in the code above systematically, with a small regression for users who did not set their SQITCH role/warehouse names with the right case (quoted identifiers are case sensistive, unquoted ones case insensitive)
• detect if the role/warehouse names have to be quoted or not, and quote them or not accordingly.

[theory]

Would you try the change in #688 please? Thank you!

[marc-marketparts]

Hi,
I use docker sqitch, so could give me some hints (or point me to the right doc) to build the image from your branch ?

[theory]

Oh. Bah. The docker project Only knows how to build released versions. Maybe we should look into changing that. Easiest might be to make a new Dockerfile, build FROM sqitch:v1.3.1-snowflake, and apply this patch (using patch -p1 < path/to/patch.diff:

--- a/lib/perl5/App/Sqitch/Engine/snowflake.pm
+++ b/lib/perl5/App/Sqitch/Engine/snowflake.pm
@@ -192,8 +192,6 @@ has dbh => (
         my $self = shift;
         $self->use_driver;
         my $uri = $self->uri;
-        my $wh = $self->warehouse;
-        my $role = $self->role;
         DBI->connect($uri->dbi_dsn, $self->username, $self->password, {
             PrintError        => 0,
             RaiseError        => 0,
@@ -210,11 +208,13 @@ has dbh => (
                 connected => sub {
                     my $dbh = shift;
                     try {
+                        my $wh = $dbh->quote_identifier($self->warehouse);
+                        my $role = $self->role;
                         $dbh->do($_) for (
-                            ($role ? ("USE ROLE $role") : ()),
+                            ($role ? ("USE ROLE " . $dbh->quote_identifier($role)) : ()),
                             "ALTER WAREHOUSE $wh RESUME IF SUSPENDED",
                             "USE WAREHOUSE $wh",
-                            'USE SCHEMA ' . $self->registry,
+                            'USE SCHEMA ' . $dbh->quote_identifier($self->registry),
                             'ALTER SESSION SET TIMESTAMP_TYPE_MAPPING=TIMESTAMP_LTZ',
                             "ALTER SESSION SET TIMESTAMP_OUTPUT_FORMAT='YYYY-MM-DD HH24:MI:SS'",
                             "ALTER SESSION SET TIMEZONE='UTC'",
@@ -224,7 +224,9 @@ has dbh => (
                     return;
                 },
                 disconnect => sub {
-                    shift->do("ALTER WAREHOUSE $wh SUSPEND");
+                    my $dbh = shift;
+                    my $wh = $dbh->quote_identifier($self->warehouse);
+                    $dbh->do("ALTER WAREHOUSE $wh SUSPEND");
                     return;
                 },
             },

[psabo]

This change seems to have had some unintended consequences. My automated deploys started failing on 8/2 and no code changes were made. I finally had time to dig into the issue today and noticed the difference in "USE SHCEMA" statements below. The first query, which was successful, was ran using sqitch 1.3.1. A client running 1.4 tried to deploy the same exact change and ran the improperly quoted statement in the second query. Pinning 1.3.1 resolved the issue for me.

USE SCHEMA UTIL_DB.SQITCH_REPORTING_DEVELOPMENT 01ae4fd0-0403-7178-000d-340302b9451a Success MIGRATIONS_USER — 44ms 8/14/2023, 3:05 PM
USE SCHEMA "UTIL_DB.SQITCH_REPORTING_DEVELOPMENT" 01ae4f8a-0403-7178-000d-340302b9444a Failed MIGRATIONS_USER — 28ms 8/14/2023, 3:02 PM

[theory]

Bloody hell, you can reference a schema in a different database???

[theory]

@marc-marketparts Were you able to make it work by quoting the role and warehouse identifiers yourself? I'm beginning to think the Sqitch should do no quoting itself. :-(

[marc-marketparts]

@theory
I tried to include double quotes in the connection string but it failed a couple month ago. We ended up creating a specific custom role derogating to our naming convention.

What I have tried is:

1. using single quotes to enclose uri string
   [target "staging_app"] uri = 'db:snowflake:///STAGING_APP?Driver=Snowflake&role="STAGING__APP_SQITCH"&warehouse=STAGING_ADMIN'
   but it fails with error No engine specified by URI db:'db:snowflake:///STAGING_APP?Driver=Snowflake&role=STAGING__APP_SQITCH&warehouse=STAGING_ADMIN'; URI must start with "db:$engine:"

2. escaping double quotes in uri string
   [target "staging_app"] uri = "db:snowflake:///STAGING_APP?Driver=Snowflake&role=\"STAGING__APP_SQITCH\"&warehouse=STAGING_ADMIN"
   but it fails because the escaping character backslash is passed as is to Sbowflake
   Role '\"STAGING__APP_SQITCH\"' specified in the connect string does not exist or not authorized.

The good news is that I have just gave it another try with a new idea and it works !
We have to pass the double quote in the connection string as the urlencoded value %22.
[target "staging_app"] uri = "db:snowflake:///STAGING_APP?Driver=Snowflake&role=%22STAGING__APP_SQITCH%22&warehouse=STAGING_ADMIN"
I have checked the Snowflake query history and the the role is double quoted USE ROLE "STAGING__APP_SQITCH" .

So it is a matter of documentation and this dev can be reverted.

[theory]

Ah yes, I didn't realize you were specifying it in the URI, which yes, must be percent-escaped. I will get this reverted and some docs added shortly.

[theory]

Okay have a look at #780. Might want to try using the environment variables, too! :-)