Skip to content

Fix the issue where query with DistinctLimit failed due to turning on delete as join optimization#22106

Merged
tdcmeehan merged 1 commit intoprestodb:masterfrom
wypb:fix_delete_as_join
Mar 8, 2024
Merged

Fix the issue where query with DistinctLimit failed due to turning on delete as join optimization#22106
tdcmeehan merged 1 commit intoprestodb:masterfrom
wypb:fix_delete_as_join

Conversation

@wypb
Copy link
Contributor

@wypb wypb commented Mar 7, 2024

Description

com.facebook.presto.iceberg.optimizer.IcebergEqualityDeleteAsJoin.DeleteAsJoinRewriter#visitTableScan returns FilterNode's outputVariables will contain the $data_sequence_number column from the table on the right side of ConnectorJoinNode, An exception occurs when the parent of the returned FilterNode is a DistinctLimitNode.

In fact, the $data_sequence_number column is only used in the FilterNode node, and its parent node is not used. This PR adds a ProjectNode on FilterNode that filters out the $data_sequence_number column.

Fixes #22105

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Iceberg Changes
* Fix the issue where query with DistinctLimit failed due to turning on delete as join optimization.

CC: @tdcmeehan @yingsu00 @jasonf20

@wypb wypb requested a review from a team as a code owner March 7, 2024 05:09
@wypb wypb requested a review from presto-oss March 7, 2024 05:09
@wypb wypb changed the title Fixed the issue where query with DistinctLimit failed due to turning … Fixed the issue where query with DistinctLimit failed due to turning on delete as join optimization Mar 7, 2024
@wypb wypb force-pushed the fix_delete_as_join branch 2 times, most recently from 3386321 to 1d4235e Compare March 7, 2024 07:05
Copy link
Collaborator

@jasonf20 jasonf20 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I think it's a little cleaner to use !IcebergMetadataColumn.isMetadataColumnId here

Copy link
Contributor Author

@wypb wypb Mar 7, 2024

Choose a reason for hiding this comment

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

!IcebergMetadataColumn.isMetadataColumnId  doesn't work here, Because the value of variableReferenceExpression#name is similar to $data_sequence_number_7.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah you only have the variable references. I guess it's fine like this 👍

@tdcmeehan tdcmeehan self-assigned this Mar 7, 2024
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

LGTM. Please reword the commit message. Perhaps

Fix equality delete with DistinctLimit

CONTEXT

@wypb wypb force-pushed the fix_delete_as_join branch from 1d4235e to 85f2820 Compare March 8, 2024 01:05
@wypb wypb changed the title Fixed the issue where query with DistinctLimit failed due to turning on delete as join optimization Fix the issue where query with DistinctLimit failed due to turning on delete as join optimization Mar 8, 2024
@tdcmeehan tdcmeehan merged commit 67eb17d into prestodb:master Mar 8, 2024
@wypb wypb deleted the fix_delete_as_join branch March 8, 2024 14:20
@wanglinsong wanglinsong mentioned this pull request May 1, 2024
48 tasks
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.

[Iceberg] Stream-level local properties contain columns not present in node's output

4 participants