Skip to content

Commit 0461ed3

Browse files
ifesdjeenolim7t
ifesdjeen
authored and
olim7t
committed
JAVA-1196: Include hash of result set metadata in prepared statement id
Motivation: See CASSANDRA-10786. When a table is altered (column is added), the wildcard queries keep the old result metadata and never return the added column for all but the very first client (as the very first client is the only one that receives UNPREPARED statement). After adding result metadata to md5 on Cassandra side, as the message metadata on the client is never updated and new client ID of the re-prepared message is not saved anywhere, client would fall into the infinite re-prepare loop. Modifications: With Protocol V5: - Include result metadata id in EXECUTE messages. - Read result metadata id in PREPARED and ROWS responses. - Replace cached prepared statements when result metadata id has changed. Result: Wildcard queries are now safe to use (see JAVA-420). Result metadata id is now correctly handled.
1 parent 5920f47 commit 0461ed3

File tree

15 files changed

+532
-66
lines changed

15 files changed

+532
-66
lines changed

‎changelog/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
### 3.3.2 (in progress)
44

55
- [bug] JAVA-1666: Fix keyspace export when a UDT has case-sensitive field names.
6+
- [improvement] JAVA-1196: Include hash of result set metadata in prepared statement id.
67

78

89
### 3.3.1

‎driver-core/src/main/java/com/datastax/driver/core/ArrayBackedResultSet.java‎

