Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove empty UPDATE #13755

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Remove empty UPDATE #13755

merged 1 commit into from
Aug 23, 2022

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Aug 19, 2022

Description

Replace the UPDATE plan node with an empty VALUES node when the predicate is optimized to false.

Is this change a fix, improvement, new feature, refactoring, or other?

Fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Core query engine

How would you describe this change to a non-technical end user or system administrator?

Support queries like the following:

UPDATE table_name SET col1 = 'some_value' WHERE false;

Related issues, pull requests, and links

Fixes #12422

This PR takes inspiration from the commit 02be06c which was dealing with empty DELETE plan nodes.

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Core
* Replace the `UPDATE` plan node with an empty `VALUES` node when the predicate is optimized to `false`.

@cla-bot cla-bot bot added the cla-signed label Aug 19, 2022
@findinpath findinpath requested review from kokosing, martint and losipiuk and removed request for kokosing August 19, 2022 16:09
public class TestRemoveEmptyUpdate
extends BaseRuleTest
{
@Test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm inexperienced in the query planning area of Trino so I'd very much benefit of some hints regarding to improve/extend the tests for this fix.

@findinpath findinpath changed the title Remove empty UPDATE Replace empty UPDATE with VALUES node containing 0 value Aug 19, 2022
@findinpath findinpath changed the title Replace empty UPDATE with VALUES node containing 0 value Replace empty UPDATE Aug 19, 2022
@findinpath findinpath changed the title Replace empty UPDATE Remove empty UPDATE Aug 19, 2022
@hashhar hashhar requested review from kasiafi, losipiuk and kokosing and removed request for kokosing, losipiuk and kasiafi August 19, 2022 16:32
Replace the `UPDATE` plan node with an empty `VALUES`
node when the predicate is optimized to `false`.
@findinpath findinpath requested a review from findepi August 22, 2022 13:33
.addAll(RemoveEmptyDeleteRuleSet.rules())
.add(new RemoveEmptyUpdate())
Copy link
Member

Choose a reason for hiding this comment

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

for Delete we have a rule set (since we match empty delete with and without an exchange).
wonder whether this is some left over (or otherwise: why don't we need same for Update?)

Copy link
Member

Choose a reason for hiding this comment

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

followed up with #13794

@findepi
Copy link
Member

findepi commented Aug 23, 2022

# Core
* Replace the `UPDATE` plan node with an empty `VALUES` node when the predicate is optimized to `false`.

RN are meant to be understood by users. I will reformulate this -- #13723 (comment)

@findepi findepi merged commit 4b0408d into trinodb:master Aug 23, 2022
@findepi findepi mentioned this pull request Aug 23, 2022
@github-actions github-actions bot added this to the 394 milestone Aug 23, 2022
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.

UPDATE does not support WHERE false
3 participants