scylladb / scylladb/python-rs-driver

Bug in SerializeRow for dicts

Open
#43 0 comments 0 reactions 1 assignee View on GitHub

@paulinaczajkowska is already working on this.

Since Feb 19, 2026.

area/serialization bug
Dominant language
Rust
Stars
6
Forks
7
Avg merge
5d 13h
Merged PRs (30d)
9

Description

SerializeRow impl for dicts asserts that dict has the same len as row serialization context.
This is a bug. In CQL, requests can use bind marker with the same name more than once.
This is a tricky case, because its handling depends on DB.
As far as I remember it works like this:

  • Old versions of Scylla, and all versions of Cassandra, will send a marker for each marker in query syntax, no matter if their names are the same or not
  • Current Scylla versions will deduplicate names, sending only a single marker with given name
  • Current Scylla has some config to return Cassandra-compatible behavior

When using Cassandra-compatible behavior, you may have multiple markers with the same name, and they will of course all use the same item from dict. In such case, correct dict size will be smaller than number of items in row serialization context!
This is impl from Rust Driver, using HashSet to protect from this problem:

macro_rules! impl_serialize_row_for_map {
    () => {
        fn serialize(
            &self,
            ctx: &RowSerializationContext<'_>,
            writer: &mut RowWriter,
        ) -> Result<(), SerializationError> {
            // Unfortunately, column names aren't guaranteed to be unique.
            // We need to track not-yet-used columns in order to see
            // whether some values were not used at the end, and report an error.
            let mut unused_columns: HashSet<&str> = self.keys().map(|k| k.as_ref()).collect();

            for col in ctx.columns.iter() {
                match self.get(col.name()) {
                    None => {
                        return Err(mk_typck_err::<Self>(
                            BuiltinTypeCheckErrorKind::ValueMissingForColumn {
                                name: col.name().to_owned(),
                            },
                        ));
                    }
                    Some(v) => {
                        $crate::_macro_internal::ser::row::serialize_column::<Self>(
                            v, col, writer,
                        )?;
                        let _ = unused_columns.remove(col.name());
                    }
                }
            }

            if !unused_columns.is_empty() {
                // Report the lexicographically first value for deterministic error messages
                let name = unused_columns.iter().min().unwrap();
                return Err(mk_typck_err::<Self>(
                    BuiltinTypeCheckErrorKind::NoColumnWithName {
                        name: name.to_string(),
                    },
                ));
            }

            Ok(())
        }

        #[inline]
        fn is_empty(&self) -> bool {
            Self::is_empty(self)
        }
    };
}

We need to do the same thing here.
It would be great to have a regression test for that, using either Cassandra, or Scylla with the compat config.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.