Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
Arthur Axel 'fREW' Schmidt committed Aug 22, 2015
1 parent 0011e48 commit 8570231
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ sub results_exist {
->result_source
->resultset
->search({ -exists => $self->as_query })
->first
->search(undef, {
columns => { exists => \1 },
rows => 1,
})
->get_column('exists')
}

1;

8 comments on commit 8570231

@ribasushi
Copy link
Contributor

@ribasushi ribasushi commented on 8570231 Nov 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frioux I think what you want here is the following:

sub results_exist {
   my $self   = shift;

   my $reified = $self->get_column( \1 )->as_query;

   # this would have been the "clean" way to do it
   # alas we can not use it - we do not know what WHERE's a fresh
   # resultset would have tacked on it via default_attrs
   #$$reified->[0] = "EXISTS $$reified->[0]";
   #
   #( $self->result_source->resultset->get_column($reified)->all )[0];
                                                   
                                                   
   my $storage = $self->result_source->schema->storage;
   my( $sql, @bind ) = @$$reified;

   my( undef, $sth ) = $storage->dbh_do( _dbh_execute =>
      "SELECT EXISTS $sql",
      \@bind,
      $storage->_dbi_attrs_for_bind( undef, \@bind ),
   );

   my $rv = $sth->fetchall_arrayref;
   $rv->[0][0];
}

@ribasushi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frioux having had to work with this more - this is actually what you want. However this is no longer a ::Shortcut...

diff --git a/lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm b/lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm
index f60d710..eb36bb2 100644
--- a/lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm
+++ b/lib/DBIx/Class/Helper/ResultSet/Shortcut/ResultsExist.pm
@@ -5,14 +5,41 @@ use warnings;
 
 use parent 'DBIx::Class::ResultSet';
 
+sub results_exist_as_query {
+   my $self = shift;
+
+   my $reified = $self->get_column( \1 )->as_query;
+
+   # this would have been the "clean" way to do it
+   # alas we can not use it - we do not know what WHERE's a fresh
+   # resultset would have tacked on it via default_attrs
+   #$$reified->[0] = "EXISTS $$reified->[0]";
+   #
+   #( $self->result_source->resultset->get_column($reified)->all )[0];
+
+   $$reified->[0] = "( SELECT EXISTS $$reified->[0] )";
+
+   $reified;
+}
+
+
 sub results_exist {
-   my $self   = shift;
+   my $self = shift;
+
+   my $storage = $self->result_source->schema->storage;
+   my( $sql, @bind ) = @${ $self->results_exist_as_query };
+
+   # ugly as fuck but meh - DBIC has to do this too in places
+   $sql =~ s/\A \s* \( \s* (.+) \s* \) \s* \z/$1/sx;
+
+   my( undef, $sth ) = $storage->dbh_do( _dbh_execute =>
+      $sql,
+      \@bind,
+      $storage->_dbi_attrs_for_bind( undef, \@bind ),
+   );
 
-   $self
-      ->result_source
-      ->resultset
-      ->search({ -exists => $self->as_query })
-      ->first
+   my $rv = $sth->fetchall_arrayref;
+   $rv->[0][0];
 }
 
 1;
diff --git a/t/ResultSet/Shortcut/ResultsExist.t b/t/ResultSet/Shortcut/ResultsExist.t
index 53aacbf..0e42125 100644
--- a/t/ResultSet/Shortcut/ResultsExist.t
+++ b/t/ResultSet/Shortcut/ResultsExist.t
@@ -16,4 +16,32 @@ my $rs2 = $schema->resultset( 'Foo' )->search({ id => { '<' => 0 } });
 ok( $rs->results_exist, 'check rs has some results' );
 ok(!$rs2->results_exist, 'and check that rs has no results' );
 
+is_deeply(
+   [
+      $rs->search({}, { order_by => 'id', columns => {
+
+         id => "id",
+
+         has_lesser => $rs->search(
+            { 'correlation.id' => { '<' => { -ident => "me.id" } } },
+            { alias => 'correlation' }
+         )->results_exist_as_query,
+
+         has_greater => $rs->search(
+            { 'correlation.id' => { '>' => { -ident => "me.id" } } },
+            { alias => 'correlation' }
+         )->results_exist_as_query,
+
+      }})->hri->all
+   ],
+   [
+      { id => 1, has_lesser => 0, has_greater => 1 },
+      { id => 2, has_lesser => 1, has_greater => 1 },
+      { id => 3, has_lesser => 1, has_greater => 1 },
+      { id => 4, has_lesser => 1, has_greater => 1 },
+      { id => 5, has_lesser => 1, has_greater => 0 },
+   ],
+   "Correlated-existence works",
+);
+
 done_testing;

@frioux
Copy link
Owner

@frioux frioux commented on 8570231 Dec 2, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ribasushi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frioux so I thought some more - according to https://blog.afoolishmanifesto.com/posts/read-write-splitter/ you send _select where it belongs, so I guess results_exist can be rewritten as such:

sub results_exist {
   my $self = shift;

   my( undef, $sth ) = $self->result_source
                             ->schema
                              ->storage
                               ->_select(
                                  $self->results_exist_as_query,
                                  \'*',
                                  {},
                                  {}
                               );

   my $rv = $sth->fetchall_arrayref;
   $rv->[0][0];
}

The sql it produces is uglier but meh

@ribasushi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also not entirely sure whether it is worth it to be explicit in the return, instead of relying on the boolean that came out of the DBD. That is perhaps it should end with:

return(  $rv->[0][0] ? 1 : 0 )

@frioux
Copy link
Owner

@frioux frioux commented on 8570231 Dec 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your first suggestion generates sql like this:

SELECT EXISTS(          
   SELECT 1            
     FROM $some-table WHERE $foo
  )

We already have an internal version that generates this:

SELECT 1                
 FROM $some-table me          
WHERE EXISTS(          
   SELECT 1            
     FROM $some-table me  WHERE $foo    
  )

What does the new one generate? Is it (or any of these) actually distinct from the rest wrt performance?

@ribasushi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

SELECT EXISTS(          
   SELECT 1            
     FROM $some-table WHERE $foo
  )

^^ this will always return a single row ( low-level, on the cursor, or if you run it in a cmd client )

SELECT 1                
 FROM $some-table me          
WHERE EXISTS(          
   SELECT 1            
     FROM $some-table me  WHERE $foo    
  )

^^ for a table with 1 million matching records the above will return a set of 1 million single column 1 values ( this is why this issue got started in the 1st place )

the new way generates this ( essentially an extra select-wrap against the first variant ):

SELECT *
  FROM (
    SELECT EXISTS (
      SELECT 1
        FROM $some-table "me"
      WHERE $foo
    )
  )

which again is a single result, it is just uglier than the very top one in my reply ( but it works with your splitter transparrently )

@frioux
Copy link
Owner

@frioux frioux commented on 8570231 Dec 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks

Please sign in to comment.