ossc-db / pg_hint_plan

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

unable to update extension #60

Closed 20hyun closed 1 year ago

20hyun commented 3 years ago

Hi, In a797bcf944d7f5c17d6d26cdaa2c57ef50b73a81 , It seems line break leads to syntax error when updating extension.

/* pg_hint_plan/pg_hint_plan--1.3.6--1.3.7.sql */

-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "ALTER EXTENSION pg_dbms_stats UPDATE TO '1.3.7'" to load this file.
\quit
postgres=# CREATE EXTENSION pg_hint_plan VERSION "1.3.6";
CREATE EXTENSION
postgres=# ALTER EXTENSION pg_hint_plan UPDATE TO "1.3.7";
ERROR:  syntax error at or near "\"
postgres=# \dx pg_hint_plan 
           List of installed extensions
     Name     | Version |  Schema   | Description 
--------------+---------+-----------+-------------
 pg_hint_plan | 1.3.6   | hint_plan | 
(1 row)

By editing complain back to one line made work as normal

/* pg_hint_plan/pg_hint_plan--1.3.6--1.3.7.sql */

-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "ALTER EXTENSION pg_dbms_stats UPDATE TO '1.3.7'" to load this file. \quit
postgres=# ALTER EXTENSION pg_hint_plan UPDATE TO "1.3.7";
ALTER EXTENSION
postgres=# \dx pg_hint_plan 
           List of installed extensions
     Name     | Version |  Schema   | Description 
--------------+---------+-----------+-------------
 pg_hint_plan | 1.3.7   | hint_plan | 
(1 row)
michaelpq commented 1 year ago

I think that this points to a deeper problem than just what is reported here: the maintenance of the module is simply broken in my opinion. Postgres has changed the way we handle upgrades of modules by using as a base SQL file a past version, then we create an incremental SQL file up to the newest version. pg_hint_plan does the opposite: we create a new base script and have an intermediate file to a new major version, however it is basically impossible to have regression tests where we would check an upgrade from a older to a newer version.

PG13, PG14 and master don't have an issue currently, because they have only one SQL file, but PG12, PG11 and PG10 don't follow this reasoning. What we need to do is to:

michaelpq commented 1 year ago

Adding @horiguti @MasahikoSawada and @rjuju in CC for input. I can fix that by myself.. Still I'll wait a bit.

rjuju commented 1 year ago

It's worth noting that the new approach taken upstream is only valid for pg10+. Since we don't support EOL major versions I agree that we should use the same approach and keep the initial full SQL file and only provide upgrade scripts from there, and keep doing that for newer versions.

michaelpq commented 1 year ago

It's worth noting that the new approach taken upstream is only valid for pg10+. Since we don't support EOL major versions I agree that we should use the same approach and keep the initial full SQL file and only provide upgrade scripts from there, and keep doing that for newer versions.

What should we do for versions between each branch? PG14/master are on 1.4 for example. Do you think that we should also have upgrade paths from 1.3.X to 1.4 or even future versions? I am not sure what this implies in terms of complexity and binary compatibility, TBH (for example of function arguments have changed, we need to keep track of the function at run-time like pg_stat_statements, or if results are changed we can adapt the returned tuple dynamically).

rjuju commented 1 year ago

I think we could provide (empty) extension upgrade scripts for all the latest version for each back branches. The extension doesn't contain much (only the table to store the hints per normalized query), and I don't think we will ever have very complex extension scripts so the extra maintenance burden vs ease of major version upgrade ratio should still be interesting.

michaelpq commented 1 year ago

Proposal of patch sent here: https://github.com/ossc-db/pg_hint_plan/pull/111