Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stability fixes for Oracle #165

Closed
wants to merge 4 commits into from
Closed

Conversation

tprocter
Copy link

Can we include these 2 fixes in the next release?

@theory
Copy link
Collaborator

theory commented May 22, 2014

Um, I don't understand. I developed the Oracle tutorial, including a section that deploys reworked (and therefore including @) files, and it worked fine. Could this be a Windows issue? What file system are you on?

@tprocter
Copy link
Author

I'm able to reproduce the error on both windows and linux (Ubuntu). It
might also be related to the version of Oracle or the Oracle client.
Here you can see the error directly in sqlplus:

dev@devVM ~ $ sqlplus -v

SQL*Plus: Release 11.2.0.2.0 Production

dev@devVM ~ $ touch test.sql
dev@devVM ~ $ touch test@tag.sql
dev@devVM ~ $ sqlplus dbadeploy@XE

SQL*Plus: Release 11.2.0.2.0 Production on Thu May 22 18:44:03 2014

Connected to:
Oracle Database 11g Express Edition Release 11.2.0.2.0 - 64bit Production

SQL> @test.sql
SQL> @test@tag.sql
SP2-0310: unable to open file "testXEtag.sql"

On 22/05/2014 5:18 PM, David E. Wheeler wrote:

Um, I don't understand. I developed the Oracle tutorial, including [a
section|https://metacpan.org/pod/sqitchtutorial-oracle#Merges-Mastered] that
deploys reworked (and therefore including |@|) files, and it worked
fine. Could this be a Windows issue? What file system are you on?


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP

@theory
Copy link
Collaborator

theory commented May 23, 2014

Hrm. I used the Developer days VM, and connected with the x86 instant client on OS X, which was 11.1. Maybe that makes a difference?

Also, I thought Sqitch spooled the script into SQL_Plus by piping the script into SQL_Plus:

my $script = $self->_script(@_);
open my $fh, '<:utf8_strict', \$script;
return $self->sqitch->spool( $fh, $self->sqlplus );

Am I missing something?

@tprocter
Copy link
Author

I looked into it a bit further.

  • The database version could be the problem here. I don't have an 11.1
    version to test with at this time to confirm, but we probably want this
    to work regardless of the version.
  • The "_script" method might be a bit misleading. It doesn't return the
    SQL change file directly, but returns a generated SQL script as a
    wrapper that calls the change file within the sqlplus session, so we're
    spooling a wrapper rather than a change file.

On 22/05/2014 9:19 PM, David E. Wheeler wrote:

Hrm. I used the Developer days VM
http://www.oracle.com/technetwork/database/enterprise-edition/databaseappdev-vm-161299.html,
and connected with the x86 instant client on OS X
http://www.oracle.com/technetwork/topics/intel-macsoft-096467.html,
which was 11.1. Maybe that makes a difference?

Also, I thought Sqitch spooled the script into SQL_Plus by piping the
script into SQL_Plus:

|my $script = $self->script(@);
open my $fh, '<:utf8_strict', $script;
return $self->sqitch->spool( $fh, $self->sqlplus );
|

Am I missing something?


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP
Software Development Manager

Pythian - Love your data

procter@pythian.com

Tel: +1 613 565 8696 x 1292
Mobile: +1 613 266 3525
www.pythian.com http://www.pythian.com

@theory
Copy link
Collaborator

theory commented May 23, 2014

Ah, right. So, could the _run() and _capture() methods handle the temporary file without the need for a _cleanup() method?

@tprocter
Copy link
Author

That would also work. I went this way to give us another chance to run
the cleanup if the command throws an exception, which is caught in a
try-catch further up the chain. I won't be offended if you want to
switch strategies for that.

On 22/05/2014 10:13 PM, David E. Wheeler wrote:

Ah, right. So, could the |_run()| and |_capture()| methods handle the
temporary file without the need for a |_cleanup()| method?


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP
Software Development Manager

Pythian - Love your data

procter@pythian.com

Tel: +1 613 565 8696 x 1292
Mobile: +1 613 266 3525
www.pythian.com http://www.pythian.com

Include fixes from David for Issue#166 (possible failed rollback)
@theory
Copy link
Collaborator

theory commented Jun 2, 2014

Reading this thread, I'm wondering if it works if you quote the file name or escape the @ with a backslash. Also, are you using the 10.2.0.1.0 client? It appears to be a bug in that version.

@theory theory closed this in e5c9fb7 Jun 2, 2014
@theory
Copy link
Collaborator

theory commented Jun 2, 2014

I think e5c9fb7 should fix the issue. I replicated the issue with my copy of SQL*Plus, then found the answer in this post. Can you confirm, @tprocter?

@tprocter
Copy link
Author

tprocter commented Jun 2, 2014

Using ESCCHAR should work in Oracle client versions 11.1 and up
(http://docs.oracle.com/cd/B28359_01/server.111/b31189/whatsnew.htm),
but not in older versions. For older versions, you could use "SET
ESCAPE " to define "" as an escape character, then prefix special
characters ($, @, ?) with that.

On 02/06/2014 5:14 PM, David E. Wheeler wrote:

I think e5c9fb7 e5c9fb7
should fix the issue. I replicated the issue with my copy of SQL*Plus,
then found the answer in this post
https://community.oracle.com/thread/2372119. Can you confirm,
@tprocter https://github.com/tprocter?


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP

@theory
Copy link
Collaborator

theory commented Jun 3, 2014

Bah, alas, no, ESCAPE seems to only apply to escaping & variables. ESCCHAR is clearly the right solution, but what version of SQL*Plus are you using?

theory added a commit that referenced this pull request Jun 3, 2014
Instead of just relying on `ESCCHAR`, because that's only available on
SQL*Plus 11.1 or higher, and we claim to support 8.4.0! So we instead create a
temporary directory, which is deleted on exit, and either symlink to that
directory, or copy the contents of the file (on Windows).

Should be a better solution to issue #165.
@theory
Copy link
Collaborator

theory commented Jun 3, 2014

@tprocter Can you confirm that ee0d825 fixes the issue for you? Nice thing about File::Temp is that it will clean up for us on exit, so no need for additional cleanup methods.

@tprocter
Copy link
Author

tprocter commented Jun 3, 2014

Yeah, this seems to work. For reference, most of my tests are on Windows 7, Oracle client 12.

@theory
Copy link
Collaborator

theory commented Jun 3, 2014

Thanks. Can you also give it a try on your Ubuntu system, if you still have it lying around? Thanks!

@theory theory reopened this Jun 3, 2014
@tprocter
Copy link
Author

tprocter commented Jun 3, 2014

Glad you asked. I'm hitting the error again. It looks like it won't
accept a symlink as the script name. I switched to code to use
file-copy_to for both windows and other, and it works fine again.

On 03/06/2014 11:02 AM, David E. Wheeler wrote:

Thanks. Can you also give it a try on your Ubuntu system, if you still
have it lying around? Thanks!


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP
Software Development Manager

Pythian - Love your data

procter@pythian.com

Tel: +1 613 565 8696 x 1292
Mobile: +1 613 266 3525
www.pythian.com http://www.pythian.com

@theory
Copy link
Collaborator

theory commented Jun 3, 2014

Hrm. I just tried it. Created a symlink and ran sqlplus 11.1 and pointed it at the symlink and it ran fine. You on 10.x still? What is the error message you get?

@tprocter
Copy link
Author

tprocter commented Jun 4, 2014

I was using 11.2 for that. I ran another test, and the symlink works if
it has an absolute path. Importing Cwd, you can create the symlink with:

symlink abs_path($file), $alias;

On 03/06/2014 7:46 PM, David E. Wheeler wrote:

Hrm. I just tried it. Created a symlink and ran sqlplus 11.1 and
pointed it at the symlink and it ran fine. You on 10.x still?


Reply to this email directly or view it on GitHub
#165 (comment).

Timothy Procter, P.Eng., CSDP
Software Development Manager

Pythian - Love your data

procter@pythian.com

Tel: +1 613 565 8696 x 1292
Mobile: +1 613 266 3525
www.pythian.com http://www.pythian.com

@theory
Copy link
Collaborator

theory commented Jun 4, 2014

Wait, what? It should always be an absolute path. That's what File::Temp provides. Will have to dig some more, I guess.

theory added a commit that referenced this pull request Jun 4, 2014
Hope this is the final nail in the coffin for #165.
@theory
Copy link
Collaborator

theory commented Jun 4, 2014

Sorry @tprocter, read too fast yesterday. Would you try again with 56140c8, please? Thank you for all your help with this, much appreciated!

@tprocter
Copy link
Author

tprocter commented Jun 4, 2014

PASS - works for me now.

@theory
Copy link
Collaborator

theory commented Jun 4, 2014

Phew, thanks!

@theory theory closed this Jun 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants