scylladb / scylladb/scylla-cdc-java

API improvement: Add Optional-returning variants of lookup methods to avoid IllegalArgumentException

Open
#164 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
32
Forks
23
Avg merge
2d 6h
Merged PRs (30d)
3

Description

Summary

The current API in ChangeSchema and RawChange throws IllegalArgumentException when looking up columns that don't exist. This makes it difficult to safely check for optional CDC columns like cdc$deleted_elements_<column> which only exist for non-frozen collection types.

Problem

When using methods like:

  • ChangeSchema.getColumnDefinition(String columnName)
  • ChangeSchema.getDeletedColumnDefinition(String columnName)
  • ChangeSchema.getDeletedElementsColumnDefinition(String columnName)
  • RawChange.getCell(String columnName) (which internally calls getColumnDefinition)

These methods throw IllegalArgumentException if the column is not present in the schema:

// From ChangeSchema.class (decompiled)
public ColumnDefinition getColumnDefinition(String columnName) {
    return columnDefinitions.stream()
        .filter(cd -> cd.getColumnName().equals(columnName))
        .findFirst()
        .orElseThrow(() -> new IllegalArgumentException(
            "Column name " + columnName + " is not present in change schema."));
}

This is problematic when checking for CDC columns that may or may not exist depending on the column type. For example, cdc$deleted_elements_<column> only exists for non-frozen LIST, SET, and MAP columns - not for primitive types.

Current Workaround

Users must either:

  1. Check the column type before attempting to access the CDC column (requires knowledge of internal CDC schema details)
  2. Wrap calls in try-catch blocks (poor practice for flow control)

Proposed Solution

Add Optional-returning variants of the lookup methods:

public interface ChangeSchema {
    // Existing (keep for backwards compatibility)
    ColumnDefinition getColumnDefinition(String columnName);
    
    // New - returns Optional.empty() instead of throwing
    Optional<ColumnDefinition> findColumnDefinition(String columnName);
    Optional<ColumnDefinition> findDeletedColumnDefinition(String columnName);
    Optional<ColumnDefinition> findDeletedElementsColumnDefinition(String columnName);
    
    // Or alternatively, add a simple existence check
    boolean hasColumn(String columnName);
    boolean hasDeletedColumn(String columnName);
    boolean hasDeletedElementsColumn(String columnName);
}

public interface RawChange {
    // New - returns null or Optional.empty() instead of throwing
    Optional<Cell> findCell(String columnName);
}

Use Case Example

// Current (requires type checking or try-catch)
ChangeSchema.CqlType baseType = cdef.getBaseTableDataType().getCqlType();
if (baseType == CqlType.LIST || baseType == CqlType.SET || baseType == CqlType.MAP) {
    Cell deletedElementsCell = change.getCell("cdc$deleted_elements_" + columnName);
    // ...
}

// With proposed API
change.findCell("cdc$deleted_elements_" + columnName).ifPresent(cell -> {
    // ...
});

// Or with hasColumn
if (change.getSchema().hasColumn("cdc$deleted_elements_" + columnName)) {
    Cell cell = change.getCell("cdc$deleted_elements_" + columnName);
    // ...
}

Benefits

  1. Follows Java best practices - use exceptions for exceptional conditions, not flow control
  2. More ergonomic API for optional columns
  3. No breaking changes if added as new methods alongside existing ones
  4. Better performance (avoids exception throwing overhead in normal flow)

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.

Research direction

Start with the ChangeSchema and RawChange interfaces and inspect the existing lookup methods named in the issue, along with their callers. Determine which Optional-returning variants are appropriate, then add coverage showing absent columns can be checked without exceptions and existing methods remain compatible.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.