Skip to content

Rework prepared queries #677

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jun 19, 2020
Merged

Rework prepared queries #677

merged 11 commits into from
Jun 19, 2020

Conversation

vietj
Copy link
Member

@vietj vietj commented Jun 5, 2020 •
edited
Loading

@vietj vietj mentioned this pull request Jun 5, 2020
Closed
@vietj vietj force-pushed the rework-prepared-queries branch from 799881d to 7d1fcdf Compare June 5, 2020 16:43
@BillyYccc
Copy link
Member

Does this imply the statement auto-closing after one-shot preparedQuery is handled by the codec layer?

@vietj
Copy link
Member Author

vietj commented Jun 6, 2020 •
edited
Loading

@BillyYccc yes it is implied when there is no caching

@vietj
Copy link
Member Author

vietj commented Jun 6, 2020

@BillyYccc @aguibert I would like to know if the above is fine and then we can start finishing this PR together and after back-port it to 3.9

@vietj vietj force-pushed the rework-prepared-queries branch from 7d1fcdf to f3f3056 Compare June 15, 2020 15:11
@vietj
Copy link
Member Author

vietj commented Jun 15, 2020

I updated this PR with an intermediate change that does not require modification of current codecs and is actually more satisfying. I'll update the proposal issue to be in sync with this change.

@vietj
Copy link
Member Author

vietj commented Jun 15, 2020

so this should not involve contributions from @aguibert and @BillyYccc

@vietj vietj force-pushed the rework-prepared-queries branch from eaa976a to 612e3d2 Compare June 15, 2020 20:41
@vietj
Copy link
Member Author

vietj commented Jun 15, 2020

Updated with prepared query caching simplification and corrections.

@vietj vietj marked this pull request as ready for review June 16, 2020 11:09
@vietj
Copy link
Member Author

vietj commented Jun 16, 2020 •
edited
Loading

I'm moving this PR to review so people can comment and make feedback. I'll review it myself also as there might be simplifications to do.

@vietj
Copy link
Member Author

vietj commented Jun 16, 2020

To ease the review I will push the first commit that refactor the extended query commands in a single command to master as it is not related to the PR and the goal was to simplify and help this change

@vietj vietj force-pushed the rework-prepared-queries branch 2 times, most recently from 9b0c9d6 to bcdc979 Compare June 16, 2020 15:26
@vietj
Copy link
Member Author

vietj commented Jun 16, 2020

this is ready for review

@vietj vietj requested review from BillyYccc and aguibert June 16, 2020 16:29
@vietj vietj force-pushed the rework-prepared-queries branch from bcdc979 to 098da73 Compare June 18, 2020 10:17
@vietj
Copy link
Member Author

vietj commented Jun 18, 2020

Pushed a fix for the incorrect pipelining count that was found by a case provided by @BillyYccc

@vietj vietj force-pushed the rework-prepared-queries branch from 098da73 to b55f690 Compare June 18, 2020 10:20
@vietj vietj force-pushed the rework-prepared-queries branch from b55f690 to 1141abe Compare June 18, 2020 12:41
@vietj vietj mentioned this pull request Jun 18, 2020
Merged
BillyYccc
BillyYccc reviewed Jun 18, 2020
View reviewed changes
Copy link
Member

@BillyYccc BillyYccc left a comment

Choose a reason for hiding this comment

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

Looks overall to me now, I have added some commits and we need to backport them to 3.9 as well

@vietj
Copy link
Member Author

vietj commented Jun 19, 2020

I back ported the commits @BillyYccc

@vietj vietj force-pushed the rework-prepared-queries branch from 60a7654 to 87b490f Compare June 19, 2020 09:47
@vietj
Copy link
Member Author

vietj commented Jun 19, 2020

@BillyYccc added support for SQL cache filtering, I think now we are good and we can do a final pass on the review

BillyYccc
BillyYccc approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@BillyYccc BillyYccc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @vietj for the effort on this!

@vietj
Copy link
Member Author

vietj commented Jun 19, 2020

it was a collaboration as usual @BillyYccc :-)

@vietj vietj merged commit b83783b into master Jun 19, 2020
@vietj vietj mentioned this pull request Jun 22, 2020
Closed
@vietj vietj deleted the rework-prepared-queries branch July 2, 2020 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BillyYccc BillyYccc BillyYccc approved these changes

@aguibert aguibert Awaiting requested review from aguibert

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vietj @BillyYccc