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

Remove quote() Workaround in Oracle Engine #91

Closed
theory opened this issue May 9, 2013 · 5 comments
Closed

Remove quote() Workaround in Oracle Engine #91

theory opened this issue May 9, 2013 · 5 comments
Assignees
Milestone

Comments

@theory
Copy link
Collaborator

theory commented May 9, 2013

For some reason, the COLLECT() aggregate does not play nice with placeholders, at least in the copy of Oracle I ran. So there are a few methods I had to copy from DBIEngine to oracle.pm just to replace the placeholder with a quoted variable. These can be ripped out assuming I ever figure out what the problem was (StackOverflow question; Perl DBI enquiry. Grep oracle.pm for "stackoverflow" to find the bits to rip out.

@theory theory closed this as completed in f9213bf Jun 20, 2013
@theory
Copy link
Collaborator Author

theory commented Jun 20, 2013

Fat-finered that close.

@theory theory reopened this Jun 20, 2013
@theory
Copy link
Collaborator Author

theory commented May 2, 2014

Thanks to some detective work from Timothy Procter, it seems that the issue is that COLLECT needs its results cast to an existing data type. He got things to work with this patch:

--- lib/App/Sqitch/Engine/oracle.pm    2014-01-16 18:23:30.000000000 -0500
+++ lib/App/Sqitch/Engine/oracle.pm   2014-05-02 12:57:00.095527500 -0400
@@ -139,8 +139,8 @@
 }

 sub _listagg_format {
-    # http://stackoverflow.com/q/16313631/79202
-    return q{COLLECT(%s)};
+    ## http://stackoverflow.com/q/16313631/79202
+    return q{CAST(COLLECT(cast(%s as varchar2(512))) AS sqitch_array)};
 }

 sub _regex_op { 'REGEXP_LIKE(%s, ?)' }
@@ -622,17 +622,19 @@
         ora_type => DBD::Oracle::ORA_VARCHAR2()
     });
     $sth->execute;
+
+    my $aggcol = sprintf $self->_listagg_format, 'dependency';

     # Retrieve dependencies.
-    my ($req, $conf) = $dbh->selectrow_array(q{
+    my ($req, $conf) = $dbh->selectrow_array(qq{
         SELECT (
-            SELECT COLLECT(dependency)
+            SELECT $aggcol
               FROM dependencies
              WHERE change_id = ?
                AND type = 'require'
         ),
         (
-            SELECT COLLECT(dependency)
+            SELECT $aggcol
               FROM dependencies
              WHERE change_id = ?
                AND type = 'conflict'

@theory theory added the bug label May 2, 2014
@theory theory self-assigned this May 2, 2014
@benzvan
Copy link

benzvan commented May 29, 2014

This seems to work really well for me. I vote for a merge.

@theory
Copy link
Collaborator Author

theory commented Jun 1, 2014

It’s on my list, lots of tavel lately and overwhelmed with a million little things. I plan to fix all those places in oracle.pm that have this line:

XXX Oy, placeholders do not work with COLLECT() in this query.

Which means I should actually be able to rip out a bunch of code. Yay!

@benzvan
Copy link

benzvan commented Jun 1, 2014

I'm totally in favor of ripping out code!


Ben Zvan
Photos - http://photography.zvan.net/
Blog - http://ben.personal.zvan.net/
Flickr - http://www.flickr.com/photos/ben-zvan-photography/

On Jun 1, 2014, at 11:43 AM, David E. Wheeler notifications@github.com wrote:

It’s on my list, lots of tavel lately and overwhelmed with a million little things. I plan to fix all those places in oracle.pm that have this line:

XXX Oy, placeholders do not work with COLLECT() in this query.
Which means I should actually be able to rip out a bunch of code. Yay!


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

2 participants