search

substrait-io / substrait-java

Apache License 2.0
72 stars 70 forks source link

fix: issue-134 to choose cast failure condition - needs calcite 1.34 #236

Closed mbwhite closed 3 months ago

mbwhite commented 4 months ago

Resolves #134

To get the SqlKind.SAFE_CAST - that needs Calcite 1.35 (implemented here) However 1.34 also had a change to the external API

I've included both in the PR as they are very small changes; anything much larger though I'd prefer to do as a separate change.

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

mbwhite commented 4 months ago

Just looking at the test failures now; though at present I can quite see how the expected output is being created.

vbarua commented 4 months ago

Could you add a test to verify that this conversion works end-to-end. The ExpressionConvertabilityTest file could be a good place for this.

The assertRuleRoundTrip(Rel ...) function in PlanTestBase is an easy way to perform this end-to-end check.

mbwhite commented 4 months ago

@vbarua yes _ will do so; just got the existing tests sorted out. Accidentally end up using Java 11 that messed a few things up... back to Java 17 and all is well.

vbarua commented 3 months ago

@mbwhite I rebased your branch to deal with merge conflicts, and made one of your tests stronger to verify full Substrait end-to-end. It looks like we're missing a piece.

mbwhite commented 3 months ago

@vbarua took longer than I would have liked but found the Calcite deprecated function that was being used.