lukasz-antoniak commented on code in PR #1782:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1782#discussion_r1728628641
##########
conn.go:
##########
@@ -1394,9 +1394,10 @@ func (c *Conn) executeQuery(ctx context.Context, qry
*Query) *Iter {
params.skipMeta = !(c.session.cfg.DisableSkipMetadata ||
qry.disableSkipMetadata)
frame = &writeExecuteFrame{
- preparedID: info.id,
- params: params,
- customPayload: qry.customPayload,
+ preparedID: info.id,
+ params: params,
+ customPayload: qry.customPayload,
+ preparedMetadataID: info.request.id,
Review Comment:
This is definitely not correct. You should send here `<result_metadata_id>`
received from PREPARE response, not the `<id>` of PREPARE response.
##########
frame_test.go:
##########
@@ -127,3 +127,27 @@ func TestFrameReadTooLong(t *testing.T) {
t.Fatalf("expected to get header %v got %v", opReady, head.op)
}
}
+
+func Test_framer_writeExecuteFrame_preparedMetadataID(t *testing.T) {
Review Comment:
We need an integration test to validate `<result_metadata_id>` logic.
Details of why it was included can be found in ticket
[CASSANDRA-10786](https://issues.apache.org/jira/browse/CASSANDRA-10786).
##########
frame.go:
##########
@@ -1604,23 +1611,31 @@ type writeExecuteFrame struct {
// v4+
customPayload map[string][]byte
+
+ // v5+
+ preparedMetadataID []byte
Review Comment:
I would consider following naming convention from protocol specification, so
this would be `result_metadata_id`.
##########
frame.go:
##########
@@ -952,6 +955,10 @@ func (f *framer) parsePreparedMetadata() preparedMetadata {
// TODO: deduplicate this from parseMetadata
meta := preparedMetadata{}
+ if f.proto > protoVersion4 {
Review Comment:
`<id>` field of PREPARE response is already present in V4. Here we are
missing reading of `<result_metadata_id>`.
##########
conn.go:
##########
@@ -1394,9 +1394,10 @@ func (c *Conn) executeQuery(ctx context.Context, qry
*Query) *Iter {
params.skipMeta = !(c.session.cfg.DisableSkipMetadata ||
qry.disableSkipMetadata)
frame = &writeExecuteFrame{
- preparedID: info.id,
- params: params,
- customPayload: qry.customPayload,
+ preparedID: info.id,
+ params: params,
+ customPayload: qry.customPayload,
+ preparedMetadataID: info.request.id,
Review Comment:
Following part of specification has not been implemented:
```
If a RESULT/Rows message reports
changed resultset metadata with the Metadata_changed flag, the reported new
resultset metadata must be used in subsequent executions.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
