Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JAVA-3084: Add integration test coverage for ExtraTypeCodecs #1679

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

hhughes
Copy link
Contributor

@hhughes hhughes commented Jul 12, 2023

No description provided.

@hhughes hhughes force-pushed the JAVA-3084 branch 2 times, most recently from 01bf391 to e74025d Compare July 12, 2023 15:31
.get(field.name, codec))
.isEqualTo(value);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not essential, but opening a new codec for each codec we want to test is probably more than we need to do here. We can apply codecs at read time directly to a row and if we use prepared statements we can also supply a codec when we generate a BoundStatement from a PreparedStatement. Using that approach we can do everything through a single session:

  private <T> void insertAndRead(TableField field, T value, TypeCodec<T> codec) {
    CqlSession session = SESSION_RULE.session();
    PreparedStatement psi =
        session.prepare(
            String.format("INSERT INTO extra_type_codecs_it (key, %s) VALUES (?, ?)", field.name));
    // write value under new key using provided codec
    UUID key = UUID.randomUUID();
    session.execute(psi.boundStatementBuilder().setUuid(0, key).set(1, value, codec).build());

    PreparedStatement pss =
        session.prepare(
            String.format("SELECT %s FROM extra_type_codecs_it WHERE key = ?", field.name));
    // read value using provided codec
    assertThat(
            session
                .execute(pss.boundStatementBuilder().setUuid(0, key).build())
                .one()
                .get(field.name, codec))
        .isEqualTo(value);
  }

The above passes for me locally against C* 4.0.0.

This is not a particularly efficient way to use prepared statements, but it's probably somewhat better than having to open and close a session for each type. As I mentioned above it's not essential (although it probably is a bit faster); there's nothing wrong with what you have now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Updated insertAndRead to use single session with bound statements per your suggestion. If the tests pass I'll merge this one :)

Copy link
Contributor

@absurdfarce absurdfarce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the overall approach taken here; this is a solid implementation and a clever way to handle the various types of codecs involved. Left a comment on how to do this without having to open a session for each codec but it's not strictly required.

Nice work!

@hhughes
Copy link
Contributor Author

hhughes commented Jul 20, 2023

Test failures unrelated - merging

@hhughes hhughes merged commit ec93ef9 into 4.x Jul 20, 2023
3 checks passed
@hhughes hhughes deleted the JAVA-3084 branch July 26, 2023 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants