Item9808: all tests pass on SQL server; that makes a full set.

git-svn-id: http://svn.foswiki.org/trunk/DBIStoreContrib@17296 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

data/System/DBIStoreTest.txt

@@ -20,7 +20,7 @@ To reset the cache, click on %SCRIPTURL{"view"}%/%WEB%/%WOPIC%?dbistore_init=1 (
    * Table.selector: %TA% 2 %TB% "form.name='DBIStoreTestForm' AND length('x')=1" %TC%
    * Array: %TA% 1 %TB% "fields[name='string'].value='String'" %TC%
    * Field: %TA% 2 %TB% "number" %TC%
-   * Boolean field: %TA% 0 %TB% "boolean" %TC%
+   * Boolean field: %TA% 1 %TB% "boolean" %TC%
    * Field cmp: %TA% 1 %TB% "number=99" %TC%
    * Simple regex: %TA% 2 %TB% "'AA'=~'A'" %TC%
    * Simple LIKE: %TA% 1 %TB% "name~'DBIStoreT*orm'" %TC%
@@ -44,11 +44,12 @@ To reset the cache, click on %SCRIPTURL{"view"}%/%WEB%/%WOPIC%?dbistore_init=1 (
    * Table Ref      %TA% 2 %TB% "'DBIStoreTest'/META:FORM.name='DBIStoreTestForm'" %TC%
    * Complex ref     %TA% 2 %TB% "(fields[name='Other'].value)/META:FORM.name='DBIStoreTestForm'" %TC%
    * Escapes  %TA% 0 %TB% "name =~ '\\\' OR name ~ '\\\' OR name = '\\\'" %TC%
-   * Table=Table     %TA% 0 %TB% "fields=attachments" %TC%
+   * Escapes  %TA% 0 %TB% "name =~ '\\.x.y\\\'" %TC%
+   * Table=Table     %#TA% 0 %#TB% "fields=attachments" %#TC%
 
 %META:FORM{name="DBIStoreTestForm"}%
 %META:FIELD{name="number" attributes="" title="number" value="99"}%
 %META:FIELD{name="string" attributes="" title="string" value="String"}%
-%META:FIELD{name="boolean" attributes="" title="boolean" value=""}%
+%META:FIELD{name="boolean" attributes="" title="boolean" value="Boolean"}%
 %META:FIELD{name="Other" attributes="" title="Other" value="DBIStoreTestForm"}%
 %META:PREFERENCE{name="Red" value="0"}%

data/System/DBIStoreTestForm.txt

@@ -1,8 +1,7 @@
-%META:TOPICINFO{author="SimianApe" comment="pending" date="1392048186" format="1.1" version="2"}%
 | *Name* | *Type* | *Size* | *Values* | *Tooltip message* | *Attributes* |
 | number |  text | 10 | | A Number | |
 | string | text | 32 | | A String | |
-| boolean | checkbox | | | A Checkbox |
+| boolean | radio | 1 | Boolean | A Checkbox |
 | Other | text | 32 | | Another topic |
 
 This topic defines a form used for testing the DBIStoreContrib.

lib/Foswiki/Contrib/DBIStoreContrib.pm

@@ -7,8 +7,36 @@ use Foswiki ();
 our $VERSION = '1.1';           # version of *this file*.
 our $RELEASE = '12 Dec 2013';
 
-use constant MONITOR => 0;
+use constant MONITOR => 1;
 
 our $SHORTDESCRIPTION =
 'Use DBI to implement searching using an SQL database. Supports SQL queries over Form data.';
 
+# Type identifiers.
+# FIRST 3 MUST BE KEPT IN LOCKSTEP WITH Foswiki::Infix::Node
+# Declared again here because the constants are not defined
+# in Foswiki 1.1 and earlier
+use constant {
+    NAME   => 1,
+    NUMBER => 2,
+    STRING => 3,
+};
+
+# Additional types local to this module - must not overlap known
+# types
+use constant {
+    UNKNOWN => 0,
+
+    # Gap for 1.2 types HASH and META
+
+    BOOLEAN  => 10,
+    SELECTOR => 11,    # Temporary, synonymous with UNKNOWN
+
+    VALUE => 12,
+    TABLE => 13,
+
+    # An integer type used during hoisting where DB doesn't support
+    # a BOOLEAN type
+    PSEUDO_BOOL => 14,
+};
+

lib/Foswiki/Contrib/DBIStoreContrib/HoistSQL.pm

@@ -26,40 +26,26 @@ our $parser;
 
 use constant MONITOR => Foswiki::Contrib::DBIStoreContrib::MONITOR;
 
-# Type identifiers.
-# FIRST 3 MUST BE KEPT IN LOCKSTEP WITH Foswiki::Infix::Node
-# Declared again here because the constants are not defined
-# in Foswiki 1.1 and earlier
-use constant {
-    NAME   => 1,
-    NUMBER => 2,
-    STRING => 3,
-};
-
-# Additional types local to this module - must not overlap known
-# types
-use constant {
-    UNKNOWN => 0,
-
-    # Gap for 1.2 types HASH and META
-
-    BOOLEAN  => 10,
-    SELECTOR => 11,    # Temporary, synonymous with UNKNOWN
-
-    VALUE => 12,
-    TABLE => 13,
-
-    # An integer type used where DB doesn't support a BOOLEAN type
-    PSEUDO_BOOL => 14,
-};
-
 our $table_name_RE = qr/^\w+$/;
 
 # Pseudo-constants, from the Personality
-our $CQ;               # character string quote
+our $CQ;    # character string quote
 our $TRUE;
 our $TRUE_TYPE;
 
+# Copy constants for shorthand
+use constant {
+    NAME        => Foswiki::Contrib::DBIStoreContrib::NAME,
+    NUMBER      => Foswiki::Contrib::DBIStoreContrib::NUMBER,
+    STRING      => Foswiki::Contrib::DBIStoreContrib::STRING,
+    UNKNOWN     => Foswiki::Contrib::DBIStoreContrib::UNKNOWN,
+    BOOLEAN     => Foswiki::Contrib::DBIStoreContrib::BOOLEAN,
+    SELECTOR    => Foswiki::Contrib::DBIStoreContrib::SELECTOR,
+    VALUE       => Foswiki::Contrib::DBIStoreContrib::VALUE,
+    TABLE       => Foswiki::Contrib::DBIStoreContrib::TABLE,
+    PSEUDO_BOOL => Foswiki::Contrib::DBIStoreContrib::PSEUDO_BOOL,
+};
+
 BEGIN {
 
     # Foswiki 1.1 doesn't have makeConstant; monkey-patch it
@@ -328,17 +314,36 @@ sub hoist {
     $query = _rewrite( $query, UNKNOWN );
     print STDERR "Rewritten " . recreate($query) . "\n" if MONITOR;
 
-    # Top level selectors are relative to the 'topic' table
     my %h = _hoist( $query, 'topic' );
     my $alias = _alias(__LINE__);    # SQL server requires this!
-    if ( $h{sql} =~ /^SELECT/ ) {
+    if ( $h{is_table_name} ) {
         $h{sql} = "topic.tid IN (SELECT tid FROM ($h{sql}) AS $alias)";
     }
-    elsif ( $h{is_table_name} ) {
-        $h{sql} = "topic.tid in (SELECT tid FROM ($h{sql}) AS alias)";
+    elsif ( $h{sql} =~ /^SELECT/ ) {
+
+        # It's a table; test if the selector is a true value
+        my $where = '';
+        if ( $h{sel} ) {
+            if ( $h{type} == NUMBER || $h{type} == PSEUDO_BOOL ) {
+                $where = '!=0';
+            }
+            elsif ( $h{type} == STRING ) {
+                $where = "!=$CQ$CQ";
+            }
+            else {
+                $where = '';    # BOOLEAN
+            }
+            $where = " WHERE $h{sel}$where";
+        }
+        my $a2 = _alias(__LINE__);    # SQL server requires this!
+             # This rather clumsy construction is required because SQL server
+             # can't use an aliased column in the WHERE condition of the same
+             # SELECT.
+        $h{sql} =
+"topic.tid IN (SELECT tid FROM (SELECT * FROM ($h{sql}) AS $a2 $where) AS $alias)";
     }
-    elsif ( $h{type} == NUMBER ) {
-        $h{sql} = "($h{sql})!= 0";
+    elsif ( $h{type} == NUMBER || $h{type} == PSEUDO_BOOL ) {
+        $h{sql} = "($h{sql})!=0";
     }
     elsif ( $h{type} == STRING ) {
         $h{sql} = "($h{sql})!=$CQ$CQ";
@@ -459,16 +464,19 @@ sub _hoist {
         elsif ( $where{type} == NUMBER ) {
             $where{sql} = "($where{sql})!=0";
         }
-        elsif ( $where{type} == BOOLEAN ) {
+        elsif ( $where{type} == PSEUDO_BOOL ) {
             $where{sql} = "($where{sql})!=0";
         }
 
+        my $where = "$where{sql}$tid_constraint";
+
         $result{sql} = _SELECT(
             select  => '*',
             FROM    => $lhs{sql},
-            WHERE   => "$where{sql}$tid_constraint",
+            WHERE   => $where,
             monitor => __LINE__
         );
+        $result{has_where}  = length($where);
         $result{type}       = STRING;
         $result{ignore_tid} = $lhs{ignore_tid};
 
@@ -526,13 +534,14 @@ sub _hoist {
         my %lhs = _hoist( $node->{params}[0], undef );
         my $lhs_where;
         my $select = '';
+        my $wtn    = _personality()
+          ->strcat( "$topic_alias.web", "$CQ.$CQ", "$topic_alias.name" );
         if ( $lhs{sql} =~ /^SELECT/ ) {
             my $tnames = _alias(__LINE__);
             $select = _AS( $lhs{sql} => $tnames ) . ',';
             my $tname_sel = $tnames;
             $tname_sel = "$tnames.$lhs{sel}" if $lhs{sel};
-            $lhs_where = "($topic_alias.name=$tname_sel OR "
-              . "($topic_alias.web||$CQ.$CQ||$topic_alias.name)=$tname_sel)";
+            $lhs_where = "($topic_alias.name=$tname_sel OR ($wtn)=$tname_sel)";
         }
         elsif ( $lhs{is_table_name} ) {
 
@@ -543,8 +552,7 @@ sub _hoist {
 
             # Not a selector or simple table name, must be a simple
             # expression yielding a selector
-            $lhs_where = "($lhs{sql}) IN "
-              . "($topic_alias.name,$topic_alias.web||$CQ.$CQ||$topic_alias.name)";
+            $lhs_where = "($lhs{sql}) IN ($topic_alias.name,$wtn)";
         }
 
         # Expand the RHS *without* a constraint on the topic table
@@ -576,6 +584,7 @@ sub _hoist {
             monitor => __LINE__
         );
 
+        $result{has_where}  = 1;
         $result{type}       = $rhs{type};
         $result{ignore_tid} = 1;
     }
@@ -703,7 +712,8 @@ sub _hoist {
                     WHERE   => $where,
                     monitor => __LINE__
                 );
-                $result{type} = $optype;
+                $result{has_where} = length($where);
+                $result{type}      = $optype;
             }
         }
         elsif ( $lhs_is_SELECT || $lhs{is_table_name} ) {
@@ -743,7 +753,8 @@ sub _hoist {
                 WHERE   => $where,
                 monitor => __LINE__ . " $op"
             );
-            $result{type} = $optype;
+            $result{has_where} = length($where);
+            $result{type}      = $optype;
 
         }
         elsif ($rhs_is_SELECT) {
@@ -761,7 +772,7 @@ sub _hoist {
                 $r_sel    => $rhs{type}
             );
 
-            my $where = '_IGNORE_';
+            my $where;
             if ( $optype == BOOLEAN ) {
                 $where  = $expr;
                 $expr   = $TRUE;
@@ -783,7 +794,8 @@ sub _hoist {
                 WHERE   => $where,
                 monitor => __LINE__
             );
-            $result{type} = $optype;
+            $result{has_where} = length($where);
+            $result{type}      = $optype;
         }
         else {
 
@@ -805,7 +817,7 @@ sub _hoist {
             $result{ignore_tid} = 0;
             my ( $expr, $optype ) = &$opfn( $arg_sel => UNKNOWN );
 
-            my $where = '_IGNORE_';
+            my $where;
             if ( $optype == BOOLEAN ) {
                 $where  = $expr;
                 $expr   = $TRUE;
@@ -822,10 +834,11 @@ sub _hoist {
             $result{sql} = _SELECT(
                 select => _AS( $expr => $result{sel} ) . ",$ret_tid",
                 FROM    => $tid_table . _AS( $kid{sql} => $arg_alias ),
-                WHERE   => $expr,
+                WHERE   => $where,
                 monitor => __LINE__
             );
-            $result{type} = $optype;
+            $result{has_where} = length($expr);
+            $result{type}      = $optype;
         }
         else {
             ( $result{sql}, $result{type} ) = &$opfn( $kid{sql}, $kid{type} );

lib/Foswiki/Contrib/DBIStoreContrib/Personality.pm

@@ -5,6 +5,8 @@ use strict;
 use warnings;
 use Assert;
 
+use Foswiki::Contrib::DBIStoreContrib ();
+
 # We try to use the ANSI SQL standard as far as possible, for the most
 # part different SQL DB implementations support it fairly well. However
 # they all have nuances, and there are areas where the support is not
@@ -40,7 +42,7 @@ sub new {
             string_quote    => "'",
             text_type       => 'TEXT',
             true_value      => '1=1',
-            true_type => Foswiki::Contrib::DBIStoreContrib::HoistSQL::BOOLEAN,
+            true_type       => Foswiki::Contrib::DBIStoreContrib::BOOLEAN,
         },
         $class
     );
@@ -199,15 +201,13 @@ sub length {
 ---++ safe_id($id) -> $safeid
 Make sure the ID is safe to use in this dialect of SQL.
 Unsafe IDs should be quoted using the dialect's identifier
-quoting rule. The default is to double-quote it.
+quoting rule. The default is to double-quote all identifiers.
 
 =cut
 
 sub safe_id {
     my ( $this, $id ) = @_;
-    if ( $this->{reserved}->{$id} ) {
-        $id = "\"$id\"";
-    }
+    $id = "\"$id\"";
     return $id;
 }
 
@@ -247,6 +247,18 @@ sub make_comment {
     return '/*' . join( ' ', @_ ) . '*/';
 }
 
+=begin TML
+
+---++ strcat($str1 [$str2 [, ... strN]) -> $concatenated
+Use the SQL string concatenation operator to concatente strings.
+
+=cut
+
+sub strcat {
+    my $this = shift;
+    return join( '||', @_ );
+}
+
 1;
 __DATA__
 

lib/Foswiki/Contrib/DBIStoreContrib/Personality/ODBC.pm

@@ -31,10 +31,9 @@ sub new {
           TRY_CONVERT TSEQUAL UNPIVOT UPDATETEXT USE USER VARYING VIEW WAITFOR
           WHILE WITHIN GROUP WRITETEXT/
     );
-    $this->{text_type}  = 'VARCHAR(MAX)';
-    $this->{true_value} = 'CAST(1 AS BIT)';
-    $this->{true_type} =
-      Foswiki::Contrib::DBIStoreContrib::HoistSQL::PSEUDO_BOOL;
+    $this->{text_type}       = 'VARCHAR(MAX)';
+    $this->{true_value}      = 'CAST(1 AS BIT)';
+    $this->{true_type}       = Foswiki::Contrib::DBIStoreContrib::PSEUDO_BOOL;
     $this->{requires_COMMIT} = 0;
 
     return $this;
@@ -50,13 +49,13 @@ sub startup {
     my $exists = $this->{store}->{handle}->do(<<'SQL');
 SELECT 1 WHERE OBJECT_ID('dbo.foswiki_CONVERT') IS NOT NULL
 SQL
-    unless ($exists) {
+    if ( $exists == 0 ) {
 
         # make_number derived from is_numeric by Dmitri Golovan of Micralyne.
         $this->{store}->{handle}->do(<<'SQL');
 CREATE FUNCTION foswiki_CONVERT( @value VARCHAR(MAX) ) RETURNS FLOAT AS
 BEGIN
-  (
+  RETURN (
     CASE
       WHEN @value NOT LIKE '%[^-0-9.+]%'
            AND (
@@ -102,6 +101,11 @@ sub length {
     return "LEN($_[1])";
 }
 
+sub strcat {
+    my $this = shift;
+    return join( '+', @_ );
+}
+
 1;
 __DATA__
 

lib/Foswiki/Contrib/DBIStoreContrib/Personality/Pg.pm

@@ -108,7 +108,7 @@ sub regexp {
     # single quote.
     $rhs =~ s/'/\\'/g;
     $rhs =~ s/\\/\\/g;
-    return "$lhs ~$i E'$rhs'";
+    return "$lhs ~$i '$rhs'";
 }
 
 sub length {