Lines changed: 63 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ abstract class ArrayBackedResultSet implements ResultSet {
3939

4040
private static final Queue<List<ByteBuffer>> EMPTY_QUEUE = new ArrayDeque<List<ByteBuffer>>(0);
4141

42-
protected final ColumnDefinitions metadata;
42+
protected volatile ColumnDefinitions metadata;
4343
protected final Token.Factory tokenFactory;
4444
private final boolean wasApplied;
4545

@@ -60,18 +60,42 @@ static ArrayBackedResultSet fromMessage(Responses.Result msg, SessionManager ses
6060
case ROWS:
6161
Responses.Result.Rows r = (Responses.Result.Rows) msg;
6262

63-
ColumnDefinitions columnDefs;
64-
if (r.metadata.columns == null) {
65-
Statement actualStatement = statement;
66-
if (statement instanceof StatementWrapper) {
67-
actualStatement = ((StatementWrapper) statement).getWrappedStatement();
68-
}
69-
assert actualStatement instanceof BoundStatement;
70-
columnDefs = ((BoundStatement) actualStatement).statement.getPreparedId().resultSetMetadata;
71-
assert columnDefs != null;
63+
Statement actualStatement = statement;
64+
if (statement instanceof StatementWrapper) {
65+
actualStatement = ((StatementWrapper) statement).getWrappedStatement();
66+
}
67+
68+
ColumnDefinitions columnDefs = r.metadata.columns;
69+
if (columnDefs == null) {
70+
// If result set metadata is not present, it means the request had SKIP_METADATA set, the driver
71+
// only ever does that for bound statements.
72+
BoundStatement bs = (BoundStatement) actualStatement;
73+
columnDefs = bs.preparedStatement().getPreparedId().resultSetMetadata.variables;
7274
} else {
73-
columnDefs = r.metadata.columns;
75+
// Otherwise, always use the response's metadata.
76+
// In addition, if a new id is present it means we're executing a bound statement with protocol v5,
77+
// the schema changed server-side, and we need to update the prepared statement (see
78+
// CASSANDRA-10786).
79+
MD5Digest newMetadataId = r.metadata.metadataId;
80+
assert !(actualStatement instanceof BoundStatement) ||
81+
ProtocolFeature.PREPARED_METADATA_CHANGES.isSupportedBy(protocolVersion) ||
82+
newMetadataId == null;
83+
if (newMetadataId != null) {
84+
BoundStatement bs = ((BoundStatement) actualStatement);
85+
PreparedId preparedId = bs.preparedStatement().getPreparedId();
86+
// Extra test for CASSANDRA-13992: conditional updates yield a different result set depending on
87+
// whether the update was applied or not, so the prepared statement must never have result
88+
// metadata, and we should always execute with skip_metadata = false.
89+
// However the server sends a new_metadata_id in the response, so make sure we ignore it if the
90+
// prepared statement did not have metadata in the first place.
91+
// TODO remove the "if" (i.e. always assign resultSetMetadata) if CASSANDRA-13992 gets fixed before 4.0.0 GA
92+
if (preparedId.resultSetMetadata.variables != null) {
93+
preparedId.resultSetMetadata =
94+
new PreparedId.PreparedMetadata(newMetadataId, columnDefs);
95+
}
96+
}
7497
}
98+
assert columnDefs != null;
7599

76100
Token.Factory tokenFactory = (session == null) ? null
77101
: session.getCluster().manager.metadata.tokenFactory();
@@ -224,7 +248,7 @@ public List<ExecutionInfo> getAllExecutionInfo() {
224248
private static class MultiPage extends ArrayBackedResultSet {
225249

226250
private Queue<List<ByteBuffer>> currentPage;
227-
private final Queue<Queue<List<ByteBuffer>>> nextPages = new ConcurrentLinkedQueue<Queue<List<ByteBuffer>>>();
251+
private final Queue<NextPage> nextPages = new ConcurrentLinkedQueue<NextPage>();
228252

229253
private final Deque<ExecutionInfo> infos = new LinkedBlockingDeque<ExecutionInfo>();
230254

@@ -280,8 +304,8 @@ public Row one() {
280304
@Override
281305
public int getAvailableWithoutFetching() {
282306
int available = currentPage.size();
283-
for (Queue<List<ByteBuffer>> page : nextPages)
284-
available += page.size();
307+
for (NextPage page : nextPages)
308+
available += page.data.size();
285309
return available;
286310
}
287311

@@ -297,9 +321,12 @@ private void prepareNextRow() {
297321
// Grab the current state now to get a consistent view in this iteration.
298322
FetchingState fetchingState = this.fetchState;
299323

300-
Queue<List<ByteBuffer>> nextPage = nextPages.poll();
324+
NextPage nextPage = nextPages.poll();
301325
if (nextPage != null) {
302-
currentPage = nextPage;
326+
if (nextPage.metadata != null) {
327+
this.metadata = nextPage.metadata;
328+
}
329+
currentPage = nextPage.data;
303330
continue;
304331
}
305332
if (fetchingState == null)
@@ -363,7 +390,16 @@ public void onSet(Connection connection, Message.Response response, ExecutionInf
363390
if (rm.kind == Responses.Result.Kind.ROWS) {
364391
Responses.Result.Rows rows = (Responses.Result.Rows) rm;
365392
info = update(info, rm, MultiPage.this.session, rows.metadata.pagingState, protocolVersion, codecRegistry, statement);
366-
MultiPage.this.nextPages.offer(rows.data);
393+
// If the query is a prepared 'SELECT *', the metadata can change between pages
394+
ColumnDefinitions newMetadata = null;
395+
if (rows.metadata.metadataId != null) {
396+
newMetadata = rows.metadata.columns;
397+
assert statement instanceof BoundStatement;
398+
BoundStatement bs = (BoundStatement) statement;
399+
bs.preparedStatement().getPreparedId().resultSetMetadata =
400+
new PreparedId.PreparedMetadata(rows.metadata.metadataId, rows.metadata.columns);
401+
}
402+
MultiPage.this.nextPages.offer(new NextPage(newMetadata, rows.data));
367403
MultiPage.this.fetchState = rows.metadata.pagingState == null ? null : new FetchingState(rows.metadata.pagingState, null);
368404
} else if (rm.kind == Responses.Result.Kind.VOID) {
369405
// We shouldn't really get a VOID message here but well, no harm in handling it I suppose
@@ -443,6 +479,16 @@ private static class FetchingState {
443479
this.inProgress = inProgress;
444480
}
445481
}
482+
483+
private static class NextPage {
484+
final ColumnDefinitions metadata;
485+
final Queue<List<ByteBuffer>> data;
486+
487+
NextPage(ColumnDefinitions metadata, Queue<List<ByteBuffer>> data) {
488+
this.metadata = metadata;
489+
this.data = data;
490+
}
491+
}
446492
}
447493

448494
// This method checks the value of the "[applied]" column manually, to avoid instantiating an ArrayBackedRow

‎driver-core/src/main/java/com/datastax/driver/core/BatchStatement.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ public enum Type {
6161
COUNTER
6262
}
6363

64-
;
65-
6664
final Type batchType;
6765
private final List<Statement> statements = new ArrayList<Statement>();
6866

@@ -97,7 +95,7 @@ IdAndValues getIdAndValues(ProtocolVersion protocolVersion, CodecRegistry codecR
9795
// We handle BatchStatement in add() so ...
9896
assert statement instanceof BoundStatement;
9997
BoundStatement st = (BoundStatement) statement;
100-
idAndVals.ids.add(st.statement.getPreparedId().id);
98+
idAndVals.ids.add(st.statement.getPreparedId().boundValuesMetadata.id);
10199
idAndVals.values.add(Arrays.asList(st.wrapper.values));
102100
}
103101
}

‎driver-core/src/main/java/com/datastax/driver/core/BoundStatement.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,10 @@ public BoundStatement setRoutingKey(ByteBuffer... routingKeyComponents) {
293293
*/
294294
@Override
295295
public String getKeyspace() {
296-
return statement.getPreparedId().metadata.size() == 0 ? null : statement.getPreparedId().metadata.getKeyspace(0);
296+
ColumnDefinitions defs = statement.getPreparedId().boundValuesMetadata.variables;
297+
return defs.size() == 0
298+
? null
299+
: defs.getKeyspace(0);
297300
}
298301

299302
/**

‎driver-core/src/main/java/com/datastax/driver/core/Cluster.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2218,13 +2218,15 @@ public void ensurePoolsSizing() {
22182218
}
22192219

22202220
public PreparedStatement addPrepared(PreparedStatement stmt) {
2221-
PreparedStatement previous = preparedQueries.putIfAbsent(stmt.getPreparedId().id, stmt);
2221+
PreparedStatement previous = preparedQueries.putIfAbsent(stmt.getPreparedId().boundValuesMetadata.id, stmt);
22222222
if (previous != null) {
22232223
logger.warn("Re-preparing already prepared query is generally an anti-pattern and will likely affect performance. "
22242224
+ "Consider preparing the statement only once. Query='{}'", stmt.getQueryString());
22252225

22262226
// The one object in the cache will get GCed once it's not referenced by the client anymore since we use a weak reference.
22272227
// So we need to make sure that the instance we do return to the user is the one that is in the cache.
2228+
// However if the result metadata changed since the last PREPARE call, this also needs to be updated.
2229+
previous.getPreparedId().resultSetMetadata = stmt.getPreparedId().resultSetMetadata;
22282230
return previous;
22292231
}
22302232
return stmt;

‎driver-core/src/main/java/com/datastax/driver/core/DefaultPreparedStatement.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,18 @@ static DefaultPreparedStatement fromMessage(Responses.Result.Prepared msg, Clust
5656
ColumnDefinitions defs = msg.metadata.columns;
5757

5858
ProtocolVersion protocolVersion = cluster.getConfiguration().getProtocolOptions().getProtocolVersion();
59-
60-
if (defs.size() == 0) {
61-
return new DefaultPreparedStatement(new PreparedId(msg.statementId, defs, msg.resultMetadata.columns, null, protocolVersion), query, queryKeyspace, msg.getCustomPayload(), cluster);
59+
PreparedId.PreparedMetadata boundValuesMetadata = new PreparedId.PreparedMetadata(msg.statementId, defs);
60+
PreparedId.PreparedMetadata resultSetMetadata = new PreparedId.PreparedMetadata(msg.resultMetadataId, msg.resultMetadata.columns);
61+
62+
int[] pkIndices = null;
63+
if (defs.size() > 0) {
64+
pkIndices = (protocolVersion.compareTo(V4) >= 0)
65+
? msg.metadata.pkIndices
66+
: computePkIndices(cluster.getMetadata(), defs);
6267
}
6368

64-
int[] pkIndices = (protocolVersion.compareTo(V4) >= 0)
65-
? msg.metadata.pkIndices
66-
: computePkIndices(cluster.getMetadata(), defs);
67-
68-
PreparedId prepId = new PreparedId(msg.statementId, defs, msg.resultMetadata.columns, pkIndices, protocolVersion);
69-
70-
return new DefaultPreparedStatement(prepId, query, queryKeyspace, msg.getCustomPayload(), cluster);
69+
PreparedId preparedId = new PreparedId(boundValuesMetadata, resultSetMetadata, pkIndices, protocolVersion);
70+
return new DefaultPreparedStatement(preparedId, query, queryKeyspace, msg.getCustomPayload(), cluster);
7171
}
7272

7373
private static int[] computePkIndices(Metadata clusterMetadata, ColumnDefinitions boundColumns) {
@@ -117,7 +117,7 @@ private static boolean allSet(int[] pkColumns) {
117117

118118
@Override
119119
public ColumnDefinitions getVariables() {
120-
return preparedId.metadata;
120+
return preparedId.boundValuesMetadata.variables;
121121
}
122122

123123
@Override

‎driver-core/src/main/java/com/datastax/driver/core/PreparedId.java‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,37 @@
1919
* Identifies a PreparedStatement.
2020
*/
2121
public class PreparedId {
22-
// This class is mostly here to group PreparedStatement data that are need for
23-
// execution but that we don't want to expose publicly (see JAVA-195)
24-
final MD5Digest id;
2522

26-
final ColumnDefinitions metadata;
27-
final ColumnDefinitions resultSetMetadata;
23+
// This class is mostly here to group PreparedStatement data that are needed for
24+
// execution but that we don't want to expose publicly (see JAVA-195)
2825

2926
final int[] routingKeyIndexes;
27+
3028
final ProtocolVersion protocolVersion;
3129

32-
PreparedId(MD5Digest id, ColumnDefinitions metadata, ColumnDefinitions resultSetMetadata, int[] routingKeyIndexes, ProtocolVersion protocolVersion) {
33-
this.id = id;
34-
this.metadata = metadata;
30+
final PreparedMetadata boundValuesMetadata;
31+
32+
// can change over time, see JAVA-1196, JAVA-420
33+
volatile PreparedMetadata resultSetMetadata;
34+
35+
PreparedId(PreparedMetadata boundValuesMetadata, PreparedMetadata resultSetMetadata, int[] routingKeyIndexes, ProtocolVersion protocolVersion) {
36+
assert boundValuesMetadata != null;
37+
assert resultSetMetadata != null;
38+
this.boundValuesMetadata = boundValuesMetadata;
3539
this.resultSetMetadata = resultSetMetadata;
3640
this.routingKeyIndexes = routingKeyIndexes;
3741
this.protocolVersion = protocolVersion;
3842
}
43+
44+
45+
static class PreparedMetadata {
46+
47+
final MD5Digest id;
48+
final ColumnDefinitions variables;
49+
50+
PreparedMetadata(MD5Digest id, ColumnDefinitions variables) {
51+
this.id = id;
52+
this.variables = variables;
53+
}
54+
}
3955
}

‎driver-core/src/main/java/com/datastax/driver/core/ProtocolFeature.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright (C) 2012-2017 DataStax Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.datastax.driver.core;
17+
18+
/**
19+
* A listing of features that may or not apply to a given {@link ProtocolVersion}.
20+
*/
21+
enum ProtocolFeature {
22+
23+
/**
24+
* The capability of updating a prepared statement if the result's metadata changes at runtime (for example, if the
25+
* query is a {@code SELECT *} and the table is altered).
26+
*/
27+
PREPARED_METADATA_CHANGES,
28+
//
29+
;
30+
31+
/**
32+
* Determines whether or not the input version supports ths feature.
33+
*
34+
* @param version the version to test against.
35+
* @return true if supported, false otherwise.
36+
*/
37+
boolean isSupportedBy(ProtocolVersion version) {
38+
switch (this) {
39+
case PREPARED_METADATA_CHANGES:
40+
return version == ProtocolVersion.V5;
41+
default:
42+
return false;
43+
}
44+
}
45+
46+
}

‎driver-core/src/main/java/com/datastax/driver/core/Requests.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,38 +183,48 @@ static class Execute extends Message.Request {
183183
@Override
184184
public void encode(Execute msg, ByteBuf dest, ProtocolVersion version) {
185185
CBUtil.writeBytes(msg.statementId.bytes, dest);
186+
if (ProtocolFeature.PREPARED_METADATA_CHANGES.isSupportedBy(version))
187+
CBUtil.writeBytes(msg.resultMetadataId.bytes, dest);
186188
msg.options.encode(dest, version);
187189
}
188190

189191
@Override
190192
public int encodedSize(Execute msg, ProtocolVersion version) {
191-
return CBUtil.sizeOfBytes(msg.statementId.bytes)
192-
+ msg.options.encodedSize(version);
193+
int size = CBUtil.sizeOfBytes(msg.statementId.bytes);
194+
if (ProtocolFeature.PREPARED_METADATA_CHANGES.isSupportedBy(version))
195+
size += CBUtil.sizeOfBytes(msg.resultMetadataId.bytes);
196+
size += msg.options.encodedSize(version);
197+
return size;
193198
}
194199
};
195200

196201
final MD5Digest statementId;
202+
final MD5Digest resultMetadataId;
197203
final QueryProtocolOptions options;
198204

199-
Execute(MD5Digest statementId, QueryProtocolOptions options, boolean tracingRequested) {
205+
Execute(MD5Digest statementId, MD5Digest resultMetadataId, QueryProtocolOptions options, boolean tracingRequested) {
200206
super(Message.Request.Type.EXECUTE, tracingRequested);
201207
this.statementId = statementId;
208+
this.resultMetadataId = resultMetadataId;
202209
this.options = options;
203210
}
204211

205212
@Override
206213
protected Request copyInternal() {
207-
return new Execute(statementId, options, isTracingRequested());
214+
return new Execute(statementId, resultMetadataId, options, isTracingRequested());
208215
}
209216

210217
@Override
211218
protected Request copyInternal(ConsistencyLevel newConsistencyLevel) {
212-
return new Execute(statementId, options.copy(newConsistencyLevel), isTracingRequested());
219+
return new Execute(statementId, resultMetadataId, options.copy(newConsistencyLevel), isTracingRequested());
213220
}
214221

215222
@Override
216223
public String toString() {
217-
return "EXECUTE " + statementId + " (" + options + ')';
224+
if (resultMetadataId != null)
225+
return "EXECUTE preparedId: " + statementId + " resultMetadataId: " + resultMetadataId + " (" + options + ')';
226+
else
227+
return "EXECUTE preparedId: " + statementId + " (" + options + ')';
218228
}
219229
}
220230

0 commit comments

Comments
 (0)