Skip to content

feat(isthmus): add WindowRelFunctionConverter - #234

Merged
vbarua merged 1 commit into
substrait-io:mainfrom
bvolpato:window-rel-function-converter
Feb 24, 2024
Merged

feat(isthmus): add WindowRelFunctionConverter#234
vbarua merged 1 commit into
substrait-io:mainfrom
bvolpato:window-rel-function-converter

Conversation

@bvolpato

@bvolpato bvolpato commented Feb 23, 2024

Copy link
Copy Markdown
Member

We recently added POJOs for ConsistentPartitionWindowRel and WindowRelFunction. #231

The next step is starting to get isthmus support for it, so I went ahead and created WindowRelFunctionConverter so we can start using it to properly bind our window function calls.

It is heavily based on WindowFunctionConverter (although both the input and output changes - input from RexOver to RexWinAggCall, and output from WindowFunctionInvocation to WindowRelFunctionInvocation), so I extracted some converters to top-level classes to improve reusability.

@vbarua vbarua left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding this in!

Extracting the SortFieldConverter and WindowBoundConverter should be useful for anyone trying to implement or extend the function converters, and the WindowRelFunctionConverter should help anyone experimenting with ConsistentPartitionWindowRel.

public class SortFieldConverter {

/** Converts a {@link RexFieldCollation} to a {@link Expression.SortField}. */
public static Expression.SortField toSortField(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✨ Good call extracting this. It should be useful to any downstream uses who wish to implement their own conversion.

public class WindowBoundConverter {

/** Converts a {@link RexWindowBound} to a {@link WindowBound}. */
public static WindowBound toWindowBound(RexWindowBound rexWindowBound) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

✨ Good call extracting this. It should be useful to any downstream uses who wish to implement their own conversion.

@vbarua
vbarua merged commit a5e1a21 into substrait-io:main Feb 24, 2024
ajegou pushed a commit to ajegou/substrait-java that referenced this pull request Mar 29, 2024
Extracted out SortFieldConverter and WindowBoundConverter for easier reuse
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.

3 participants