From 45cf7f1fa1e2c00afde506bfada625af9d7d4eb3 Mon Sep 17 00:00:00 2001
From: Sven Klemm <sven@timescale.com>
Date: Sat, 12 Feb 2022 18:05:20 +0100
Subject: [PATCH] Document requirements for statements in sql files

Since we now lock down search_path during update/downgrade there
are some additional requirements for writing sql files.
---
 scripts/check_update_scripts.sh |  2 +-
 sql/updates/README.md           | 48 ++++++++++++++++++++-------------
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/scripts/check_update_scripts.sh b/scripts/check_update_scripts.sh
index f0daab9a5..d770e4331 100755
--- a/scripts/check_update_scripts.sh
+++ b/scripts/check_update_scripts.sh
@@ -3,7 +3,7 @@
 SCRIPT_DIR=$(dirname ${0})
 SRC_DIR=$(dirname ${SCRIPT_DIR})
 
-if grep -ir "IF NOT EXISTS" ${SRC_DIR}/sql; then
+if grep -i "IF NOT EXISTS" ${SRC_DIR}/sql/*.sql ${SRC_DIR}/sql/*/*.sql; then
   cat <<EOF
 
 Update scripts must unconditionally add new objects and fail when the object
diff --git a/sql/updates/README.md b/sql/updates/README.md
index 4f9b53d61..93331d467 100644
--- a/sql/updates/README.md
+++ b/sql/updates/README.md
@@ -1,3 +1,32 @@
+## General principles for statements in update/downgrade scripts
+
+1. The `search_path` for these scripts will be locked down to
+  `pg_catalog`. Locking down `search_path` happens in `pre-update.sql`.
+  Therefore all object references need to be fully qualified unless
+  they reference objects from `pg_catalog`. Use `@extschema@` to refer
+  to the target schema of the installation (resolves to `public` by
+  default).
+2. Creating objects must not use IF NOT EXISTS as this will
+  introduce privilege escalation vulnerabilities.
+3. All functions should have explicit `search_path`. Setting explicit
+  `search_path` will prevent SQL function inlining for functions and
+  transaction control for procedures so for some functions/procedures
+  it is acceptable to not have explicit `search_path`. Special care
+  needs to be taken with those functions/procedures by either setting
+  `search_path` in function body or having only fully qualified object
+  references including operators.
+4. When generating the install scripts `CREATE OR REPLACE` will be
+  changed to `CREATE` to prevent users from precreating extension
+  objects. Since we need `CREATE OR REPLACE` for update scripts and
+  we don't want to maintain two versions of the sql files containing
+  the function definitions we use `CREATE OR REPLACE` in those.
+5. Any object added in a new version needs to have an equivalent
+  `CREATE` statement in the update script without `OR REPLACE` to
+  prevent precreation of the object.
+6. The creation of new metadata tables need to be part of modfiles,
+   similar to `ALTER`s of such tables. Otherwise, later modfiles
+   cannot rely on those tables being present.
+
 ## Extension updates
 
 This directory contains "modfiles" (SQL scripts) with modifications
@@ -21,25 +50,6 @@ by using multiple modfiles in order. There are two types of modfiles:
   typically contain fixes for bugs that are specific to the origin
   version, but are no longer present in the transition modfiles.
 
-To ensure that this update process works, there are a few principles
-to consider.
-
-1. Modfiles should, in most cases, only contain `ALTER` or `DROP`
-   commands that change or remove objects. In some cases,
-   modifications of metadata are also necessary.
-2. `DROP FUNCTION` needs to be idempotent. In most cases that means
-   commands should have an `IF EXISTS` clause. The reason is that
-   some modfiles might try to, e.g., `DROP` functions that aren't
-   present because they only exist in an intermediate version of the
-   database, which is skipped over.
-3. Modfiles cannot rely on objects or functions that are present in a
-   previous version of the extension. This is because a particular
-   modfile should work when upgrading from any previous version of the
-   extension, where those functions or objects aren't present yet.
-4. The creation of new metadata tables need to be part of modfiles,
-   similar to `ALTER`s of such tables. Otherwise, later modfiles
-   cannot rely on those tables being present.
-
 Notes on post_update.sql
    We use a special config var (timescaledb.update_script_stage )
 to notify that dependencies have been setup and now timescaledb