When N of batch of statements is smaller than N of values, session.batch() silently drops extra values.

[terencezl]

I was trying to bulk ingest with the same statement. So I went like this:

    let N = 100;
    let mut batch = Batch::default();
    for _ in 0..N {
        batch.append_statement("INSERT INTO metadata.metadata (id, data) VALUES(?, ?)");
    }
    let mut prepared_batch = session.prepare_batch(&batch).await?;

    for chunk in &db_iter.into_iter().chunks(N) {
        ...
        let mut values = vec![];
        let mut c = 0;
        for item in chunk {
            values.push((id_str.to_owned(), data));
            c += 1;
        }
        if c != N {
            // init new prepared_batch to size
            batch = Batch::default();
            for _ in 0..c {
                batch.append_statement(
                    "INSERT INTO metadata.metadata (id, data) VALUES(?, ?)",
                );
            }
            prepared_batch = session.prepare_batch(&batch).await?;
        }
        session.batch(&prepared_batch, values).await?;
    }

My understanding is you need to duplicate the prepared statement 100 times in a batch, even if they are identical. And the N of values need to match N of statements in a batch. So I had to take care of the last batch being not rounded to the step size.

When I tried to play around and have more statements than values in the batch:

    for _ in 0..200 {
        batch.append_statement("INSERT INTO metadata.metadata (id, data) VALUES(?, ?)");
    }

I am seeing

Error: Invalid message: Frame error: Could not serialize frame: Length of provided values must be equal to number of batch statements (got 100 values, 200 statements)

That's good. However, when I commented out the initial, basically having an empty batch of statements:

//    for _ in 0..N {
//        batch.append_statement("INSERT INTO metadata.metadata (id, data) VALUES(?, ?)");
//    }

This was when N statements < N values, it's silently going through. I looked at the count through cqlsh, and the ingested count was 0. If I only had 1 statement in the batch, and 100 values, it would just ingest 1 single entry. So maybe the client was doing an iterator zip, and trimmed the extra values silently. I would like the client to error on the inequality of N statements and N values.

[Lorak-mmk]

I think I see the problem.
The code that serializes the batch does check for this condition:

scylla-rust-driver/scylla-cql/src/frame/request/batch.rs

Lines 139 to 144 in 64b4afc

	if value_lists.skip_next().is_some() {
	return Err(counts_mismatch_err(
	n_serialized_statements + 1 /*skipped above*/ + value_lists.count(),
	n_serialized_statements,
	));
	}

The problem is that in connection code here

scylla-rust-driver/scylla/src/transport/connection.rs

Lines 1252 to 1268 in 64b4afc

	let contexts = batch.statements.iter().map(|bs| match bs {
	BatchStatement::Query(_) => RowSerializationContext::empty(),
	BatchStatement::PreparedStatement(ps) => {
	RowSerializationContext::from_prepared(ps.get_prepared_metadata())
	}
	});
	let values = RawBatchValuesAdapter::new(values, contexts);
	let batch_frame = batch::Batch {
	statements: Cow::Borrowed(&batch.statements),
	values,
	batch_type: batch.get_type(),
	consistency,
	serial_consistency,
	timestamp: batch.get_timestamp(),
	};

we create a RawBatchValuesAdapter from values (with length N) and contexts (with length equal to amount of statements, so < N).
RawBatchValuesIteratorAdapter (

scylla-rust-driver/scylla-cql/src/types/serialize/raw_batch.rs

Lines 145 to 161 in 64b4afc

	fn serialize_next(&mut self, writer: &mut RowWriter) -> Option<Result<(), SerializationError>> {
	let ctx = self.contexts.next()?;
	self.batch_values_iterator.serialize_next(&ctx, writer)
	}
	fn is_empty_next(&mut self) -> Option<bool> {
	self.contexts.next()?;
	let ret = self.batch_values_iterator.is_empty_next()?;
	Some(ret)
	}
	#[inline]
	fn skip_next(&mut self) -> Option<()> {
	self.contexts.next()?;
	self.batch_values_iterator.skip_next()?;
	Some(())
	}

), in its methods first calls let ctx = self.contexts.next()?; - effectively limiting the length of the iterator to the amount of statements.

Possible fix: RawBatchValuesIteratorAdapter should return Some(Err(...)) when length of contexts and values doesn't match,and the code in connection should propagate this error (instead of only checking for the element to be Some).

[Lorak-mmk]

It's a bit sad that we don't have a test for that :(

[fee-mendes]

Possible fix: RawBatchValuesIteratorAdapter should return Some(Err(...)) when length of contexts and values doesn't match,and the code in connection should propagate this error (instead of only checking for the element to be Some).

Yep, looks good enough. Nice catch!