JAVA-3057 Allow decoding a UDT that has more fields than expected

[akhaku]

[lukasz-antoniak]

I've had hard time trying to replicate this issue. Attempts to ignore schema updates on control connection did not replicate the problem. In fact, DefaultCodecRegistry will always attempt to create new codec if it does not find corresponding one in the cache (source). Cache key takes into account UDT name, but also names and types of fields.

This triggered my attention to the way application attempts to read UDT from row. Leveraging row.getUdtValue(...) will again register new codec (source). However, trying to read the data with given UdtCodec will indeed expose the issue. Since there is a public API method to read the column using certain codec, I think that this issue is valid.

Below is the integration test that exposes the problem. Shall we add it to the PR?

/**
 * @jira_ticket JAVA-3057
 */
@Test
public void should_decoding_udt_be_backward_compatible() {
    CqlSession session = sessionRule.session();
    session.execute("CREATE TYPE test_type_1 (a text, b int)");
    session.execute("CREATE TABLE test_table_1 (e int primary key, f frozen<test_type_1>)");
    // insert a row using version 1 of the UDT schema
    session.execute("INSERT INTO test_table_1(e, f) VALUES(1, {a: 'a', b: 1})");
    UserDefinedType udt = session
            .getMetadata()
            .getKeyspace(sessionRule.keyspace())
            .flatMap(ks -> ks.getUserDefinedType("test_type_1"))
            .orElseThrow(IllegalStateException::new);
    TypeCodec<?> oldCodec = session.getContext().getCodecRegistry().codecFor(udt);
    // update UDT schema
    session.execute("ALTER TYPE test_type_1 add i text");
    // insert a row using version 2 of the UDT schema
    session.execute("INSERT INTO test_table_1(e, f) VALUES(2, {a: 'b', b: 2, i: 'b'})");
    Row row = session.execute("SELECT f FROM test_table_1 WHERE e = ?", 2).one();
    // Try to read new row with old codec. Using row.getUdtValue() would not cause any issues,
    // because new codec will be automatically registered (using all 3 attributes).
    // If application leverages generic row.get(String, Codec) method, data reading with old codec should
    // be backward-compatible.
    UdtValue value = (UdtValue) row.get("f", oldCodec);
    assertThat(value.getString("a")).isEqualTo("b");
    assertThat(value.getInt("b")).isEqualTo(2);
    assertThatThrownBy(() -> value.getString("i")).hasMessage("i is not a field in this UDT");
}

[akhaku]

Awesome, thank you for doing that work for me Lukasz! I added it in a new integration test since I couldn't find another existing one that fit. Also rebased.

The case we ran into this issue had to do with the object mapper since it holds on to schema somewhere, and uses that to generate the cql queries for example. But this test is great too!

[absurdfarce]

I'm a bit reluctant to remove the safeguard of decoding a UDT whose set of fields is different from what we expect. If we have a mismatch there it seems like we can't say anything with certainty about what's actually in the UDT in that case.

It seems like the best case we can do from a client perspective in this case is something like the following:

1. Try to read the UDT
2. If the read fails because of the indicated exception try a schema refresh (since def on server might have changed since schema was originally loaded)
3. Try to read again with new UDT def. If this fails again you've just got bad data and there's nothing you can do about it.

Am I missing something here?

[akhaku]

That will work in some cases, but a few things that make me feel like the current approach is better:

1. I'm hesitant to do a schema refresh in the request path of the driver, it will cause that request to look latent from the caller's perspective. The closest existing parallel I can think of is preparing a statement on an UNPREPARED response but that particular case seems warranted.
2. A refresh in that code path isn't enough, the codec is likely be cached in the CachingCodecRegistry and there's (currently) no way to invalidate that. For example when using the object mapper it validates the schema against your POJOs by default and ends up caching the codec at init time. You can't even re-register your UdtCodec on an error since it hits the collides check and doesn't get re-registered - changing that will be a fair bit more invasive.

whose set of fields is different from what we expect.

3. The only changes you can make to a UDT are to add fields, and this code path deals specifically with having more fields than we expect. Additionally, given Cassandra is byte buffers all over the place, there's nothing stopping a user from trying to read bytes with the wrong codec except for schema verification, and that's what protects us from the general "wrong UDT definition" case. I think that means we can assume users only make "sane" changes to a UDT while a client app is running (i.e. UDT field additions) and so the "right" thing to do is ignore fields that were added that we don't recognize, to make it easier to roll out a UDT change across the writer apps and reader apps.

Let me know if you want to chat about this over a call, it took me a couple hours to page in enough context about this to be able to respond to you, would be nice to resolve this before it gets paged out again :)