[Oracle] Sqitch does not detect missing schema

[vectro]

Steps to reproduce:

1. Run sqitch deploy --registry xxx, where xxx is a schema that does not exist on the target database; and where there is a sqitch registry in the connecting user's schema.

Expected behavior:

• Sqitch would report an error and take no action.

Actual behavior:

• Sqitch will query the registry in the connecting user's schema instead.

[theory]

Odd. Can you apply this patch, try again, and send me the output?

--- a/lib/App/Sqitch/Command/deploy.pm
+++ b/lib/App/Sqitch/Command/deploy.pm
@@ -114,6 +114,7 @@ sub execute {
 
     # Warn on multiple targets.
     my $target = shift @{ $targets };
+    say 'Registry: ', $target->registry;
     $self->warn(__x(
         'Too many targets specified; connecting to {target}',
         target => $target->name,

[vectro]

with --registry does_not_exist, I see

Registry: does_not_exist
Nothing to deploy (already at "first-change")

[theory]

Oh interesting!

[theory]

Oh I see what's happening. Sqitch attempts to set the schema path on connect, but doesn't fail if the schema doesn't exist, on the assumption that it will be created and set on the first deploy.

https://github.com/sqitchers/sqitch/blob/f2fb8f89522b7eb9fbbb512c6c5d43f296bbe2f0/lib/App/Sqitch/Engine/oracle.pm#L112-L118

And since you're not doing a deploy, it just defaults to the current schema. It seems as though you previously used the same schema for your own database object and for Sqitch, yes? This usually doesn't fail because the ideal is to use a separate schema for the registry, and you don't specify (or default to) the registry for your target.

So my short answer is "don't use your own schema for the registry".

The longer response is…I'm not sure this is something that'd be easy to change.

[vectro]

Hmm, in Oracle at least I wouldn't expect Sqitch to automatically create the registry schema.

That is because in Oracle there is an isomorphism between schemas and users, and to me creating users is something that shouldn't be done implicitly.

Do we rely on this error-swallowing functionality?

[theory]

Yep:

https://github.com/sqitchers/sqitch/blob/ef7767d83d396e98c7a57222f1765bcda6189809/lib/App/Sqitch/Engine/oracle.pm#L112-L118

It gets created here:

https://github.com/sqitchers/sqitch/blob/ef7767d83d396e98c7a57222f1765bcda6189809/lib/App/Sqitch/Engine/oracle.pm#L461-L464

But the Oracle registry script doesn't create the schema (user). Instead, according to the tutorial, it just uses the current schema. From the tutorial:

First Sqitch created the registry tables used to track database changes. The structure and name of the registry varies between databases, but in Oracle they are simply stored in the current schema -- that is, the schema with the same name as the user you've connected as. In this example, that schema is scott. Ideally, only Sqitch data will be stored in this schema, so it probably makes the most sense to create a superuser named sqitch or something similar and use it to deploy changes.

[theory]

I just worked through it all again, committing 8be3608c to improve the consistency of handling the registry username. But I still can't work out how this is happening, unless it's not capturing some errors somewhere. Would you try again with this patch, please?

--- a/lib/App/Sqitch/Engine/oracle.pm
+++ b/lib/App/Sqitch/Engine/oracle.pm
@@ -459,8 +459,11 @@ sub initialize {
 
     # Load up our database.
     (my $file = file(__FILE__)->dir->file('oracle.sql')) =~ s/"/""/g;
+    say "Initialize $schema";
     $self->_run_with_verbosity($file);
+    say "Set session to $schema";
     $self->dbh->do("ALTER SESSION SET CURRENT_SCHEMA = $schema") if $schema;
+    say "Register release in $schema";
     $self->_register_release;
 }
 
@@ -742,6 +745,7 @@ sub _script {
 sub _run {
     my $self = shift;
     my $script = $self->_script(@_);
+    say "Run $script";
     open my $fh, '<:utf8_strict', \$script;
     return $self->sqitch->spool( $fh, $self->sqlplus );
 }
@@ -749,6 +753,7 @@ sub _run {
 sub _capture {
     my $self = shift;
     my $conn = $self->_script(@_);
+    say "Capture $conn";
     my @out;
 
     require IPC::Run3;

[vectro]

So, this might be frustrating, but that patch does not result in any extra output. Presumably because Sqitch does not detect that schema initialization is needed?

[theory]

No output at all? Weird. What happens if you try to print the schema name in initialized()?

[vectro]

Okay so I dug into this a bit today and there is an extra condition required to trigger the behavior documented above.

I would amend the "steps to reproduce" above as follows:

1. Run sqitch deploy --registry xxx, where xxx is a schema that does not exist on the target database; and where there is a sqitch registry in the connecting user's schema, and where that registry contains an entry in the changes table whose project corresponds to the current project name.

What happens is that we query the changes table (in _select_state()) for this project; and if it is found, then deploy() will never call initialized().

[theory]

IIRC, it currently defaults to the current database if the sqitch registry doesn't exist, but it shouldn't fall back if the registry schema name is explicitly named, as in your example. Will have to think about how to deal with that…

[theory]

#762 should fix the issue. It detects when the registry schema does not exist at connection time, and doesn't bother to call _select_state() if it doesn't exist.

[vectro]

This seems reasonable, but maybe there is one more case that slipped through the cracks? What if:

• The default schema contains a changes table whose project corresponds to the current project name
• The registry schema exists, but is empty

It seems like in this case we will check if the registry schema exists but then still end up querying the default schema instead. What do you think?

[theory]

Ugh, yeah, that's quite the edge case, but a very real possibility. The only solution I can think of offhand is to schema-qualify the changes table in that query — for those databases that use schemas, that is. Will have to give that some thought, but perhaps in a separate PR.

[theory]

Him, the docs suggest that ALTER SESSION SET CURRENT_SCHEMA changes the default schema. I don't see anything about it falling back on the previous default schema, or a schema path. Am I missing something?