From ea71336278f7a4a9ed129bcd15655bf8c91bb46b Mon Sep 17 00:00:00 2001 From: Jacek Lewandowski Date: Thu, 16 Mar 2023 18:43:20 +0100 Subject: [PATCH] Improve transaction statement validation patch by Jacek Lewandowski; reviewed by David Capwell and Caleb Rackliffe for CASSANDRA-18302 --- src/antlr/Parser.g | 38 ++++++++-- .../cassandra/cql3/StatementSource.java | 76 +++++++++++++++++++ .../cql3/statements/BatchStatement.java | 50 +++++++++--- .../cql3/statements/DeleteStatement.java | 27 +++++-- .../statements/ModificationStatement.java | 53 ++++++++++--- .../cql3/statements/SelectStatement.java | 34 +++++++-- .../cql3/statements/TransactionStatement.java | 65 ++++++++-------- .../cql3/statements/UpdateStatement.java | 49 ++++++++---- .../org/apache/cassandra/db/view/View.java | 22 ++++-- .../cassandra/cql3/StatementSourceTest.java | 61 +++++++++++++++ .../statements/TransactionStatementTest.java | 26 ++++--- 11 files changed, 387 insertions(+), 114 deletions(-) create mode 100644 src/java/org/apache/cassandra/cql3/StatementSource.java create mode 100644 test/unit/org/apache/cassandra/cql3/StatementSourceTest.java diff --git a/src/antlr/Parser.g b/src/antlr/Parser.g index 8ac85ec1f4..f9fc2842df 100644 --- a/src/antlr/Parser.g +++ b/src/antlr/Parser.g @@ -34,6 +34,8 @@ options { protected List references; + private Token statementBeginMarker; + public static final Set reservedTypeNames = new HashSet() {{ add("byte"); @@ -216,6 +218,19 @@ options { { // Do nothing. } + + public Token stmtBegins() + { + statementBeginMarker = input.LT(1); + return statementBeginMarker; + } + + public StatementSource stmtSrc() + { + StatementSource stmtSrc = StatementSource.create(statementBeginMarker); + statementBeginMarker = null; + return stmtSrc; + } } /** STATEMENTS **/ @@ -292,6 +307,7 @@ selectStatement returns [SelectStatement.RawStatement expr] List groups = new ArrayList<>(); boolean allowFiltering = false; boolean isJson = false; + stmtBegins(); } : K_SELECT // json is a valid column name. By consequence, we need to resolve the ambiguity for "json - json" @@ -311,7 +327,7 @@ selectStatement returns [SelectStatement.RawStatement expr] isJson, null); WhereClause where = wclause == null ? WhereClause.empty() : wclause.build(); - $expr = new SelectStatement.RawStatement(cf, params, $sclause.selectors, where, limit, perPartitionLimit); + $expr = new SelectStatement.RawStatement(cf, params, $sclause.selectors, where, limit, perPartitionLimit, stmtSrc()); } ; @@ -324,11 +340,12 @@ letStatement returns [SelectStatement.RawStatement expr] Term.Raw limit = null; } : K_LET txnVar=IDENT '=' - '(' K_SELECT assignments=letSelectors K_FROM cf=columnFamilyName K_WHERE wclause=whereClause ( K_LIMIT rows=intValue { limit = rows; } )? ')' + '(' { stmtBegins(); } K_SELECT assignments=letSelectors K_FROM cf=columnFamilyName K_WHERE wclause=whereClause ( K_LIMIT rows=intValue { limit = rows; } )? ')' { SelectStatement.Parameters params = new SelectStatement.Parameters(Collections.emptyList(), Collections.emptyList(), false, false, false, $txnVar.text); WhereClause where = wclause == null ? WhereClause.empty() : wclause.build(); - $expr = new SelectStatement.RawStatement(cf, params, assignments, where, limit, null); + + $expr = new SelectStatement.RawStatement(cf, params, assignments, where, limit, null, stmtSrc()); } ; @@ -535,6 +552,9 @@ groupByClause[List groups] * */ insertStatement returns [ModificationStatement.Parsed expr] + @init { + stmtBegins(); + } : K_INSERT K_INTO cf=columnFamilyName ( st1=normalInsertStatement[cf] { $expr = st1; } | K_JSON st2=jsonInsertStatement[cf] { $expr = st2; }) @@ -553,7 +573,7 @@ normalInsertStatement [QualifiedName qn] returns [UpdateStatement.ParsedInsert e ( K_IF K_NOT K_EXISTS { ifNotExists = true; } )? ( usingClause[attrs] )? { - $expr = new UpdateStatement.ParsedInsert(qn, attrs, columnNames, values, ifNotExists); + $expr = new UpdateStatement.ParsedInsert(qn, attrs, columnNames, values, ifNotExists, stmtSrc()); } ; @@ -573,7 +593,7 @@ jsonInsertStatement [QualifiedName qn] returns [UpdateStatement.ParsedInsertJson ( K_IF K_NOT K_EXISTS { ifNotExists = true; } )? ( usingClause[attrs] )? { - $expr = new UpdateStatement.ParsedInsertJson(qn, attrs, val, defaultUnset, ifNotExists); + $expr = new UpdateStatement.ParsedInsertJson(qn, attrs, val, defaultUnset, ifNotExists, stmtSrc()); } ; @@ -604,6 +624,7 @@ updateStatement returns [UpdateStatement.ParsedUpdate expr] Attributes.Raw attrs = new Attributes.Raw(); UpdateStatement.OperationCollector operations = new UpdateStatement.OperationCollector(); boolean ifExists = false; + stmtBegins(); } : K_UPDATE cf=columnFamilyName ( usingClause[attrs] )? @@ -617,7 +638,8 @@ updateStatement returns [UpdateStatement.ParsedUpdate expr] wclause.build(), conditions == null ? Collections.emptyList() : conditions, ifExists, - isParsingTxn); + isParsingTxn, + stmtSrc()); } ; @@ -638,6 +660,7 @@ deleteStatement returns [DeleteStatement.Parsed expr] Attributes.Raw attrs = new Attributes.Raw(); List columnDeletions = Collections.emptyList(); boolean ifExists = false; + stmtBegins(); } : K_DELETE ( dels=deleteSelection { columnDeletions = dels; } )? K_FROM cf=columnFamilyName @@ -650,7 +673,8 @@ deleteStatement returns [DeleteStatement.Parsed expr] columnDeletions, wclause.build(), conditions == null ? Collections.emptyList() : conditions, - ifExists); + ifExists, + stmtSrc()); } ; diff --git a/src/java/org/apache/cassandra/cql3/StatementSource.java b/src/java/org/apache/cassandra/cql3/StatementSource.java new file mode 100644 index 0000000000..2f07ec4f53 --- /dev/null +++ b/src/java/org/apache/cassandra/cql3/StatementSource.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import java.util.Objects; + +import org.antlr.runtime.Token; + +import static java.lang.Math.max; +import static java.lang.Math.min; + +public class StatementSource +{ + public static final StatementSource INTERNAL = new StatementSource(0, 0); + + public final int line; + public final int charPositionInLine; + + public StatementSource(int line, int charPositionInLine) + { + this.line = line; + this.charPositionInLine = charPositionInLine; + } + + @Override + public String toString() + { + if (this == INTERNAL) + { + return "<<>>"; + } + else + { + if (!isEmpty()) + return String.format("at [%d:%d]", line + 1, charPositionInLine + 1); + else + return ""; + } + } + + public boolean isEmpty() + { + return line > Character.MAX_VALUE || line == Character.MAX_VALUE && charPositionInLine > Character.MAX_VALUE; + } + + // note - this can also reproduce the original statement raw text by getting TokenStream and calling toString(startToken, endToken) + public static StatementSource create(Token startToken) + { + Objects.requireNonNull(startToken); + + if (startToken.getType() == Token.EOF) + return new StatementSource(Character.MAX_VALUE + 1, 0); + + int startLine = min(max(startToken.getLine(), 1) - 1, Character.MAX_VALUE); + int startChar = min(max(startToken.getCharPositionInLine(), 0), Character.MAX_VALUE); + + return new StatementSource(startLine, startChar); + } + +} diff --git a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java index c99dac3121..939d7df767 100644 --- a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java @@ -18,7 +18,16 @@ package org.apache.cassandra.cql3.statements; import java.nio.ByteBuffer; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import com.google.common.annotations.VisibleForTesting; @@ -32,19 +41,37 @@ import org.slf4j.helpers.MessageFormatter; import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; -import org.apache.cassandra.db.guardrails.Guardrails; -import org.apache.cassandra.schema.TableId; -import org.apache.cassandra.schema.TableMetadata; -import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.config.DatabaseDescriptor; -import org.apache.cassandra.cql3.*; -import org.apache.cassandra.db.*; +import org.apache.cassandra.cql3.Attributes; +import org.apache.cassandra.cql3.BatchQueryOptions; +import org.apache.cassandra.cql3.CQLStatement; +import org.apache.cassandra.cql3.ColumnSpecification; +import org.apache.cassandra.cql3.QueryOptions; +import org.apache.cassandra.cql3.ResultSet; +import org.apache.cassandra.cql3.VariableSpecifications; +import org.apache.cassandra.db.Clustering; +import org.apache.cassandra.db.ConsistencyLevel; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.IMutation; +import org.apache.cassandra.db.RegularAndStaticColumns; +import org.apache.cassandra.db.Slice; +import org.apache.cassandra.db.Slices; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.rows.RowIterator; -import org.apache.cassandra.exceptions.*; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.exceptions.RequestExecutionException; +import org.apache.cassandra.exceptions.RequestValidationException; +import org.apache.cassandra.exceptions.UnauthorizedException; import org.apache.cassandra.metrics.BatchMetrics; import org.apache.cassandra.metrics.ClientRequestSizeMetrics; -import org.apache.cassandra.service.*; +import org.apache.cassandra.schema.ColumnMetadata; +import org.apache.cassandra.schema.TableId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.ClientWarn; +import org.apache.cassandra.service.QueryState; +import org.apache.cassandra.service.StorageProxy; import org.apache.cassandra.tracing.Tracing; import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.transport.messages.ResultMessage; @@ -53,7 +80,6 @@ import org.apache.cassandra.utils.NoSpamLogger; import org.apache.cassandra.utils.Pair; import static java.util.function.Predicate.isEqual; - import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; /** @@ -202,7 +228,7 @@ public class BatchStatement implements CQLStatement.CompositeCQLStatement for (ModificationStatement statement : statements) { if (timestampSet && statement.isTimestampSet()) - throw new InvalidRequestException("Timestamp must be set either on BATCH or individual statements"); + throw new InvalidRequestException("Timestamp must be set either on BATCH or individual statements: " + statement.source); if (statement.isCounter()) hasCounters = true; @@ -243,7 +269,7 @@ public class BatchStatement implements CQLStatement.CompositeCQLStatement for (ModificationStatement stmt : statements) { if (ksName != null && (!stmt.keyspace().equals(ksName) || !stmt.table().equals(cfName))) - throw new InvalidRequestException("Batch with conditions cannot span multiple tables"); + throw new InvalidRequestException("Batch with conditions cannot span multiple tables: " + stmt.source); ksName = stmt.keyspace(); cfName = stmt.table(); } diff --git a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java index 3ba34319a7..4ce68fd75d 100644 --- a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java @@ -20,9 +20,19 @@ package org.apache.cassandra.cql3.statements; import java.util.Collections; import java.util.List; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; + import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; -import org.apache.cassandra.cql3.*; +import org.apache.cassandra.cql3.Attributes; +import org.apache.cassandra.cql3.Operation; +import org.apache.cassandra.cql3.Operations; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.cql3.StatementSource; +import org.apache.cassandra.cql3.UpdateParameters; +import org.apache.cassandra.cql3.VariableSpecifications; +import org.apache.cassandra.cql3.WhereClause; import org.apache.cassandra.cql3.conditions.ColumnCondition; import org.apache.cassandra.cql3.conditions.Conditions; import org.apache.cassandra.cql3.restrictions.StatementRestrictions; @@ -33,8 +43,6 @@ import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.ClientState; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; import static org.apache.cassandra.cql3.statements.RequestValidations.checkTrue; @@ -49,9 +57,10 @@ public class DeleteStatement extends ModificationStatement Operations operations, StatementRestrictions restrictions, Conditions conditions, - Attributes attrs) + Attributes attrs, + StatementSource source) { - super(StatementType.DELETE, bindVariables, cfm, operations, restrictions, conditions, attrs); + super(StatementType.DELETE, bindVariables, cfm, operations, restrictions, conditions, attrs, source); } @Override @@ -132,9 +141,10 @@ public class DeleteStatement extends ModificationStatement List deletions, WhereClause whereClause, List conditions, - boolean ifExists) + boolean ifExists, + StatementSource source) { - super(name, StatementType.DELETE, attrs, conditions, false, ifExists); + super(name, StatementType.DELETE, attrs, conditions, false, ifExists, source); this.deletions = deletions; this.whereClause = whereClause; } @@ -175,7 +185,8 @@ public class DeleteStatement extends ModificationStatement operations, restrictions, conditions, - attrs); + attrs, + source); if (stmt.hasConditions() && !restrictions.hasAllPrimaryKeyColumnsRestrictedByEqualities()) { diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index a4a3c50de0..d34b73a8a7 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -35,7 +35,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.HashMultiset; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; - import com.google.common.collect.Lists; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,6 +52,7 @@ import org.apache.cassandra.cql3.QualifiedName; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.QueryProcessor; import org.apache.cassandra.cql3.ResultSet; +import org.apache.cassandra.cql3.StatementSource; import org.apache.cassandra.cql3.UpdateParameters; import org.apache.cassandra.cql3.Validation; import org.apache.cassandra.cql3.VariableSpecifications; @@ -76,25 +76,48 @@ import org.apache.cassandra.cql3.selection.ResultSetBuilder; import org.apache.cassandra.cql3.selection.Selection; import org.apache.cassandra.cql3.selection.Selection.Selectors; import org.apache.cassandra.cql3.transactions.ReferenceOperation; -import org.apache.cassandra.db.*; -import org.apache.cassandra.db.filter.*; +import org.apache.cassandra.db.CBuilder; +import org.apache.cassandra.db.Clustering; +import org.apache.cassandra.db.ConsistencyLevel; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.IMutation; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.RegularAndStaticColumns; +import org.apache.cassandra.db.SinglePartitionReadCommand; +import org.apache.cassandra.db.SinglePartitionReadQuery; +import org.apache.cassandra.db.Slice; +import org.apache.cassandra.db.Slices; +import org.apache.cassandra.db.filter.ClusteringIndexFilter; +import org.apache.cassandra.db.filter.ClusteringIndexNamesFilter; +import org.apache.cassandra.db.filter.ClusteringIndexSliceFilter; +import org.apache.cassandra.db.filter.ColumnFilter; +import org.apache.cassandra.db.filter.DataLimits; +import org.apache.cassandra.db.filter.RowFilter; import org.apache.cassandra.db.marshal.BooleanType; -import org.apache.cassandra.db.partitions.*; +import org.apache.cassandra.db.partitions.FilteredPartition; +import org.apache.cassandra.db.partitions.Partition; +import org.apache.cassandra.db.partitions.PartitionIterator; +import org.apache.cassandra.db.partitions.PartitionIterators; +import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.rows.RowIterator; import org.apache.cassandra.db.view.View; -import org.apache.cassandra.exceptions.*; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.exceptions.RequestExecutionException; +import org.apache.cassandra.exceptions.RequestValidationException; +import org.apache.cassandra.exceptions.UnauthorizedException; import org.apache.cassandra.metrics.ClientRequestSizeMetrics; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.service.StorageProxy; +import org.apache.cassandra.service.accord.txn.TxnReferenceOperation; +import org.apache.cassandra.service.accord.txn.TxnReferenceOperations; +import org.apache.cassandra.service.accord.txn.TxnWrite; import org.apache.cassandra.service.disk.usage.DiskUsageBroadcaster; import org.apache.cassandra.service.paxos.Ballot; import org.apache.cassandra.service.paxos.BallotGenerator; import org.apache.cassandra.service.paxos.Commit.Proposal; import org.apache.cassandra.transport.Dispatcher; -import org.apache.cassandra.service.accord.txn.TxnReferenceOperation; -import org.apache.cassandra.service.accord.txn.TxnReferenceOperations; -import org.apache.cassandra.service.accord.txn.TxnWrite; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.triggers.TriggerExecutor; import org.apache.cassandra.utils.ByteBufferUtil; @@ -140,13 +163,16 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa private final List functions; + public final StatementSource source; + public ModificationStatement(StatementType type, VariableSpecifications bindVariables, TableMetadata metadata, Operations operations, StatementRestrictions restrictions, Conditions conditions, - Attributes attrs) + Attributes attrs, + StatementSource source) { this.type = type; this.bindVariables = bindVariables; @@ -155,6 +181,7 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa this.operations = operations; this.conditions = conditions; this.attrs = attrs; + this.source = source; if (!conditions.isEmpty()) { @@ -1062,13 +1089,15 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa private final List conditions; private final boolean ifNotExists; private final boolean ifExists; + protected final StatementSource source; protected Parsed(QualifiedName name, StatementType type, Attributes.Raw attrs, List conditions, boolean ifNotExists, - boolean ifExists) + boolean ifExists, + StatementSource source) { super(name); this.type = type; @@ -1076,6 +1105,7 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa this.conditions = conditions == null ? Collections.emptyList() : conditions; this.ifNotExists = ifNotExists; this.ifExists = ifExists; + this.source = source; } public ModificationStatement prepare(ClientState state) @@ -1202,6 +1232,7 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa null, null, ONE, - null); + null, + StatementSource.INTERNAL); } } diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 26d8d3f467..4b104e0a79 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -56,6 +56,7 @@ import org.apache.cassandra.cql3.QualifiedName; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.QueryProcessor; import org.apache.cassandra.cql3.ResultSet; +import org.apache.cassandra.cql3.StatementSource; import org.apache.cassandra.cql3.VariableSpecifications; import org.apache.cassandra.cql3.WhereClause; import org.apache.cassandra.cql3.functions.Function; @@ -187,6 +188,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, */ private final ColumnComparator> orderingComparator; + public final StatementSource source; + // Used by forSelection below public static final Parameters defaultParameters = new Parameters(Collections.emptyList(), Collections.emptyList(), @@ -203,7 +206,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, AggregationSpecification.Factory aggregationSpecFactory, ColumnComparator> orderingComparator, Term limit, - Term perPartitionLimit) + Term perPartitionLimit, + StatementSource source) { this.table = table; this.bindVariables = bindVariables; @@ -215,6 +219,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, this.parameters = parameters; this.limit = limit; this.perPartitionLimit = perPartitionLimit; + this.source = source; } @Override @@ -281,7 +286,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, null, null, null, - null); + null, + StatementSource.INTERNAL); } @Override @@ -761,6 +767,11 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, return restrictions; } + public boolean isPartitionRangeQuery() + { + return isForPartitionRange(restrictions); + } + private ReadQuery getSliceCommands(QueryOptions options, ClientState state, ColumnFilter columnFilter, RowFilter rowFilter, DataLimits limit, long nowInSec) { @@ -1205,6 +1216,11 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, cqlRows.rows.sort(comparator); } + private static boolean isForPartitionRange(StatementRestrictions restrictions) + { + return restrictions.isKeyRange() || restrictions.usesSecondaryIndexing(); + } + public static class RawStatement extends QualifiedStatement { public final Parameters parameters; @@ -1213,13 +1229,15 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, public final Term.Raw limit; public final Term.Raw perPartitionLimit; private ClientState state; + private final StatementSource source; public RawStatement(QualifiedName cfName, Parameters parameters, List selectClause, WhereClause whereClause, Term.Raw limit, - Term.Raw perPartitionLimit) + Term.Raw perPartitionLimit, + StatementSource source) { super(cfName); this.parameters = parameters; @@ -1227,6 +1245,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, this.whereClause = whereClause; this.limit = limit; this.perPartitionLimit = perPartitionLimit; + this.source = source; } public SelectStatement prepare(ClientState state) @@ -1315,7 +1334,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, aggregationSpecFactory, orderingComparator, prepareLimit(variableSpecifications, limit, keyspace(), limitReceiver()), - prepareLimit(variableSpecifications, perPartitionLimit, keyspace(), perPartitionLimitReceiver())); + prepareLimit(variableSpecifications, perPartitionLimit, keyspace(), perPartitionLimitReceiver()), + source); } private Set getResultSetOrdering(StatementRestrictions restrictions, Map orderingColumns) @@ -1646,7 +1666,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, private void checkNeedsFiltering(TableMetadata table, StatementRestrictions restrictions) throws InvalidRequestException { // non-key-range non-indexed queries cannot involve filtering underneath - if (!parameters.allowFiltering && (restrictions.isKeyRange() || restrictions.usesSecondaryIndexing())) + if (!parameters.allowFiltering && isForPartitionRange(restrictions)) { // We will potentially filter data if the row filter is not the identity and there isn't any index group // supporting all the expressions in the filter. @@ -1857,7 +1877,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, private String loggableTokens(QueryOptions options, ClientState state) { - if (restrictions.isKeyRange() || restrictions.usesSecondaryIndexing()) + if (isPartitionRangeQuery()) { AbstractBounds bounds = restrictions.getPartitionKeyBounds(options); return "token range: " + (bounds.inclusiveLeft() ? '[' : '(') + @@ -1894,7 +1914,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, sb.append("SELECT ").append(queriedColumns().toCQLString()); sb.append(" FROM ").append(table.keyspace).append('.').append(table.name); - if (restrictions.isKeyRange() || restrictions.usesSecondaryIndexing()) + if (isPartitionRangeQuery()) { // partition range ClusteringIndexFilter clusteringIndexFilter = makeClusteringIndexFilter(options, state, columnFilter); diff --git a/src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java b/src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java index a596a5a0e9..1625ab40e9 100644 --- a/src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java @@ -58,7 +58,6 @@ import org.apache.cassandra.cql3.transactions.ConditionStatement; import org.apache.cassandra.cql3.transactions.ReferenceOperation; import org.apache.cassandra.cql3.transactions.RowDataReference; import org.apache.cassandra.cql3.transactions.SelectReferenceSource; -import org.apache.cassandra.db.ReadQuery; import org.apache.cassandra.db.SinglePartitionReadCommand; import org.apache.cassandra.db.SinglePartitionReadQuery; import org.apache.cassandra.db.marshal.AbstractType; @@ -79,7 +78,6 @@ import org.apache.cassandra.service.accord.txn.TxnWrite; import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.utils.FBUtilities; -import org.apache.cassandra.utils.LazyToString; import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; import static org.apache.cassandra.cql3.statements.RequestValidations.checkNotNull; @@ -90,14 +88,13 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, private static final Logger logger = LoggerFactory.getLogger(TransactionStatement.class); public static final String DUPLICATE_TUPLE_NAME_MESSAGE = "The name '%s' has already been used by a LET assignment."; - public static final String INCOMPLETE_PRIMARY_KEY_LET_MESSAGE = "SELECT in LET assignment must specify either all primary key elements or all partition key elements and LIMIT 1. In both cases partition key elements must be always specified with equality operators; CQL %s"; - public static final String INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE = "Normal SELECT must specify either all primary key elements or all partition key elements and LIMIT 1. In both cases partition key elements must be always specified with equality operators; CQL %s"; - public static final String NO_CONDITIONS_IN_UPDATES_MESSAGE = "Updates within transactions may not specify their own conditions."; - public static final String NO_TIMESTAMPS_IN_UPDATES_MESSAGE = "Updates within transactions may not specify custom timestamps."; + public static final String INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE = "SELECT must specify either all primary key elements or all partition key elements and LIMIT 1. In both cases partition key elements must be always specified with equality operators; %s %s"; + public static final String NO_CONDITIONS_IN_UPDATES_MESSAGE = "Updates within transactions may not specify their own conditions; %s statement %s"; + public static final String NO_TIMESTAMPS_IN_UPDATES_MESSAGE = "Updates within transactions may not specify custom timestamps; %s statement %s"; public static final String EMPTY_TRANSACTION_MESSAGE = "Transaction contains no reads or writes"; public static final String SELECT_REFS_NEED_COLUMN_MESSAGE = "SELECT references must specify a column."; public static final String TRANSACTIONS_DISABLED_MESSAGE = "Accord transactions are disabled. (See accord_transactions_enabled in cassandra.yaml)"; - public static final String ILLEGAL_RANGE_QUERY_MESSAGE = "Range queries are not allowed for reads within a transaction"; + public static final String ILLEGAL_RANGE_QUERY_MESSAGE = "Range queries are not allowed for reads within a transaction; %s %s"; static class NamedSelect { @@ -208,12 +205,9 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, TxnNamedRead createNamedRead(NamedSelect namedSelect, QueryOptions options, ClientState state) { SelectStatement select = namedSelect.select; - ReadQuery readQuery = select.getQuery(options, 0); - checkTrue(readQuery instanceof SinglePartitionReadQuery.Group, ILLEGAL_RANGE_QUERY_MESSAGE, select.asCQL(options, state)); - // We reject reads from both LET and SELECT that do not specify a single row. @SuppressWarnings("unchecked") - SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) readQuery; + SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) select.getQuery(options, 0); if (selectQuery.queries.size() != 1) throw new IllegalArgumentException("Within a transaction, SELECT statements must select a single partition; found " + selectQuery.queries.size() + " partitions"); @@ -224,12 +218,9 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, List createNamedReads(NamedSelect namedSelect, QueryOptions options, ClientState state) { SelectStatement select = namedSelect.select; - ReadQuery readQuery = select.getQuery(options, 0); - checkTrue(readQuery instanceof SinglePartitionReadQuery.Group, ILLEGAL_RANGE_QUERY_MESSAGE, select.asCQL(options, state)); - // We reject reads from both LET and SELECT that do not specify a single row. @SuppressWarnings("unchecked") - SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) readQuery; + SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) select.getQuery(options, 0); if (selectQuery.queries.size() == 1) return Collections.singletonList(new TxnNamedRead(namedSelect.name, Iterables.getOnlyElement(selectQuery.queries))); @@ -340,24 +331,24 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, } } - private static void checkAtMostOneRowSpecified(ClientState clientState, @Nullable QueryOptions options, SelectStatement select, String failureMessage) + /** + * Returns {@code true} only if the statement selects multiple clusterings in a partition + */ + private static boolean isSelectingMultipleClusterings(SelectStatement select, @Nullable QueryOptions options) { if (select.getRestrictions().hasAllPrimaryKeyColumnsRestrictedByEqualities()) - return; + return false; if (options == null) { - // If the limit is a non-terminal marker (because we're preparing), defer validation until execution. + // if the limit is a non-terminal marker (because we're preparing), defer validation until execution (when options != null) if (select.isLimitMarker()) - return; + return false; - // The limit is already defined, so proceed with validation... options = QueryOptions.DEFAULT; } - int limit = select.getLimit(options); - QueryOptions finalOptions = options; // javac thinks this is mutable so requires a copy - checkTrue(limit == 1 && select.getRestrictions().hasAllPartitionKeyColumnsRestrictedByEqualities(), failureMessage, LazyToString.lazy(() -> select.asCQL(finalOptions, clientState))); + return select.getLimit(options) != 1; } @Override @@ -367,21 +358,19 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, try { + // check again since now we have query options; note that statements are quaranted to be single partition reads at this point for (NamedSelect assignment : assignments) - checkAtMostOneRowSpecified(state.getClientState(), options, assignment.select, INCOMPLETE_PRIMARY_KEY_LET_MESSAGE); + checkFalse(isSelectingMultipleClusterings(assignment.select, options), INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "LET assignment", assignment.select.source); if (returningSelect != null) - checkAtMostOneRowSpecified(state.getClientState(), options, returningSelect.select, INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE); + checkFalse(isSelectingMultipleClusterings(returningSelect.select, options), INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "returning SELECT", returningSelect.select.source); TxnData data = AccordService.instance().coordinate(createTxn(state.getClientState(), options), options.getConsistency()); if (returningSelect != null) { - ReadQuery readQuery = returningSelect.select.getQuery(options, 0); - checkTrue(readQuery instanceof SinglePartitionReadQuery.Group, ILLEGAL_RANGE_QUERY_MESSAGE, returningSelect.select.asCQL(options, state.getClientState())); - @SuppressWarnings("unchecked") - SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) readQuery; + SinglePartitionReadQuery.Group selectQuery = (SinglePartitionReadQuery.Group) returningSelect.select.getQuery(options, 0); Selection.Selectors selectors = returningSelect.select.getSelection().newSelectors(options); ResultSetBuilder result = new ResultSetBuilder(resultMetadata, selectors, false); if (selectQuery.queries.size() == 1) @@ -513,7 +502,7 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, SelectStatement prepared = select.prepare(bindVariables); NamedSelect namedSelect = new NamedSelect(name, prepared); - checkAtMostOneRowSpecified(state, null, namedSelect.select, INCOMPLETE_PRIMARY_KEY_LET_MESSAGE); + checkAtMostOneRowSpecified(namedSelect.select, "LET assignment " + name.name()); preparedAssignments.add(namedSelect); refSources.put(name, new SelectReferenceSource(prepared)); } @@ -526,7 +515,7 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, if (select != null) { returningSelect = new NamedSelect(TxnDataName.returning(), select.prepare(bindVariables)); - checkAtMostOneRowSpecified(state, null, returningSelect.select, INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE); + checkAtMostOneRowSpecified(returningSelect.select, "returning select"); } List returningReferences = null; @@ -547,8 +536,8 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, ModificationStatement.Parsed parsed = updates.get(i); ModificationStatement prepared = parsed.prepare(state, bindVariables); - checkFalse(prepared.hasConditions(), NO_CONDITIONS_IN_UPDATES_MESSAGE); - checkFalse(prepared.isTimestampSet(), NO_TIMESTAMPS_IN_UPDATES_MESSAGE); + checkFalse(prepared.hasConditions(), NO_CONDITIONS_IN_UPDATES_MESSAGE, prepared.type, prepared.source); + checkFalse(prepared.isTimestampSet(), NO_TIMESTAMPS_IN_UPDATES_MESSAGE, prepared.type, prepared.source); preparedUpdates.add(prepared); } @@ -560,5 +549,15 @@ public class TransactionStatement implements CQLStatement.CompositeCQLStatement, return new TransactionStatement(preparedAssignments, returningSelect, returningReferences, preparedUpdates, preparedConditions, bindVariables); } + + /** + * Do not use this method in execution!!! It is only allowed during prepare because it outputs a query raw text. + * We don't want it print it for a user who provided an identifier of someone's else prepared statement. + */ + private static void checkAtMostOneRowSpecified(SelectStatement select, String name) + { + checkFalse(select.isPartitionRangeQuery(), ILLEGAL_RANGE_QUERY_MESSAGE, name, select.source); + checkFalse(isSelectingMultipleClusterings(select, null), INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, name, select.source); + } } } diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index 2344850cee..d5002731f5 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -24,16 +24,29 @@ import java.util.Collections; import java.util.List; import com.google.common.base.Preconditions; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; -import org.apache.cassandra.cql3.*; +import org.apache.cassandra.cql3.Attributes; +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.Relation; +import org.apache.cassandra.cql3.terms.Constants; +import org.apache.cassandra.cql3.Json; +import org.apache.cassandra.cql3.Operation; +import org.apache.cassandra.cql3.Operations; +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.cql3.StatementSource; +import org.apache.cassandra.cql3.terms.Term; +import org.apache.cassandra.cql3.UpdateParameters; +import org.apache.cassandra.cql3.VariableSpecifications; +import org.apache.cassandra.cql3.WhereClause; import org.apache.cassandra.cql3.conditions.ColumnCondition; import org.apache.cassandra.cql3.conditions.Conditions; import org.apache.cassandra.cql3.constraints.ColumnConstraint; import org.apache.cassandra.cql3.restrictions.StatementRestrictions; -import org.apache.cassandra.cql3.terms.Constants; -import org.apache.cassandra.cql3.terms.Term; import org.apache.cassandra.cql3.transactions.ReferenceOperation; import org.apache.cassandra.cql3.transactions.ReferenceValue; import org.apache.cassandra.db.Clustering; @@ -47,8 +60,6 @@ import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.accord.txn.TxnReferenceOperation; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.Pair; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; import static org.apache.cassandra.cql3.statements.RequestValidations.checkContainsNoDuplicates; import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; @@ -70,9 +81,10 @@ public class UpdateStatement extends ModificationStatement Operations operations, StatementRestrictions restrictions, Conditions conditions, - Attributes attrs) + Attributes attrs, + StatementSource source) { - super(type, bindVariables, metadata, operations, restrictions, conditions, attrs); + super(type, bindVariables, metadata, operations, restrictions, conditions, attrs, source); } @Override @@ -145,9 +157,10 @@ public class UpdateStatement extends ModificationStatement Attributes.Raw attrs, List columnNames, List columnValues, - boolean ifNotExists) + boolean ifNotExists, + StatementSource source) { - super(name, StatementType.INSERT, attrs, null, ifNotExists, false); + super(name, StatementType.INSERT, attrs, null, ifNotExists, false, source); this.columnNames = columnNames; this.columnValues = columnValues; } @@ -219,7 +232,8 @@ public class UpdateStatement extends ModificationStatement operations, restrictions, conditions, - attrs); + attrs, + source); } } @@ -231,9 +245,9 @@ public class UpdateStatement extends ModificationStatement private final Json.Raw jsonValue; private final boolean defaultUnset; - public ParsedInsertJson(QualifiedName name, Attributes.Raw attrs, Json.Raw jsonValue, boolean defaultUnset, boolean ifNotExists) + public ParsedInsertJson(QualifiedName name, Attributes.Raw attrs, Json.Raw jsonValue, boolean defaultUnset, boolean ifNotExists, StatementSource source) { - super(name, StatementType.INSERT, attrs, null, ifNotExists, false); + super(name, StatementType.INSERT, attrs, null, ifNotExists, false, source); this.jsonValue = jsonValue; this.defaultUnset = defaultUnset; } @@ -290,7 +304,8 @@ public class UpdateStatement extends ModificationStatement operations, restrictions, conditions, - attrs); + attrs, + source); } } @@ -364,9 +379,10 @@ public class UpdateStatement extends ModificationStatement WhereClause whereClause, List conditions, boolean ifExists, - boolean isForTxn) + boolean isForTxn, + StatementSource source) { - super(name, StatementType.UPDATE, attrs, conditions, false, ifExists); + super(name, StatementType.UPDATE, attrs, conditions, false, ifExists, source); this.updates = updates; this.whereClause = whereClause; this.isForTxn = isForTxn; @@ -413,7 +429,8 @@ public class UpdateStatement extends ModificationStatement operations, restrictions, conditions, - attrs); + attrs, + source); } } diff --git a/src/java/org/apache/cassandra/db/view/View.java b/src/java/org/apache/cassandra/db/view/View.java index e926edb3a9..36894127ae 100644 --- a/src/java/org/apache/cassandra/db/view/View.java +++ b/src/java/org/apache/cassandra/db/view/View.java @@ -17,19 +17,26 @@ */ package org.apache.cassandra.db.view; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.stream.Collectors; - import javax.annotation.Nullable; import com.google.common.collect.Iterables; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -import org.apache.cassandra.cql3.*; +import org.apache.cassandra.cql3.QualifiedName; +import org.apache.cassandra.cql3.QueryOptions; +import org.apache.cassandra.cql3.StatementSource; import org.apache.cassandra.cql3.selection.RawSelector; import org.apache.cassandra.cql3.selection.Selectable; import org.apache.cassandra.cql3.statements.SelectStatement; -import org.apache.cassandra.db.*; -import org.apache.cassandra.db.rows.*; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.ReadQuery; +import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.schema.Schema; @@ -37,8 +44,6 @@ import org.apache.cassandra.schema.TableMetadataRef; import org.apache.cassandra.schema.ViewMetadata; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.FBUtilities; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A View copies data from a base table into a view table which can be queried independently from the @@ -174,7 +179,8 @@ public class View selectClause(), definition.whereClause, null, - null); + null, + StatementSource.INTERNAL); rawSelect.setBindVariables(Collections.emptyList()); diff --git a/test/unit/org/apache/cassandra/cql3/StatementSourceTest.java b/test/unit/org/apache/cassandra/cql3/StatementSourceTest.java new file mode 100644 index 0000000000..b6362747d5 --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/StatementSourceTest.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import org.junit.Test; + +import org.antlr.runtime.Token; +import org.mockito.Mockito; + +import static org.apache.cassandra.cql3.StatementSource.create; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +public class StatementSourceTest +{ + private static Token token(int line, int pos) + { + Token token = Mockito.mock(Token.class); + when(token.getLine()).thenReturn(line); + when(token.getCharPositionInLine()).thenReturn(pos); + when(token.getType()).thenReturn(1); + return token; + } + + private static Token eof() + { + Token token = Mockito.mock(Token.class); + when(token.getLine()).thenThrow(UnsupportedOperationException.class); + when(token.getCharPositionInLine()).thenThrow(UnsupportedOperationException.class); + when(token.getType()).thenReturn(Token.EOF); + return token; + } + + @Test + public void test() + { + assertThat(create(token(1, 4))).hasToString("at [1:5]"); + assertThat(create(token(3, 8))).hasToString("at [3:9]"); + assertThat(create(token(6, 8))).hasToString("at [6:9]"); + assertThat(create(token(1, 0))).hasToString("at [1:1]"); + assertThat(create(eof()).toString()).isEmpty(); + + assertThat(StatementSource.INTERNAL).hasToString("<<>>"); + } +} diff --git a/test/unit/org/apache/cassandra/cql3/statements/TransactionStatementTest.java b/test/unit/org/apache/cassandra/cql3/statements/TransactionStatementTest.java index 2b2750476d..95de83ff1b 100644 --- a/test/unit/org/apache/cassandra/cql3/statements/TransactionStatementTest.java +++ b/test/unit/org/apache/cassandra/cql3/statements/TransactionStatementTest.java @@ -36,7 +36,7 @@ import org.apache.cassandra.transport.messages.ResultMessage; import static org.apache.cassandra.cql3.statements.TransactionStatement.DUPLICATE_TUPLE_NAME_MESSAGE; import static org.apache.cassandra.cql3.statements.TransactionStatement.EMPTY_TRANSACTION_MESSAGE; -import static org.apache.cassandra.cql3.statements.TransactionStatement.INCOMPLETE_PRIMARY_KEY_LET_MESSAGE; +import static org.apache.cassandra.cql3.statements.TransactionStatement.ILLEGAL_RANGE_QUERY_MESSAGE; import static org.apache.cassandra.cql3.statements.TransactionStatement.INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE; import static org.apache.cassandra.cql3.statements.TransactionStatement.NO_CONDITIONS_IN_UPDATES_MESSAGE; import static org.apache.cassandra.cql3.statements.TransactionStatement.NO_TIMESTAMPS_IN_UPDATES_MESSAGE; @@ -162,7 +162,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_LET_MESSAGE, letSelect)); + .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "LET assignment row1", "at [2:15]")); } @Test @@ -176,7 +176,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> execute(query, 2)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_LET_MESSAGE, letSelect.replace("?", "2"))); + .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "LET assignment", "at [2:15]")); } @Test @@ -190,7 +190,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_LET_MESSAGE, letSelect)); + .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "LET assignment row1", "at [2:15]")); } @Test @@ -201,7 +201,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, select)); + .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "returning select", "at [2:1]")); } @Test @@ -212,7 +212,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, select)); + .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "returning select", "at [2:1]")); } @Test @@ -224,7 +224,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(NO_CONDITIONS_IN_UPDATES_MESSAGE); + .hasMessageContaining(NO_CONDITIONS_IN_UPDATES_MESSAGE, "INSERT", "at [2:3]"); } @Test @@ -236,7 +236,7 @@ public class TransactionStatementTest Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(NO_TIMESTAMPS_IN_UPDATES_MESSAGE); + .hasMessageContaining(NO_TIMESTAMPS_IN_UPDATES_MESSAGE, "INSERT", "at [2:3]"); } @Test @@ -336,26 +336,28 @@ public class TransactionStatementTest @Test public void shouldRejectNormalSelectWithIncompletePartitionKey() { + String select = "SELECT k, v FROM ks.tbl5 LIMIT 1"; String query = "BEGIN TRANSACTION\n" + - " SELECT k, v FROM ks.tbl5 LIMIT 1;\n" + + select + ";\n" + "COMMIT TRANSACTION;\n"; Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "SELECT v FROM ks.tbl5 LIMIT 1")); + .hasMessageContaining(String.format(ILLEGAL_RANGE_QUERY_MESSAGE, "returning select", "at [2:1]")); } @Test public void shouldRejectLetSelectWithIncompletePartitionKey() { + String select = "SELECT k, v FROM ks.tbl5 WHERE token(k) > token(123) LIMIT 1"; String query = "BEGIN TRANSACTION\n" + - " LET row1 = (SELECT k, v FROM ks.tbl5 WHERE token(k) > token(123) LIMIT 1); \n" + + " LET row1 = (" + select + "); \n" + " SELECT row1.k, row1.v;\n" + "COMMIT TRANSACTION;\n"; Assertions.assertThatThrownBy(() -> prepare(query)) .isInstanceOf(InvalidRequestException.class) - .hasMessageContaining(String.format(INCOMPLETE_PRIMARY_KEY_LET_MESSAGE, "SELECT v FROM ks.tbl5 WHERE token(k) > 0000007b LIMIT 1")); + .hasMessageContaining(String.format(ILLEGAL_RANGE_QUERY_MESSAGE, "LET assignment row1", "at [2:15]")); } private static CQLStatement prepare(String query)