Skip to content

Simplify array functions#18094

Merged
dain merged 4 commits intotrinodb:masterfrom
dain:simplify-array-functions
Jun 30, 2023
Merged

Simplify array functions#18094
dain merged 4 commits intotrinodb:masterfrom
dain:simplify-array-functions

Conversation

@dain
Copy link
Copy Markdown
Member

@dain dain commented Jun 29, 2023

Description

  • Add BLOCK_BUILDER return convention that writes the result directly to a BlockBuilder
  • Add READ_VALUE operator that can read a value from any argument convention to any return convention
  • Using the above two techniques, most array functions with an implementation per stack type are converted to a simple single generic implementation
  • Migrate more array functions from PageBuilder cache to use the simpler BufferedArrayValueBuilder

Release notes

(X) Release notes are required, with the following suggested text:

# Section
* Add `BLOCK_BUILDER` return convention that writes the function result directly to a `BlockBuilder`. ({issue}`18094`)
* Add `READ_VALUE` operator that can read a value from any argument convention to any return convention.  ({issue}`18094`)

@dain dain requested a review from electrum June 29, 2023 20:38
@cla-bot cla-bot bot added the cla-signed label Jun 29, 2023
@electrum
Copy link
Copy Markdown
Member

Typo "BlockBuild" in first commit description

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.

We normally use AssertionError, but this seems better. We should probably switch to it for static initializers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I copied this from somewhere else, but I commonly see RuntimeException for these... we should pick one

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.

You can inline this now that there is only one usage. Also, consider converting to checkCondition

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is actually referenced from the ExpressionInterpreter

@dain dain force-pushed the simplify-array-functions branch 3 times, most recently from 0b8c877 to 81c7b80 Compare June 30, 2023 04:00
@dain dain force-pushed the simplify-array-functions branch from 81c7b80 to 7b6a8c3 Compare June 30, 2023 05:21
@dain dain merged commit 0498029 into trinodb:master Jun 30, 2023
@dain dain deleted the simplify-array-functions branch June 30, 2023 18:50
@github-actions github-actions bot added this to the 421 milestone Jun 30, 2023
@martint martint mentioned this pull request Jul 10, 2023
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants