Skip to content

Handle redirection for Insert, Delete and Update#8683

Merged
phd3 merged 2 commits intotrinodb:masterfrom
phd3:redirection-handle-modifications
Aug 17, 2021
Merged

Handle redirection for Insert, Delete and Update#8683
phd3 merged 2 commits intotrinodb:masterfrom
phd3:redirection-handle-modifications

Conversation

@phd3
Copy link
Copy Markdown
Member

@phd3 phd3 commented Jul 27, 2021 •
edited
Loading

The current behavior silently ignores redirection, which is confusing if the user executes INSERT, SELECT sequence on source table.

@cla-bot cla-bot bot added the cla-signed label Jul 27, 2021
@phd3 phd3 requested review from electrum and findepi July 27, 2021 21:56
@findepi findepi requested a review from sopel39 July 28, 2021 08:10
@findepi
Copy link
Copy Markdown
Member

findepi commented Jul 29, 2021

@phd3 please see CI

@phd3
Copy link
Copy Markdown
Member Author

phd3 commented Jul 29, 2021

@findepi failure looks unrelated to me. The MockConnector change in this PR are around deletion, which shouldn't impact the query in failing tests. I couldn't reproduce this issue in 150 runs.

2021-07-27T22:45:39.2515616Z [ERROR] Tests run: 2453, Failures: 1, Errors: 0, Skipped: 76, Time elapsed: 2,765.593 s <<< FAILURE! - in TestSuite
2021-07-27T22:45:39.2517153Z [ERROR] testInterruptApplyFilter(io.trino.execution.TestQueryTracker)  Time elapsed: 6.108 s  <<< FAILURE!
2021-07-27T22:45:39.2520320Z org.testng.internal.thread.ThreadTimeoutException: Method io.trino.execution.TestQueryTracker.testInterruptApplyFilter() didn't finish within the time-out 5000

@findepi
Copy link
Copy Markdown
Member

findepi commented Jul 30, 2021

@phd3 thanks for looking into the failure.
Do we have an issue about that? can you please 'ping' the issue or link it here?

@phd3
Copy link
Copy Markdown
Member Author

phd3 commented Jul 31, 2021

Flaky #8432 observed in https://github.com/trinodb/trino/runs/3176409502?check_suite_focus=true

electrum
electrum approved these changes Aug 13, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

Nice work!

@phd3 phd3 force-pushed the redirection-handle-modifications branch from 504be9d to c21c502 Compare August 14, 2021 14:52
@phd3 phd3 merged commit e2577e0 into trinodb:master Aug 17, 2021
@phd3 phd3 mentioned this pull request Aug 17, 2021
Closed
10 tasks
@phd3
Copy link
Copy Markdown
Member Author

phd3 commented Aug 17, 2021

Merged #8683.

@phd3 phd3 added this to the 361 milestone Aug 17, 2021
@phd3 phd3 mentioned this pull request Nov 11, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@findepi findepi findepi left review comments

@electrum electrum electrum approved these changes

@sopel39 sopel39 Awaiting requested review from sopel39

+1 more reviewer

@ssheikin ssheikin ssheikin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Milestone

361

Development

Successfully merging this pull request may close these issues.

4 participants

@phd3 @findepi @electrum @ssheikin