-
Notifications
You must be signed in to change notification settings - Fork 75
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
developments for postgresql v14 #104
Conversation
This patch handles the change of PostgreSQL core's 67a472d7. It corresponds the change of simple_prompt() interface and removes arbitrary restrictions on password length.
PostgreSQL core code has the same function name, but the implementation is not same. So, rename from appendStringInfoVA() to appendStringInfoVA_c().
This patch handles the changes of PostgreSQL core's 9626325d. It adds heuristic incoming-message-size limits in the server.
Fix TupleParserInit() and TupleParseDumpRecord()
This patch handles the following PostgreSQL core's changes. 1. The patch(1375422c) removes es_result_relations and es_num_result_relations to simplify things. 2. The patch(a04daa97) removes es_result_relation_info to improve maintainability.
To handle the PostgreSQL's patch(a3dc9260), which refactors option handling of REINDEX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear @mikecaat .
Thank you for responding to the PostgreSQL 14.
First, I used this PR in my environment(CentOS7/x86) to
build tests and regression tests from PG9.6 to the master branch,
and there were no problems.
Looking at this patch, I'm a little shocked that so far,
I haven't had any issues with e842847, d8ceb1a, and 969896c.
Thank you so much for catching these issues and correcting them.
And I checked each commit, but there are no comments other than 9b56a46.
I'll comment on 9b56a46 in line with the source code.
Best regards.
Moon.
lib/nbtree/nbtsort.c
Outdated
* create and initialize a spool structure | ||
*/ | ||
static BTSpool * | ||
_bt_spoolinit(Relation heap, Relation index, bool isunique, bool isdead) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why your opinion moved the "_bt_spoolinit" function outside?
In my think, it's more consistent with adding to "nbtsort-14.c" like what we've worked on it so far.
If you think it is good to add the "_bt_spoolinit" function to "nbtsort.c
", I think it would be good to move all "_bt_spoolinit" functions included to "nbtsort-XX.c" to "nbtsort.c".
What do you think of this opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it's better for a maintenance reason.
I puzzled that the header comments of "nbtsort-XX.c" say the identification is "src/backend/access/nbtree/nbtsort.c", but there is a fucntion which the PostgreSQL doesn't have in the version.
I though it's consistent if "nbtsort-XX.c" is just copy of postgresql's "src/backend/access/nbtree/nbtsort.c".
If you think it is good to add the "_bt_spoolinit" function to "nbtsort.c
", I think it would be good to move all "_bt_spoolinit" functions included to "nbtsort-XX.c" to "nbtsort.c".
OK, but why don't you after merging this commit? Because _bt_spoolinit was PostgreSQL's code, for example, 9.6 has that. After merging this, I'll update all nbtsort-XX.c to all stable PostgreSQL version's nbtsort.c and if there are other functions for only pg_bulkload, I'll move to "nbtsort.c" in pg_bulkload together.
BTW, should "nbtsort.c" in pg_bulkload be renamed though I didn't come up with any good name? This makes maintainers confused, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. For that reason, I agree with your opinion.
OK, but why don't you after merging this commit?
Also, I understand moving the "_bt_spoolinit" funciton and
anyone from "nbtsort-xx.c" to "nbtsort.c" after committing the current PR.
BTW, should "nbtsort.c" in pg_bulkload be renamed though I didn't come up with any good name? This makes maintainers confused, doesn't it?
I think that "nbtsort-common.c" might be good
because this file includes a common functions.
Of course, this is my personal opinion, so I think it's fine as it is. :)
Best regards.
Moon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, OK
Also, I understand moving the "_bt_spoolinit" funciton and
anyone from "nbtsort-xx.c" to "nbtsort.c" after committing the current PR.
I made #106.
I think that "nbtsort-common.c" might be good
because this file includes a common functions.
Of course, this is my personal opinion, so I think it's fine as it is. :)
It looks good to me. I changed the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the file name and creating a new issue!
I think there's nothing problme with committing this PR.
That is, it looks good to me.
If there is no other opinion, I'll commit this PR around 15:00(JST) on June 11. Best regards. |
* Fix prompt_for_password() against PostgreSQL v14 Beta1 This patch handles the change of PostgreSQL core's 67a472d7. It corresponds the change of simple_prompt() interface and removes arbitrary restrictions on password length. * Rename from appendStringInfoVA() to appendStringInfoVA_c() PostgreSQL core code has the same function name, but the implementation is not same. So, rename from appendStringInfoVA() to appendStringInfoVA_c(). * Fix the wrong return type of *WriterInsertProc * Reuse the RelationSetNewRelfilenode's definition for v14 beta1 * Fix pg_getmessage arguments against PostgreSQL v14 Beta1 This patch handles the changes of PostgreSQL core's 9626325d. It adds heuristic incoming-message-size limits in the server. * Fix the wrong arguments types of functions Fix TupleParserInit() and TupleParseDumpRecord() * Fix EState struct againt PostgreSQL v14 Beta1 This patch handles the following PostgreSQL core's changes. 1. The patch(1375422c) removes es_result_relations and es_num_result_relations to simplify things. 2. The patch(a04daa97) removes es_result_relation_info to improve maintainability. * Fix argments of reindex_index() againt PostgreSQL v14 Beta1 To handle the PostgreSQL's patch(a3dc9260), which refactors option handling of REINDEX. * Update nbtsort.c against PostgreSQL v14 Beta1 * Update test codes for PostgreSQL v14 Beta1 * Update the github action workflow for PostgreSQL14 Beta1
TODO