search

ossc-db / pg_hint_plan

Extension adding support for optimizer hints in PostgreSQL
Other
696 stars 103 forks source link

Next release for pg_hint_plans #128

Closed michaelpq closed 12 months ago

michaelpq commented 1 year ago

Hi all,

Now that some dust has set on the last release and that PostgreSQL 16 is entering in its stability period, now would be a good time for thinking about the next release. Here are my thoughts on the matter:

Thoughts?

michaelpq commented 1 year ago

So, here is an update from me here. As of today, I have:

At this stage, all the ground work has been done to allow a release. Unfortunately, we have a few issues hanging around. So I am going to look at that next.

@mikecaat Could you check the RPM generation for CentOS?

michaelpq commented 1 year ago

@rjuju Do you think that it could be possible to post the HTML docs somewhere once the release happens?

rjuju commented 1 year ago

On Tue, 29 Aug 2023, 13:29 Michael Paquier, @.***> wrote:

@rjuju https://github.com/rjuju Do you think that it could be able to post the HTML docs somewhere once the release happens?

it can yes, once someone takes care of https://github.com/ossc-db/pg_hint_plan/pull/125#issuecomment-1469227005

michaelpq commented 1 year ago

I have little idea about the webhook part, but I suspect that I don't have the permissions to do so either. @horiguti or @mikecaat, could you comment ?

mikecaat commented 1 year ago

Thanks for working on the next release.

@mikecaat Could you check the RPM generation for CentOS?

Yes, @SeinoYuki san will make RPM packages after all the RPM specs have been bumped.

I have little idea about the webhook part, but I suspect that I don't have the permissions to do so either. @horiguti or @mikecaat, could you comment ?

Unfortunately, only Admin can manage webhooks. I've checked Manage webhooks and deploy keys.

So, I'm going to a new webhook for the page of "Read the Docs". Please let me know the "Playload URL". I've checked Manual integration setup for GitHub and I will make the webhook with the following configuration.

If I have misunderstood, please let me know.

BTW, thanks for the work on polishing the documentation. The pg_hint_plan page of "Read the Docs" is readable and nice!

rjuju commented 1 year ago

@mikecaat it's probably not a good idea to publicly disclose the payload URL. Ideally you could create an account on readthedocs and let me know either your email or account name and I will be able to give you maintainer privileges on the project. That way you can check the payload URLs (there are 2 projects for english and japanese language), regenerate them if needed or any other configuration you think is appropriate.

rjuju commented 1 year ago

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

mikecaat commented 1 year ago

@mikecaat it's probably not a good idea to publicly disclose the payload URL. Ideally you could create an account on readthedocs and let me know either your email or account name and I will be able to give you maintainer privileges on the project. That way you can check the payload URLs (there are 2 projects for english and japanese language), regenerate them if needed or any other configuration you think is appropriate.

OK, I made my account (mikecaat).

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

Since I'm not familiar with "Read the Docs", it would be quicker if you handled it.

rjuju commented 1 year ago

Great, I just invited you as a maintainer on both projects! You can find the github webhooks in the admin/integration menu.

mikecaat commented 1 year ago

Thanks! I've added two webhooks for the projects. The configuration seems to be working now. You can see "Webhook configured correctly" in admin>integrations>exchange.

michaelpq commented 1 year ago

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

While looking at that yesterday, I have noticed that one of these is caused by sphinx that does not like the '$' character in a SQL block, because this character is not allowed by the SQL specification, but our example shows a normalized query.

michaelpq commented 1 year ago

@mikecaat The Japanese documentation needs a refresh in the range of 14~HEAD to be completely correct.

mikecaat commented 1 year ago

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

While looking at that yesterday, I have noticed that one of these is caused by sphinx that does not like the '$' character in a SQL block, because this character is not allowed by the SQL specification, but our example shows a normalized query.

It seems that this is a known limitation in the Sphinx community. (WARNING: Could not lex literal_block as "sql". Highlighting skipped.)

But, another warning about cross-reference can be fixed. I'll make a PR.

XXX/pg_hint_plan/docs/hint_table.md:140: WARNING: 'myst' cross-reference target not found: 'restrictions' [myst.xref_missing]
mikecaat commented 1 year ago

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

While looking at that yesterday, I have noticed that one of these is caused by sphinx that does not like the '$' character in a SQL block, because this character is not allowed by the SQL specification, but our example shows a normalized query.

It seems that this is a known limitation in the Sphinx community. (WARNING: Could not lex literal_block as "sql". Highlighting skipped.)

I found 'plpgsql' lexer alias and it works. I made a PR to fix warnings (#149). We can see all lexer aliases supported by Pygments which is used by Sphinx.

mikecaat commented 1 year ago

Oh, also please note that there are currently a few warnings about RTD configuration. I haven't worked on that yet as it requires adding a new configuration file in the repository and I wasn't sure that we would do the integration. I can work on that if you want, once the RTD integration is done.

While looking at that yesterday, I have noticed that one of these is caused by sphinx that does not like the '$' character in a SQL block, because this character is not allowed by the SQL specification, but our example shows a normalized query.

It seems that this is a known limitation in the Sphinx community. (WARNING: Could not lex literal_block as "sql". Highlighting skipped.)

I found 'plpgsql' lexer alias and it works. I made a PR to fix warnings (#149). We can see all lexer aliases supported by Pygments which is used by Sphinx.

Pushed it to master, PG16, PG15 and PG14 branches.

mikecaat commented 1 year ago

@mikecaat The Japanese documentation needs a refresh in the range of 14~HEAD to be completely correct.

I asked @yamatattsu san to take care of it. Please wait for his reply.

michaelpq commented 1 year ago

Okay, the last thing that I have on my list is #145 , where the use of the extended query protocol messes up with the hint execution. I am not sure that the suggested fix relying on the ExecutorRun() hook is completely right, TBH.

yamatattsu commented 1 year ago

@mikecaat The Japanese documentation needs a refresh in the range of 14~HEAD to be completely correct.

I asked @yamatattsu san to take care of it. Please wait for his reply.

@michaelpq @mikecaat Currently, we only have .po files for the Japanese document of HEAD. (It would be nice if the .po file could be used for PG14 and 15, but not sure.) So, I'll refresh the Japanese documentation of HEAD by the mid-Oct. I think that there is no problem even if there is no Japanese document for PG14 and 15.

rjuju commented 1 year ago

I only bootstrapped the translation files for HEAD, but I did document how to do generate the .pot and .po files in docs/README so that anyone can easily start a new translation in any language for any branch.

michaelpq commented 1 year ago

The release is basically ready at this point. I have finished everything I have in mind. So, if you have any comments, now would be the moment..

I am planning to push the tags and write the release notes with tarballs on Monday.

egashira-yusuke commented 12 months ago

@michaelpq Sorry this is just before the new release, but we have confirmed that the issues fixed in 23cabaf902b09b6b4ed14f1f920f3ec57d7fcb68 also occur in versions prior to PG15.

This issue seems to consist of two commits:

Fortunately, 23cabaf902b09b6b4ed14f1f920f3ec57d7fcb68 and b48c7ae99912d847e5d9f9922e567c9993305d7f seems to be able to apply cherry-pick to branches from PG11 to PG14. If the next release to the PG11 branch is the final one for PostgreSQL 11 support, can you have this backpatch included in that release? I will create a Pull-Request for this modification if necessary.

michaelpq commented 12 months ago

If the next release to the PG11 branch is the final one for PostgreSQL 11 support, can you have this backpatch included in that release?

I'll look at that on Monday, there are still a couple of days until PG16 is released ;)

michaelpq commented 12 months ago

@egashira-yusuke I have applied 23cabaf9 with all its related cleanup commits down to 11, with everything squashed in a single commit.

egashira-yusuke commented 12 months ago

@michaelpq Thank you for your prompt action.

michaelpq commented 12 months ago

The release has been published, with release notes, etc.

@mikecaat Could you publish some RPM packages?

mikecaat commented 12 months ago

Thanks!

@mikecaat Could you publish some RPM packages?

Yes, @SeinoYuki san plan to do so. Please wait a few days.

SeinoYuki commented 12 months ago

@michaelpq Sorry for the delay in checking. When I tried to create an RPM, I found a problem in the Makefile. [No rule to make target 'doc/*'] was output. I think docs is correct, not doc. This occurs in PG14, 15, 16, master.

TARSOURCES = Makefile *.c  *.h COPYRIGHT* \
    pg_hint_plan--*.sql \
    pg_hint_plan.control \
    doc/* expected/*.out sql/*.sql sql/maskout.sh \
    data/data.csv input/*.source output/*.source SPECS/*.spec

We are currently working on RPM generation for RHEL7 and will continue with RHEL8 and 9.

rjuju commented 12 months ago

Agreed, I missed that one when working on the documentation as I didn't try the rpm related rules. Thanks for fixing!

SeinoYuki commented 12 months ago

I am ready to release the rpm. Sorry it took so long for the first time.

@michaelpq I would like to fix Release/TAG with a version that incorporates #138 before the RPM release. Is there a problem if I update Release/TAG on my end?

Release RPM for PG16 is after the main PostgreSQL is released. Is that okay?

SeinoYuki commented 12 months ago

REL13_1_3_9, REL12_1_3_9, REL11_1_3_9 has been released.

michaelpq commented 12 months ago

Is there a problem if I update Release/TAG on my end?

As long as the release notes include the same contents, no issues from me.

SeinoYuki commented 12 months ago

Updated Release/TAG and released rpms of REL14_1_4_2, REL15_1_5_1. The PG16 version of the rpm will be released next Monday.

SeinoYuki commented 11 months ago

The PG16 version of the rpm will be released next Monday.

RPM for REL16_1_6_0 has been released.

However, RHEL7 RPMs will not be released. (The following announcement, RHEL7 RPMs is now EOL .) https://yum.postgresql.org/news/rhel7-postgresql-rpms-end-of-life/

michaelpq commented 10 months ago

I would like to fix Release/TAG with a version that incorporates https://github.com/ossc-db/pg_hint_plan/pull/138 before the RPM release.

Unfortunately it has taken me longer than that to incorporate #138 into the tree, but that's now done for HEAD, which will make this addition available with the first release of pg_hint_plan for Postgres 17. Perhaps that's for the best anyway to give more room for the change to mature a couple of months in the tree.