fixup! fixup! fixup! fixup! handle query builder exceptions and close…

… search context when an exception is raised

CHANGES.txt

@@ -26,6 +26,9 @@ Unreleased
 
  - Optimize OR queries which involve the cluster column.
 
+ - Removed support for ``_version`` column in where clause if no
+   primary key is specified in a query.
+
  - Added support for the ``IF EXISTS`` clause to ``DROP TABLE`` and
    ``DROP BLOB TABLE``
 

docs/sql/occ.txt

@@ -52,22 +52,16 @@ Querying for the correct ``_version`` ensures that no concurrent update has
 taken place::
 
     cr> update locations set description = 'Updated description'
-    ... where name = 'Algol' and "_version" = 3;
+    ... where id=5 and "_version" = 3;
     UPDATE OK, 1 row affected (... sec)
 
 Updating a row with a wrong or outdated version number will not execute the
 update and results in 0 affected rows::
 
     cr> update locations set description = 'Updated description'
-    ... where name = 'Algol' and "_version" = 2;
+    ... where id=5 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
 =================
 
@@ -78,17 +72,16 @@ Of course the same can be done when deleting a row::
 
 
 Known Limitations
------------------
+=================
 
- - On deletes, this can only be done when using a primary key query (all
-   primary keys have to be included inside the ``WHERE`` clause). For
-   example, the query below is not possible with our used testing data
-   because ``name`` is not declared as a primary key and results in an
-   error::
+ - The ``_version`` column can only be used when specifying the whole
+   primary key in a query. For example, the query below is not
+   possible with our used testing data because ``name`` is not
+   declared as a primary key and results in an error::
 
     cr> delete from locations where name = 'Aldebaran' and "_version" = 3;
-    SQLActionException["_version" column is only valid in the WHERE clau...]
-
+    SQLActionException[_version is not allowed in delete queries
+                       without specifying a primary key]
 
 .. note::
 

sql/src/main/java/io/crate/analyze/where/EqualityExtractor.java

@@ -24,16 +24,15 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
 import io.crate.analyze.EvaluatingNormalizer;
+import io.crate.metadata.ColumnIdent;
 import io.crate.metadata.FunctionIdent;
 import io.crate.metadata.FunctionInfo;
-import io.crate.metadata.ColumnIdent;
 import io.crate.operation.operator.EqOperator;
 import io.crate.planner.symbol.*;
 import io.crate.types.DataType;
 import io.crate.types.DataTypes;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import sun.reflect.generics.reflectiveObjects.NotImplementedException;
 
 import java.io.IOException;
 import java.util.*;
@@ -60,20 +59,16 @@ public List<List<Symbol>> extractExactMatches(List<ColumnIdent> columns, Symbol
     }
 
     private List<List<Symbol>> extractMatches(Collection<ColumnIdent> columns, Symbol symbol, boolean exact){
-        EqualityExtractor.ProxyInjectingVisitor piv = new EqualityExtractor.ProxyInjectingVisitor();
         EqualityExtractor.ProxyInjectingVisitor.Context context =
                 new EqualityExtractor.ProxyInjectingVisitor.Context(columns, exact);
-        Symbol proxiedTree = piv.process(symbol, context);
+        Symbol proxiedTree = ProxyInjectingVisitor.INSTANCE.process(symbol, context);
 
         // bail out if we have any unknown part in the tree
         if (context.exact && context.seenUnknown){
             return null;
         }
 
-        //proxiedTree = normalizer.normalize(proxiedTree);
         List<Set<EqProxy>> comparisons = context.comparisonSet();
-
-
         Set<List<EqProxy>> cp = Sets.cartesianProduct(comparisons);
 
         List<List<Symbol>> result = new ArrayList<>();
@@ -146,12 +141,12 @@ public DataType valueType() {
 
         @Override
         public void readFrom(StreamInput in) throws IOException {
-            throw new NotImplementedException();
+            throw new UnsupportedOperationException();
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            throw new NotImplementedException();
+            throw new UnsupportedOperationException();
         }
 
         public String forDisplay() {
@@ -174,6 +169,11 @@ public String toString() {
 
     static class ProxyInjectingVisitor extends SymbolVisitor<ProxyInjectingVisitor.Context, Symbol> {
 
+        public static final ProxyInjectingVisitor INSTANCE = new ProxyInjectingVisitor();
+
+        private ProxyInjectingVisitor() {
+        }
+
         static class Comparison{
             final HashMap<Function, EqProxy> proxies = new HashMap<>();
 

sql/src/main/java/io/crate/analyze/where/HasColumn.java

@@ -22,10 +22,10 @@
 package io.crate.analyze.where;
 
 import io.crate.metadata.ColumnIdent;
-import io.crate.planner.symbol.DefaultTraversalSymbolVisitor;
 import io.crate.planner.symbol.Function;
 import io.crate.planner.symbol.Reference;
 import io.crate.planner.symbol.Symbol;
+import io.crate.planner.symbol.SymbolVisitor;
 
 import javax.annotation.Nullable;
 
@@ -40,7 +40,7 @@ public static boolean appliesTo(@Nullable Symbol input, ColumnIdent columnIdent)
         return false;
     }
 
-    private static class Visitor extends DefaultTraversalSymbolVisitor<ColumnIdent, Boolean> {
+    private static class Visitor extends SymbolVisitor<ColumnIdent, Boolean> {
 
         @Override
         protected Boolean visitSymbol(Symbol symbol, ColumnIdent context) {

sql/src/main/java/io/crate/analyze/where/WhereClauseAnalyzer.java

@@ -62,7 +62,6 @@ public WhereClause analyze(WhereClause whereClause) {
         }
         Set<Symbol> clusteredBy = null;
         if (whereClause.hasQuery() && !tableInfo.schemaInfo().systemSchema()){
-            // TODO: add functionality of the whereclaue validator into analyzer or planner
             WhereClauseValidator.validate(whereClause);
         }
 

sql/src/main/java/io/crate/operation/operator/CmpOperator.java

@@ -6,9 +6,7 @@
 import io.crate.planner.symbol.Function;
 import io.crate.planner.symbol.Literal;
 import io.crate.planner.symbol.Symbol;
-import io.crate.planner.symbol.SymbolFormatter;
 
-import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 
@@ -51,17 +49,7 @@ public Symbol normalizeSymbol(Function symbol) {
 
         if (left.symbolType().isValueSymbol() && right.symbolType().isValueSymbol()) {
             // must be true due to the function registration (argument DataType signature)
-
-            boolean equal = false;
-            try {
-                equal = compare(((Literal) left).compareTo((Literal) right));
-            } catch (ClassCastException e){
-                throw new IllegalArgumentException(
-                        String.format(Locale.ENGLISH, "unable to cast %s to type '%s'",
-                                SymbolFormatter.format(right),
-                                left.valueType().getName()));
-            }
-            if (equal) {
+            if (compare(((Literal) left).compareTo((Literal) right))){
                 return Literal.BOOLEAN_TRUE;
             } else {
                 return Literal.BOOLEAN_FALSE;

sql/src/main/java/io/crate/operation/scalar/cast/CastFunctionResolver.java

@@ -25,13 +25,9 @@
 import com.google.common.collect.ImmutableMap;
 import io.crate.metadata.FunctionIdent;
 import io.crate.metadata.FunctionInfo;
-import io.crate.planner.symbol.Function;
-import io.crate.planner.symbol.Symbol;
 import io.crate.types.DataType;
 import io.crate.types.DataTypes;
-import org.elasticsearch.common.Nullable;
 
-import java.util.Arrays;
 import java.util.Locale;
 
 public class CastFunctionResolver {
@@ -71,15 +67,4 @@ public static FunctionInfo functionInfo(DataType dataType, DataType returnType)
         return new FunctionInfo(new FunctionIdent(functionName, ImmutableList.of(dataType)), returnType);
     }
 
-    @Nullable
-    public static Symbol castSymbol(Symbol symbol, DataType targetType) {
-        if (symbol.valueType() == targetType) {
-            return symbol;
-        } else if (symbol.valueType().isConvertableTo(targetType)) {
-            return new Function(
-                    functionInfo(symbol.valueType(), targetType),
-                    Arrays.asList(symbol));
-        }
-        return null;
-    }
 }

sql/src/main/java/io/crate/planner/Planner.java

@@ -28,7 +28,6 @@
 import com.google.common.collect.Lists;
 import io.crate.analyze.*;
 import io.crate.analyze.relations.TableRelation;
-import io.crate.analyze.where.DocKeys;
 import io.crate.exceptions.UnhandledServerException;
 import io.crate.metadata.*;
 import io.crate.metadata.doc.DocSysColumns;
@@ -38,8 +37,8 @@
 import io.crate.planner.node.ddl.*;
 import io.crate.planner.node.dml.ESDeleteByQueryNode;
 import io.crate.planner.node.dml.ESDeleteNode;
-import io.crate.planner.node.dml.Upsert;
 import io.crate.planner.node.dml.SymbolBasedUpsertByIdNode;
+import io.crate.planner.node.dml.Upsert;
 import io.crate.planner.node.dql.CollectNode;
 import io.crate.planner.node.dql.DQLPlanNode;
 import io.crate.planner.node.dql.FileUriCollectNode;
@@ -394,7 +393,6 @@ public Plan visitSetStatement(SetAnalyzedStatement analysis, Context context) {
     }
 
     private void createESDeleteNode(TableInfo tableInfo, WhereClause whereClause, IterablePlan plan) {
-        DocKeys.DocKey docKey = whereClause.docKeys().get().getOnlyKey();
         plan.add(new ESDeleteNode(tableInfo, whereClause.docKeys().get().getOnlyKey()));
     }
 

sql/src/main/java/io/crate/planner/consumer/InsertFromSubQueryConsumer.java

@@ -28,12 +28,6 @@
 import io.crate.analyze.relations.AnalyzedRelation;
 import io.crate.analyze.relations.AnalyzedRelationVisitor;
 import io.crate.analyze.relations.PlannedAnalyzedRelation;
-import io.crate.analyze.relations.TableRelation;
-import io.crate.exceptions.VersionInvalidException;
-import io.crate.metadata.ColumnIdent;
-import io.crate.metadata.Functions;
-import io.crate.metadata.Routing;
-import io.crate.metadata.table.TableInfo;
 import io.crate.planner.PlanNodeBuilder;
 import io.crate.planner.node.dml.InsertFromSubQuery;
 import io.crate.planner.node.dql.MergeNode;

sql/src/test/java/io/crate/analyze/where/EqualityExtractorTest.java

@@ -339,20 +339,4 @@ public void testPK2_Eq1() throws Exception {
         assertNull(matches);
     }
 
-//    private void printComparisons(Set<List<EqualityExtractor.EqProxy>> cp) {
-//        for (List<EqualityExtractor.EqProxy> functions : cp) {
-//            for (EqualityExtractor.EqProxy proxy : functions) {
-//                if (proxy == EqualityExtractor.NULL_MARKER_PROXY){
-//                    System.out.print("NULL");
-//                } else {
-//                    System.out.print(((Reference) proxy.origin().arguments().get(0)).ident().columnIdent().name());
-//                    System.out.print("==");
-//                    System.out.print(((Literal) proxy.origin().arguments().get(1)).value());
-//                }
-//                System.out.print('\t');
-//            }
-//            System.out.println();
-//            System.out.println("----------");
-//        }
-//    }
 }

sql/src/test/java/io/crate/analyze/where/WhereClauseAnalyzerTest.java

@@ -176,12 +176,6 @@ private WhereClause analyzeSelect(String stmt, Object... args) {
         SelectAnalyzedStatement statement = (SelectAnalyzedStatement) analyzer.analyze(
                 SqlParser.createStatement(stmt), args, new Object[0][]).analyzedStatement();
         return statement.relation().querySpec().where();
-        //QueriedTable sourceRelation = (QueriedTable) statement.relation();
-//
-//
-//        TableRelation tableRelation = sourceRelation.tableRelation();
-//        WhereClauseAnalyzer whereClauseAnalyzer = new WhereClauseAnalyzer(ctxMetaData, tableRelation);
-//        return whereClauseAnalyzer.analyze(statement.relation().querySpec().where());
     }
 
     private WhereClause analyzeSelectWhere(String stmt) {
@@ -230,17 +224,9 @@ public void testSelectPartitionedByPK() throws Exception {
 
     }
 
-
-
-
-
-
     @Test
     public void testWherePartitionedByColumn() throws Exception {
         DeleteAnalyzedStatement statement = analyzeDelete("delete from parted where date = 1395874800000");
-        TableRelation tableRelation = statement.analyzedRelation();
-        //WhereClauseAnalyzer whereClauseAnalyzer = new WhereClauseAnalyzer(ctxMetaData, tableRelation);
-        //WhereClause whereClause = whereClauseAnalyzer.analyze(statement.whereClauses().get(0));
         WhereClause whereClause = statement.whereClauses().get(0);
 
         assertThat(whereClause.hasQuery(), is(false));