Skip to content

TestBase.protoExpressionConverter is always built with a null ProtoRelConverter #1072

Description

@nielspardon

In core/src/test/java/io/substrait/TestBase.java, protoExpressionConverter is a field initializer (lines 36-41) that reads protoRelConverter, but protoRelConverter is declared without an initializer (line 32) and only assigned in the constructor body (line 50).

Java runs field initializers before the constructor body, so protoExpressionConverter is always constructed with relConverter == null. ProtoExpressionConverter stores it unchecked (only rootType is requireNonNull'd), so nothing fails at construction time — the null just sits there.

Consequence: TestBase.verifyRoundTrip(Expression) (line 61) NPEs for any expression that needs the rel converter, i.e. anything containing a subquery. TestBase is extended by ~44 test classes, so this silently narrows what the shared helper can cover.

Fix: assign protoExpressionConverter in the constructor body after protoRelConverter, and add a round-trip test over a subquery-bearing expression to pin it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcorePull requests that update java code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions