Deprecate -nest with strong prejudice
Peter Rabbitson [Thu, 23 Dec 2010 19:39:24 +0000 (20:39 +0100)]
Changes
lib/DBIx/Class/Manual/Cookbook.pod
lib/DBIx/Class/Manual/FAQ.pod
lib/DBIx/Class/SQLMaker.pm
t/sqlmaker/nest_deprec.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index 584ceb8..44430a5 100644 (file)
--- a/Changes
+++ b/Changes
@@ -13,6 +13,8 @@ Revision history for DBIx::Class
         - +columns now behaves just like columns by not stripping a
           fully-qualified 'as' spec (i.e. foo.bar results in $obj->foo->bar)
         - Add full INSERT...RETURNING support for Oracle
+        - Deprecate use of -nest in search conditions (warn once per
+          callsite)
 
     * Fixes
         - Fixed read-only attribute set attempt in ::Storage::Replicated
index 6bbd343..7fe2058 100644 (file)
@@ -413,40 +413,29 @@ Using SQL functions on the left hand side of a comparison is generally not a
 good idea since it requires a scan of the entire table. (Unless your RDBMS
 supports indexes on expressions - including return values of functions - and
 you create an index on the return value of the function in question.) However,
-it can be accomplished with C<DBIx::Class> when necessary.
-
-Your approach for doing so will depend on whether you have turned
-quoting on via the C<quote_char> and C<name_sep> attributes. If you
-explicitly defined C<quote_char> and C<name_sep> in your
-C<connect_info> (see L<DBIx::Class::Storage::DBI/"connect_info">) then
-you are using quoting, otherwise not.
-
-If you do not have quoting on, simply include the function in your search
-specification as you would any column:
-
-  $rs->search({ 'YEAR(date_of_birth)' => 1979 });
-
-With quoting on, or for a more portable solution, use literal SQL values with
-placeholders:
+it can be accomplished with C<DBIx::Class> when necessary by resorting to
+literal SQL:
 
   $rs->search(\[ 'YEAR(date_of_birth) = ?', [ plain_value => 1979 ] ]);
 
   # Equivalent SQL:
   # SELECT * FROM employee WHERE YEAR(date_of_birth) = ?
 
-  $rs->search({
+  $rs->search({ -and => [
     name => 'Bob',
-    -nest => \[ 'YEAR(date_of_birth) = ?', [ plain_value => 1979 ] ],
-  });
+    \[ 'YEAR(date_of_birth) = ?', [ plain_value => 1979 ] ],
+  ]});
 
   # Equivalent SQL:
   # SELECT * FROM employee WHERE name = ? AND YEAR(date_of_birth) = ?
 
 Note: the C<plain_value> string in the C<< [ plain_value => 1979 ] >> part
 should be either the same as the name of the column (do this if the type of the
-return value of the function is the same as the type of the column) or
-otherwise it's essentially a dummy string currently (use C<plain_value> as a
-habit). It is used by L<DBIx::Class> to handle special column types.
+return value of the function is the same as the type of the column) or in the
+case of a function it's currently treated as a dummy string (it is a good idea
+to use C<plain_value> or something similar to convey intent). The value is
+currently only significant when handling special column types (BLOBs, arrays,
+etc.), but this may change in the future.
 
 See also L<SQL::Abstract/Literal SQL with placeholders and bind values
 (subqueries)>.
index 102c4e9..fa25e22 100644 (file)
@@ -238,19 +238,18 @@ documentation for details.
 
 =item .. search with an SQL function on the left hand side?
 
-To use an SQL function on the left hand side of a comparison:
+To use an SQL function on the left hand side of a comparison you currently need
+to resort to literal SQL:
 
- ->search({ -nest => \[ 'YEAR(date_of_birth) = ?', [ plain_value => 1979 ] ] });
+ ->search( \[ 'YEAR(date_of_birth) = ?', [ plain_value => 1979 ] ] );
 
 Note: the C<plain_value> string in the C<< [ plain_value => 1979 ] >> part
 should be either the same as the name of the column (do this if the type of the
-return value of the function is the same as the type of the column) or
-otherwise it's essentially a dummy string currently (use C<plain_value> as a
-habit). It is used by L<DBIx::Class> to handle special column types.
-
-Or, if you have quoting off:
-
- ->search({ 'YEAR(date_of_birth)' => 1979 });
+return value of the function is the same as the type of the column) or in the
+case of a function it's currently treated as a dummy string (it is a good idea
+to use C<plain_value> or something similar to convey intent). The value is
+currently only significant when handling special column types (BLOBs, arrays,
+etc.), but this may change in the future.
 
 =item .. find more help on constructing searches?
 
@@ -382,7 +381,7 @@ the rows at once.
 =item .. update a column using data from another column?
 
 To stop the column name from being quoted, you'll need to tell DBIC
-that the right hand side is an SQL identity (it will be quoted
+that the right hand side is an SQL identifier (it will be quoted
 properly if you have quoting enabled):
 
  ->update({ somecolumn => { -ident => 'othercolumn' } })
index 67f0a0a..1dd46f5 100644 (file)
@@ -140,6 +140,23 @@ sub _where_op_VALUE {
   ;
 }
 
+my $callsites_warned;
+sub _where_op_NEST {
+  # determine callsite obeying Carp::Clan rules (fucking ugly but don't have better ideas)
+  my $callsite = do {
+    my $w;
+    local $SIG{__WARN__} = sub { $w = shift };
+    carp;
+    $w
+  };
+
+  carp ("-nest in search conditions is deprecated, you most probably wanted:\n"
+      .q|{..., -and => [ \%cond0, \@cond1, \'cond2', \[ 'cond3', [ col => bind ] ], etc. ], ... }|
+  ) unless $callsites_warned->{$callsite}++;
+
+  shift->next::method(@_);
+}
+
 # Handle limit-dialect selection
 sub select {
   my ($self, $table, $fields, $where, $rs_attrs, $limit, $offset) = @_;
diff --git a/t/sqlmaker/nest_deprec.t b/t/sqlmaker/nest_deprec.t
new file mode 100644 (file)
index 0000000..98f1157
--- /dev/null
@@ -0,0 +1,32 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Warn;
+
+use lib qw(t/lib);
+use DBIC::SqlMakerTest;
+
+use_ok('DBICTest');
+
+my $schema = DBICTest->init_schema();
+
+my $sql_maker = $schema->storage->sql_maker;
+
+# a loop so that the callsite line does not change
+for my $expect_warn (1, 0) {
+  warnings_like (
+    sub {
+      my ($sql, @bind) = $sql_maker->select ('foo', undef, { -nest => \ 'bar' } );
+      is_same_sql_bind (
+        $sql, \@bind,
+        'SELECT * FROM foo WHERE ( bar )', [],
+        '-nest still works'
+      );
+    },
+    ($expect_warn ? qr/\Q-nest in search conditions is deprecated/ : []),
+    'Only one deprecation warning'
+  );
+}
+
+done_testing;