Skip to content

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

@dkropachev

Description

@dkropachev

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions