postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 14 forks source link

Upgrade scripts should use CREATE OR REPLACE and the like #111

Open esabol opened 10 months ago

esabol commented 10 months ago

I was testing 1.4.0 in a database here before it was it was 1.4.0. When I went to upgrade that database to 1.4.1, I got

ERROR:  function "spoly" already exists with same argument types

My solution was to change all the "CREATE FUNCTION" statements in the upgrade script to "CREATE OR REPLACE FUNCTION". The upgrade then worked fine. I feel this should be the standard practice for all upgrade scripts, past and future.

msdemlei commented 10 months ago

On Wed, Nov 29, 2023 at 04:26:52PM -0800, Ed Sabol wrote:

I was testing 1.4.0 in a database here before it was it was 1.4.0. When I went to upgrade to 1.4.1 and got

ERROR:  function "spoly" already exists with same argument types

My solution was to change all the "CREATE FUNCTION" statements to "CREATE OR REPLACE FUNCTION". The upgrade then worked fine. I feel this should be the standard practice for all upgrade scripts, past and future.

+1 on this. Re-runnable SQL is great; if all statements in our SQL are re-runnable we can perhaps one day stop maintaining the upgraders entirely (except perhaps a single cleanup script to drop severely misguided material) and just run the full SQL on every upgrade.

The main trouble I see is that someone would need to research when IF EXISTS and OR REPLACE were added where; I'm not absolutely sure all versions we promise to support know about them in all constructs we use in our SQL scripts.

df7cb commented 10 months ago

I'm surprised that using create or replace in the upgrade script worked to fix the problem because it actually shouldn't: https://github.com/postgres/postgres/commit/b9b21acc766db54d8c337d508d0fe2f5bf2daab0

esabol commented 10 months ago

I'm surprised that using create or replace in the upgrade script worked to fix the problem because it actually shouldn't: postgres/postgres@b9b21ac

Are we reading the same commit message? It says, "Hence, forbid CREATE OR REPLACE of an existing object unless it already belongs to the extension." (Emphasis mine.) So why shouldn't it work?

df7cb commented 10 months ago

Ah right, I had assumed you had created an spoly function outside of pg_sphere and then the upgrade complained it's already there, but it was a function from inside pg_sphere. That explains why it worked, thanks.