disallow _version in the where clause of a

select statement

CHANGES.txt

@@ -5,8 +5,8 @@ Changes for Crate
 Unreleased
 ==========
 
- - fix: throw correct exception if one uses `_version` without primary key
-   in a select statements where clause
+ - fix: throw correct exception if one uses `_version in a select statements
+   where clause
 
  - Optimized memory estimates for min()/max() aggregates for the CircuitBreaker
 

docs/sql/occ.txt

@@ -62,6 +62,11 @@ update and results in 0 affected rows::
     ... where name = 'Algol' and "_version" = 2;
     UPDATE OK, 0 rows affected (... sec)
 
+.. warning::
+
+    Specifying only the ``_version`` in the ``WHERE`` clause of an UPDATE statement 
+    without any other criteria is very expensive and should be avoided.
+
 Optimistic Delete
 =================
 

sql/src/main/java/io/crate/analyze/SelectStatementAnalyzer.java

@@ -28,6 +28,7 @@
 import io.crate.analyze.validator.SelectSymbolValidator;
 import io.crate.analyze.validator.SortSymbolValidator;
 import io.crate.exceptions.SQLParseException;
+import io.crate.exceptions.UnsupportedFeatureException;
 import io.crate.metadata.*;
 import io.crate.planner.symbol.*;
 import io.crate.planner.symbol.Literal;
@@ -115,7 +116,9 @@ protected Symbol visitQuerySpecification(QuerySpecification node, SelectAnalyzed
 
 
         context.whereClause(generateWhereClause(node.getWhere(), context));
-
+        if(context.whereClause().version().isPresent()){
+            throw new UnsupportedFeatureException("\"_version\" column is not valid in the WHERE clause of a SELECT statement");
+        }
         if (!node.getGroupBy().isEmpty()) {
             context.selectFromFieldCache = true;
         }

sql/src/main/java/io/crate/lucene/LuceneQueryBuilder.java

@@ -708,8 +708,8 @@ public Query visitFunction(Function function, Context context) {
                 query = toQuery.apply(function, context);
             } catch (IOException e) {
                 throw ExceptionsHelper.convertToRuntime(e);
-            } catch (UnsupportedOperationException e) {
-                return genericFunctionQuery(function);
+            } catch (UnsupportedOperationException e){
+                query = genericFunctionQuery(function);
             }
             if (query == null) {
                 query = queryFromInnerFunction(function, context);

sql/src/test/java/io/crate/analyze/SelectAnalyzerTest.java

@@ -24,11 +24,11 @@
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
-import io.crate.metadata.*;
 import io.crate.exceptions.AmbiguousColumnAliasException;
 import io.crate.exceptions.ColumnUnknownException;
 import io.crate.exceptions.SQLParseException;
 import io.crate.exceptions.UnsupportedFeatureException;
+import io.crate.metadata.*;
 import io.crate.metadata.sys.MetaDataSysModule;
 import io.crate.metadata.sys.SysNodesTableInfo;
 import io.crate.metadata.table.SchemaInfo;
@@ -519,12 +519,10 @@ public void testCompositePrimaryKey() throws Exception {
     }
 
     @Test
-    public void testPrimaryKeyAndVersion() throws Exception {
-        SelectAnalyzedStatement analysis = analyze(
-            "select name from users where id = 2 and \"_version\" = 1");
-        assertEquals(ImmutableList.of("2"), analysis.ids());
-        assertEquals(ImmutableList.of("2"), analysis.routingValues());
-        assertThat(analysis.whereClause().version().get(), is(1L));
+    public void testVersion() throws Exception {
+        expectedException.expect(UnsupportedFeatureException.class);
+        expectedException.expectMessage("\"_version\" column is not valid in the WHERE clause of a SELECT statement");
+        analyze("select name from users where id = 2 and \"_version\" = 1");
     }
 
     @Test

sql/src/test/java/io/crate/integrationtests/VersionHandlingIntegrationTest.java

@@ -102,7 +102,7 @@ public void testDeleteWhereVersionWithConflict() throws Exception {
     }
 
     @Test
-    public void testUpdateWhereVersion() throws Exception {
+    public void testUpdateWhereVersionWithPrimaryKey() throws Exception {
         execute("create table test (col1 integer primary key, col2 string)");
         ensureGreen();
 
@@ -124,6 +124,40 @@ public void testUpdateWhereVersion() throws Exception {
         assertEquals("ok now panic", response.rows()[0][0]);
     }
 
+    @Test
+    public void testUpdateWhereVersionWithoutPrimaryKey() throws Exception {
+        execute("create table test (col1 integer primary key, col2 string)");
+        ensureGreen();
+
+        execute("insert into test (col1, col2) values (?, ?)", new Object[]{1, "don't panic"});
+        refresh();
+
+        execute("select \"_version\" from test where col1 = 1");
+        assertEquals(1L, response.rowCount());
+        assertEquals(1L, response.rows()[0][0]);
+
+        execute("update test set col2 = ? where \"_version\" = ?",
+                new Object[]{"ok now panic", 1});
+        assertEquals(1L, response.rowCount());
+
+        // Validate that the row is really updated
+        refresh();
+        execute("select col2 from test where col1 = 1");
+        assertEquals(1L, response.rowCount());
+        assertEquals("ok now panic", response.rows()[0][0]);
+
+        // Update with version conflict
+        execute("update test set col2 = ? where \"_version\" = ?",
+                new Object[]{"panic", 1});
+        assertEquals(0L, response.rowCount());
+
+        // Validate that the row is really NOT updated
+        refresh();
+        execute("select col2 from test where col1 = 1");
+        assertEquals(1L, response.rowCount());
+        assertEquals("ok now panic", response.rows()[0][0]);
+    }
+
     @Test
     public void testUpdateWhereVersionWithConflict() throws Exception {
         execute("create table test (col1 integer primary key, col2 string)");
@@ -157,18 +191,16 @@ public void testSelectWhereVersionWithoutPrimaryKey() throws Exception {
         execute("create table test (col1 integer primary key, col2 string)");
         ensureGreen();
         expectedException.expect(SQLActionException.class);
-        expectedException.expectMessage("\"_version\" column is only valid in the WHERE clause if the primary key column is also present");
+        expectedException.expectMessage("\"_version\" column is not valid in the WHERE clause of a SELECT statement");
         execute("select _version from test where col2 = 'hello' and _version = 1");
     }
 
     @Test
     public void testSelectWhereVersionWithPrimaryKey() throws Exception {
         execute("create table test (col1 integer primary key, col2 string)");
         ensureGreen();
-
-        execute("insert into test (col1, col2) values (?, ?)", new Object[]{1, "don't panic"});
-        refresh();
-        execute("select _version from test where col1 = 1 and _version = 1");
-        assertEquals(1L, response.rowCount());
+        expectedException.expect(SQLActionException.class);
+        expectedException.expectMessage("\"_version\" column is not valid in the WHERE clause of a SELECT statement");
+        execute("select _version from test where col1 = 1 and _version = 50");
     }
 }