[*] fix error hint in timetable.cron_split_to_arrays

cmd/pg_timetable/main.go

@@ -94,7 +94,7 @@ var (
 	commit  = "000000"
 	version = "master"
 	date    = "unknown"
-	dbapi   = "00820"
+	dbapi   = "00850"
 )
 
 func printVersion() {

internal/pgengine/migration.go

@@ -174,6 +174,12 @@ var Migrations func() migrator.Option = func() migrator.Option {
 				return ExecuteMigrationScript(ctx, tx, "00820.sql")
 			},
 		},
+		&migrator.Migration{
+			Name: "00850 Fix error hint in timetable.cron_split_to_arrays",
+			Func: func(ctx context.Context, tx pgx.Tx) error {
+				return ExecuteMigrationScript(ctx, tx, "00850.sql")
+			},
+		},
 
 		// &migrator.Migration{
 		// 	Name: "000XX Short description of a migration",

internal/pgengine/migration_test.go

@@ -3,12 +3,15 @@ package pgengine_test
 import (
 	"context"
 	_ "embed"
+	"errors"
 	"testing"
 
 	"github.com/cybertec-postgresql/pg_timetable/internal/pgengine"
 	"github.com/cybertec-postgresql/pg_timetable/internal/testutils"
 	migrator "github.com/cybertec-postgresql/pgx-migrator"
+	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 //go:embed sql/migrations/00000.sql
@@ -37,6 +40,28 @@ func TestMigrations(t *testing.T) {
 		                JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
 		                WHERE n.nspname='timetable' AND c.relname='secret')`).Scan(&hasSecret))
 	assert.True(t, hasSecret, "00820 must create timetable.secret")
+
+	// 00850 replaces timetable.cron_split_to_arrays on existing installs.
+	assertCronSplitRejectsValue(t, pge)
+}
+
+// TestCronSplitToArraysUnknownValue checks the function created by cron.sql on a fresh install.
+func TestCronSplitToArraysUnknownValue(t *testing.T) {
+	container, cleanup := testutils.SetupPostgresContainer(t)
+	defer cleanup()
+	assertCronSplitRejectsValue(t, container.Engine)
+}
+
+// assertCronSplitRejectsValue expects the "not recognized" exception, not the
+// 42725 error the old `text + text` hint raised.
+func assertCronSplitRejectsValue(t *testing.T, pge *pgengine.PgEngine) {
+	t.Helper()
+	_, err := pge.ConfigDb.Exec(context.Background(), "SELECT timetable.cron_split_to_arrays('foo * * * *')")
+	var pgErr *pgconn.PgError
+	require.True(t, errors.As(err, &pgErr), "expected a PostgreSQL error, got %v", err)
+	assert.Equal(t, "P0001", pgErr.Code)
+	assert.Equal(t, `Value ("foo") not recognized`, pgErr.Message)
+	assert.Contains(t, pgErr.Hint, "fields separated by space or tab. Values allowed")
 }
 func TestExecuteMigrationScript(t *testing.T) {
 	assert.Error(t, pgengine.ExecuteMigrationScript(context.Background(), nil, "foo"), "File does not exist")

internal/pgengine/sql/cron.sql

@@ -48,9 +48,9 @@ BEGIN
                 a_res := array_cat(a_res, a_range);
             ELSE
                 RAISE EXCEPTION 'Value ("%") not recognized', a_element[i_index]
-                    USING HINT = 'fields separated by space or tab.'+
-                       'Values allowed: numbers (value list with ","), '+
-                    'any value with "*", range of value with "-" and step values with "/"!';
+                    USING HINT = 'fields separated by space or tab. ' ||
+                        'Values allowed: numbers (value list with ","), ' ||
+                        'any value with "*", range of value with "-" and step values with "/"!';
             END IF;
         END LOOP;
         SELECT

internal/pgengine/sql/init.sql

@@ -31,4 +31,5 @@ VALUES
     (15, '00733 Add params column to timetable.execution_log table'),
     (16, '00792 Add ability to enable and disable tasks'),
     (17, '00797 Add indexes to timetable.execution_log'),
-    (18, '00820 Add timetable.secret store');
+    (18, '00820 Add timetable.secret store'),
+    (19, '00850 Fix error hint in timetable.cron_split_to_arrays');

internal/pgengine/sql/migrations/00850.sql

@@ -0,0 +1,75 @@
+-- 00850 Fix error hint in timetable.cron_split_to_arrays
+CREATE OR REPLACE FUNCTION timetable.cron_split_to_arrays(
+    cron text,
+    OUT mins integer[],
+    OUT hours integer[],
+    OUT days integer[],
+    OUT months integer[],
+    OUT dow integer[]
+) RETURNS record AS $$
+DECLARE
+    a_element text[];
+    i_index integer;
+    a_tmp text[];
+    tmp_item text;
+    a_range int[];
+    a_split text[];
+    a_res integer[];
+    max_val integer;
+    min_val integer;
+    dimensions constant text[] = '{"minutes", "hours", "days", "months", "days of week"}';
+    allowed_ranges constant integer[][] = '{{0,59},{0,23},{1,31},{1,12},{0,7}}';
+BEGIN
+    a_element := regexp_split_to_array(cron, '\s+');
+    FOR i_index IN 1..5 LOOP
+        a_res := NULL;
+        a_tmp := string_to_array(a_element[i_index],',');
+        FOREACH  tmp_item IN ARRAY a_tmp LOOP
+            IF tmp_item ~ '^[0-9]+$' THEN -- normal integer
+                a_res := array_append(a_res, tmp_item::int);
+            ELSIF tmp_item ~ '^[*]+$' THEN -- '*' any value
+                a_range := array(select generate_series(allowed_ranges[i_index][1], allowed_ranges[i_index][2]));
+                a_res := array_cat(a_res, a_range);
+            ELSIF tmp_item ~ '^[0-9]+[-][0-9]+$' THEN -- '-' range of values
+                a_range := regexp_split_to_array(tmp_item, '-');
+                a_range := array(select generate_series(a_range[1], a_range[2]));
+                a_res := array_cat(a_res, a_range);
+            ELSIF tmp_item ~ '^[0-9]+[\/][0-9]+$' THEN -- '/' step values
+                a_range := regexp_split_to_array(tmp_item, '/');
+                a_range := array(select generate_series(a_range[1], allowed_ranges[i_index][2], a_range[2]));
+                a_res := array_cat(a_res, a_range);
+            ELSIF tmp_item ~ '^[0-9-]+[\/][0-9]+$' THEN -- '-' range of values and '/' step values
+                a_split := regexp_split_to_array(tmp_item, '/');
+                a_range := regexp_split_to_array(a_split[1], '-');
+                a_range := array(select generate_series(a_range[1], a_range[2], a_split[2]::int));
+                a_res := array_cat(a_res, a_range);
+            ELSIF tmp_item ~ '^[*]+[\/][0-9]+$' THEN -- '*' any value and '/' step values
+                a_split := regexp_split_to_array(tmp_item, '/');
+                a_range := array(select generate_series(allowed_ranges[i_index][1], allowed_ranges[i_index][2], a_split[2]::int));
+                a_res := array_cat(a_res, a_range);
+            ELSE
+                RAISE EXCEPTION 'Value ("%") not recognized', a_element[i_index]
+                    USING HINT = 'fields separated by space or tab. ' ||
+                        'Values allowed: numbers (value list with ","), ' ||
+                        'any value with "*", range of value with "-" and step values with "/"!';
+            END IF;
+        END LOOP;
+        SELECT
+           ARRAY_AGG(x.val), MIN(x.val), MAX(x.val) INTO a_res, min_val, max_val
+        FROM (
+            SELECT DISTINCT UNNEST(a_res) AS val ORDER BY val) AS x;
+        IF max_val > allowed_ranges[i_index][2] OR min_val < allowed_ranges[i_index][1] OR a_res IS NULL THEN
+            RAISE EXCEPTION '% is out of range % for %', tmp_item, allowed_ranges[i_index:i_index][:], dimensions[i_index];
+        END IF;
+        CASE i_index
+            WHEN 1 THEN mins := a_res;
+            WHEN 2 THEN hours := a_res;
+            WHEN 3 THEN days := a_res;
+            WHEN 4 THEN months := a_res;
+        ELSE
+            dow := a_res;
+        END CASE;
+    END LOOP;
+    RETURN;
+END;
+$$ LANGUAGE PLPGSQL STRICT;