Skip to content

Fix: Re-initialize sort key serializer for every new input added#25015

Merged
facebook-github-bot merged 1 commit into
prestodb:masterfrom
emilysun201309:export-D73903883
May 2, 2025
Merged

Fix: Re-initialize sort key serializer for every new input added#25015
facebook-github-bot merged 1 commit into
prestodb:masterfrom
emilysun201309:export-D73903883

Conversation

@emilysun201309

@emilysun201309 emilysun201309 commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

Summary:
The sort key serializer binarySortableSerializer_ needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of binarySortableSerializer_ to addInput() so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883

== RELEASE NOTES ==
Prestissimo (Native Execution) Changes
* Fix issue with PartitionAndSerialize re-using only keys from the first batch of data.

@emilysun201309 emilysun201309 requested a review from a team as a code owner April 30, 2025 07:01
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

emilysun201309 added a commit to emilysun201309/presto that referenced this pull request Apr 30, 2025
…stodb#25015)

Summary:

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

emilysun201309 added a commit to emilysun201309/presto that referenced this pull request Apr 30, 2025
…stodb#25015)

Summary:

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

emilysun201309 added a commit to emilysun201309/presto that referenced this pull request Apr 30, 2025
…stodb#25015)

Summary:

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

emilysun201309 added a commit to emilysun201309/presto that referenced this pull request Apr 30, 2025
…stodb#25015)

Summary:
Pull Request resolved: prestodb#25015

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
emilysun201309 added a commit to emilysun201309/presto that referenced this pull request Apr 30, 2025
…stodb#25015)

Summary:

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

emilysun201309 added a commit to emilysun201309/presto that referenced this pull request May 1, 2025
…stodb#25015)

Summary:

The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

@facebook-github-bot

Copy link
Copy Markdown
Collaborator

@emilysun201309 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary:
The sort key serializer `binarySortableSerializer_` needs to be initialized for every input batch. Otherwise, we end up re-using only keys from the first batch of data.
This diffs move the initialization of `binarySortableSerializer_` to `addInput()` so that the above condition is satisfied.

Reviewed By: xiaoxmeng

Differential Revision: D73903883
@facebook-github-bot

Copy link
Copy Markdown
Collaborator

This pull request was exported from Phabricator. Differential Revision: D73903883

@facebook-github-bot facebook-github-bot merged commit dde5c16 into prestodb:master May 2, 2025
@ZacBlanco ZacBlanco mentioned this pull request May 29, 2025
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants