[Bps-public-commit] dbix-searchbuilder branch master updated. 1.76-3-g9b5124e

BPS Git Server git at git.bestpractical.com
Fri Jun 30 20:35:34 UTC 2023


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "dbix-searchbuilder".

The branch, master has been updated
       via  9b5124e62f7a9bcc372a130214f9d22d0e35a666 (commit)
       via  5fa2766f1138bacc3c3c580c0de4dc8e443b5d0b (commit)
       via  498dfe64082b64c656de1ed979511f3d165ebbb1 (commit)
      from  990eeb8618598fea34b687a42f980faa683353c8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 9b5124e62f7a9bcc372a130214f9d22d0e35a666
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sat Jul 1 04:27:38 2023 +0800

    Support to specify Host/Port for Pg/mysql in tests

diff --git a/t/utils.pl b/t/utils.pl
index b662034..e21e3f9 100644
--- a/t/utils.pl
+++ b/t/utils.pl
@@ -110,6 +110,8 @@ sub connect_mysql
 	return $handle->Connect(
 		Driver => 'mysql',
 		Database => $ENV{'SB_TEST_MYSQL'},
+		Host => $ENV{'SB_TEST_MYSQL_HOST'},
+		Port => $ENV{'SB_TEST_MYSQL_PORT'},
 		User => $ENV{'SB_TEST_MYSQL_USER'} || 'root',
 		Password => $ENV{'SB_TEST_MYSQL_PASS'} || '',
 	);
@@ -121,6 +123,8 @@ sub connect_pg
 	return $handle->Connect(
 		Driver => 'Pg',
 		Database => $ENV{'SB_TEST_PG'},
+		Host => $ENV{'SB_TEST_PG_HOST'},
+		Port => $ENV{'SB_TEST_PG_PORT'},
 		User => $ENV{'SB_TEST_PG_USER'} || 'postgres',
 		Password => $ENV{'SB_TEST_PG_PASS'} || '',
 	);

commit 5fa2766f1138bacc3c3c580c0de4dc8e443b5d0b
Merge: 990eeb8 498dfe6
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat Jul 1 04:24:36 2023 +0800

    Merge branch 'distinct-and-count-and-sort'


commit 498dfe64082b64c656de1ed979511f3d165ebbb1
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed Jun 14 23:11:04 2023 +0300

    Change how DistinctQueryAndCount builds query to fix sorting
    
    If order by is on columns from the main table then old query generated by
    DistinctQueryAndCount was like this:
    
        SELECT main.*, COUNT(main.id) OVER () FROM (SELECT ... ORDER BY main...) main;
    
    It works until it doesn't. Order by clause should be on outer most query.
    
    Some evidence:
    
    * SQL standard: ORDER BY command cannot be used in a subquery. Google
      ANSI SQL order by in subquery.
    
    * mariadb explanation: https://mariadb.com/kb/en/why-is-order-by-in-a-from-subquery-ignored/
    
    * SQL server: https://learn.microsoft.com/en-us/sql/t-sql/queries/select-order-by-clause-transact-sql?view=sql-server-ver16
      "The ORDER BY clause is not valid in views, ... and *subqueries*, unless ..."
    
    Even in cases where ORDER BY is allowed in a subquery there is stated
    guaranty that order will preserved in the outer query.
    
    New query:
    
        SELECT main.*, COUNT(main.id) OVER () FROM (SELECT ...) main ORDER BY main...;
    
    Note that in case of empty ORDER BY the query is still wrapped to make
    sure the window function counts distinct records, eg:
    
        SELECT main.*, COUNT(main.id) OVER () FROM (SELECT DISTINCT ...) main;

diff --git a/lib/DBIx/SearchBuilder/Handle.pm b/lib/DBIx/SearchBuilder/Handle.pm
index eb8c7a7..747191c 100755
--- a/lib/DBIx/SearchBuilder/Handle.pm
+++ b/lib/DBIx/SearchBuilder/Handle.pm
@@ -1453,6 +1453,10 @@ sub DistinctQuery {
     my $self = shift;
     my $statementref = shift;
     my $sb = shift;
+    my %args = (
+        Wrap => 0,
+        @_
+    );
 
     my $QueryHint = $sb->QueryHint;
     $QueryHint = $QueryHint ? " /* $QueryHint */ " : " ";
@@ -1460,6 +1464,9 @@ sub DistinctQuery {
     # Prepend select query for DBs which allow DISTINCT on all column types.
     $$statementref = "SELECT" . $QueryHint . "DISTINCT main.* FROM $$statementref";
     $$statementref .= $sb->_GroupClause;
+    if ( $args{'Wrap'} ) {
+        $$statementref = "SELECT * FROM ($$statementref) main";
+    }
     $$statementref .= $sb->_OrderClause;
 }
 
@@ -1475,18 +1482,21 @@ sub DistinctQueryAndCount {
     my $statementref = shift;
     my $sb = shift;
 
-    $self->DistinctQuery($statementref, $sb);
+    # order by clause should be on outer most query
+    # SQL standard: ORDER BY command cannot be used in a Subquery
+    # mariadb explanation: https://mariadb.com/kb/en/why-is-order-by-in-a-from-subquery-ignored/
+    # pg: actually keeps order of a subquery as long as some conditions in outer query are met, but
+    #     it's just a coincidence, not a feature
+    # SQL server: https://learn.microsoft.com/en-us/sql/t-sql/queries/select-order-by-clause-transact-sql?view=sql-server-ver16
+    #             The ORDER BY clause is not valid in views, ... and *subqueries*, unless ...
 
-    # Add the count part.
-    if ( $sb->_OrderClause !~ /(?<!main)\./ ) {
-        # Wrap it with another SELECT to get distinct count.
-        $$statementref
-            = 'SELECT main.*, COUNT(main.id) OVER() AS search_builder_count_all FROM (' . $$statementref . ') main';
-    }
-    else {
-        # if order by other tables, then DistinctQuery already has an outer SELECT, which we can reuse
-        $$statementref =~ s!(?= FROM)!, COUNT(main.id) OVER() AS search_builder_count_all!;
-    }
+    my $order = $sb->_OrderClause;
+    my $wrap = $order !~ /(?<!main)\./;
+
+    $self->DistinctQuery($statementref, $sb, Wrap => $wrap);
+
+    # DistinctQuery already has an outer SELECT, which we can reuse
+    $$statementref =~ s!(?= FROM)!, COUNT(main.id) OVER() AS search_builder_count_all!;
 }
 
 
diff --git a/t/02order_outer.t b/t/02order.t
similarity index 57%
rename from t/02order_outer.t
rename to t/02order.t
index c91c7f3..969e1c7 100644
--- a/t/02order_outer.t
+++ b/t/02order.t
@@ -7,7 +7,7 @@ use Test::More;
 BEGIN { require "./t/utils.pl" }
 our (@AvailableDrivers);
 
-use constant TESTS_PER_DRIVER => 104;
+use constant TESTS_PER_DRIVER => 315;
 
 my $total = scalar(@AvailableDrivers) * TESTS_PER_DRIVER;
 plan tests => $total;
@@ -55,52 +55,103 @@ diag "generate data" if $ENV{TEST_VERBOSE};
     }
 }
 
-# ASC order
+my @fields = (
+    'Name',
+    $d eq 'Oracle' ? 'TO_CHAR(Name)' : $d eq 'mysql' ? 'BINARY(Name)' : 'CAST(Name AS TEXT)'
+);
+
+diag "test ordering objects by fields on Tags table" if $ENV{TEST_VERBOSE};
 foreach my $direction ( qw(ASC DESC) ) {
-    for my $field ( 'Name',
-        $d eq 'Oracle' ? 'TO_CHAR(Name)' : $d eq 'mysql' ? 'BINARY(Name)' : 'CAST(Name AS TEXT)' )
-    {
-        my $objs = TestApp::Objects->new($handle);
-        $objs->UnLimit;
-        my $tags_alias = $objs->Join(
-            TYPE   => 'LEFT',
-            ALIAS1 => 'main',
-            FIELD1 => 'id',
-            TABLE2 => 'Tags',
-            FIELD2 => 'Object',
-        );
-        ok($tags_alias, "joined tags table");
-        # Generated SQL is MIN(Name) or nested functions like MIN(CAST(Name AS TEXT))
-        $objs->OrderBy( ALIAS => $tags_alias, FIELD => $field, ORDER => $direction );
-
-        ok($objs->First, 'ok, we have at least one result');
+foreach my $field (@fields ) {
+foreach my $combine_search_and_count ( 0, 1 ) {
+foreach my $per_page (0, 5) {
+foreach my $page (0, 2) {
+    my $objs = TestApp::Objects->new($handle);
+    $objs->CombineSearchAndCount($combine_search_and_count);
+    $objs->UnLimit;
+    my $tags_alias = $objs->Join(
+        TYPE   => 'LEFT',
+        ALIAS1 => 'main',
+        FIELD1 => 'id',
+        TABLE2 => 'Tags',
+        FIELD2 => 'Object',
+    );
+    ok($tags_alias, "joined tags table");
+    # Generated SQL is MIN(Name) or nested functions like MIN(CAST(Name AS TEXT))
+    $objs->OrderBy( ALIAS => $tags_alias, FIELD => $field, ORDER => $direction );
+    $objs->RowsPerPage($per_page) if $per_page;
+    $objs->GotoPage($page) if $page;
+
+    ok($objs->First, 'ok, we have at least one result');
+    $objs->GotoFirstItem;
+
+    my ($order_ok, $last) = (1, $direction eq 'ASC'? '-': 'zzzz');
+    while ( my $obj = $objs->Next ) {
+        my $tmp;
+        if ( $direction eq 'ASC' ) {
+            $tmp = (substr($last, 0, 1) cmp substr($obj->Name, 0, 1));
+        } else {
+            $tmp = -(substr($last, -1, 1) cmp substr($obj->Name, -1, 1));
+        }
+        if ( $tmp > 0 ) {
+            $order_ok = 0; last;
+        }
+        $last = $obj->Name;
+    }
+    ok($order_ok, "$direction order is correct") or do {
+        diag "Wrong $direction query: ". $objs->BuildSelectQuery;
         $objs->GotoFirstItem;
-
-        my ($order_ok, $last) = (1, $direction eq 'ASC'? '-': 'zzzz');
         while ( my $obj = $objs->Next ) {
-            my $tmp;
-            if ( $direction eq 'ASC' ) {
-                $tmp = (substr($last, 0, 1) cmp substr($obj->Name, 0, 1));
-            } else {
-                $tmp = -(substr($last, -1, 1) cmp substr($obj->Name, -1, 1));
-            }
-            if ( $tmp > 0 ) {
-                $order_ok = 0; last;
-            }
-            $last = $obj->Name;
-        }
-        ok($order_ok, "$direction order is correct") or do {
-            diag "Wrong $direction query: ". $objs->BuildSelectQuery;
-            $objs->GotoFirstItem;
-            while ( my $obj = $objs->Next ) {
-                diag($obj->id .":". $obj->Name);
-            }
+            diag($obj->id .":". $obj->Name);
         }
+    };
+
+    my $got_count = $objs->CountAll;
+    is ($got_count, 30, "CountAll is expected");
+
+}}}}} # foreach variants blocks
+
+my $expected_count = 0;
+
+diag "test ordering objects by object's fields with limit by tags" if $ENV{TEST_VERBOSE};
+foreach my $direction ( qw(ASC DESC) ) {
+foreach my $field ('Name', 'id') {
+foreach my $combine_search_and_count ( 0, 1 ) {
+foreach my $per_page (0, 5) {
+foreach my $page (0, 2) {
+    my $objs = TestApp::Objects->new($handle);
+    $objs->CombineSearchAndCount($combine_search_and_count);
+    my $tags_alias = $objs->Join(
+        ALIAS1 => 'main',
+        FIELD1 => 'id',
+        TABLE2 => 'Tags',
+        FIELD2 => 'Object',
+    );
+    ok($tags_alias, "joined tags table");
+    $objs->OrderBy( FIELD => $field, ORDER => $direction );
+    $objs->RowsPerPage($per_page) if $per_page;
+    $objs->GotoPage($page) if $page;
+
+    $objs->Limit( ALIAS => $tags_alias, FIELD => 'Name', OPERATOR => 'IN', VALUE => ['c', 'a'] );
+
+    my @list;
+    while ( my $obj = $objs->Next ) {
+        push @list, $field eq 'Name'? $obj->Name : $obj->Id;
     }
-}
+    my @correct = sort {$field eq 'Name'? $a cmp $b : $a <=> $b } @list;
+    @correct = reverse @correct if $direction eq 'DESC';
+    is_deeply(\@list, \@correct, "correct order");
+
+    my $got_count = $objs->CountAll;
+    if ($expected_count) {
+        is($got_count, $expected_count, "count is correct");
+    } else {
+        $expected_count = $got_count;
+    }
+}}}}} # foreach variants blocks
 
 	cleanup_schema( 'TestApp', $handle );
-}} # SKIP, foreach blocks
+}} # SKIP, foreach driver block
 
 1;
 
@@ -118,6 +169,7 @@ sub schema_mysql { [
         Name varchar(36),
       	PRIMARY KEY (id)
     )",
+    "CREATE INDEX Tags1 ON Tags (Name)"
 ] }
 
 sub schema_pg { [
@@ -130,6 +182,7 @@ sub schema_pg { [
         Object integer NOT NULL,
         Name varchar(36)
     )",
+    "CREATE INDEX Tags1 ON Tags (Name)"
 ]}
 
 sub schema_sqlite {[
@@ -142,6 +195,7 @@ sub schema_sqlite {[
         Object integer NOT NULL,
         Name varchar(36)
     )",
+    "CREATE INDEX Tags1 ON Tags (Name)"
 ]}
 
 sub schema_oracle { [
@@ -156,6 +210,7 @@ sub schema_oracle { [
         Object integer NOT NULL,
         Name varchar(36)
     )",
+    "CREATE INDEX Tags1 ON Tags (Name)"
 ] }
 
 sub cleanup_schema_oracle { [

-----------------------------------------------------------------------

Summary of changes:
 lib/DBIx/SearchBuilder/Handle.pm |  32 ++++++----
 t/{02order_outer.t => 02order.t} | 135 +++++++++++++++++++++++++++------------
 t/utils.pl                       |   4 ++
 3 files changed, 120 insertions(+), 51 deletions(-)
 rename t/{02order_outer.t => 02order.t} (57%)


hooks/post-receive
-- 
dbix-searchbuilder


More information about the Bps-public-commit mailing list