Helper function handling: Fix detection of functions without arguments (#695)

This accidentally broke in a recent refactoring, and caused helpers
with no arguments (e.g. "get_column_stats") to not be detected correctly.

In passing refactor definitions of "get_column_stats" and
"get_relation_stats_ext" helpers to be in SQL files.

Author: lfittl

Diff for: input/postgres/collection.go

@@ -104,7 +104,10 @@ func (c *Collection) findHelperFunction(name string, inputTypes []string) (state
 		return state.PostgresFunction{}, false
 	}
 	for _, f := range funcs {
-		args := strings.Split(f.Arguments, ", ")
+		var args []string
+		if f.Arguments != "" {
+			args = strings.Split(f.Arguments, ", ")
+		}
 		if len(inputTypes) > len(args) {
 			// We're expecting more arguments than the function has
 			continue

Diff for: input/postgres/collection_test.go

@@ -50,13 +50,20 @@ var collectionHelperTests = []collectionHelperTestpair{
 		false,
 		"",
 	},
-	// Verify we allow default arguments to be ommitted
+	// Verify we allow default arguments to be omitted
 	{
 		"get_stat_statements",
 		[]string{},
 		true,
 		"SETOF pg_stat_statements",
 	},
+	// Verify functions with no arguments match as expected
+	{
+		"get_column_stats",
+		[]string{},
+		true,
+		"TABLE(schemaname name, tablename name, attname name, inherited boolean, null_frac real, avg_width integer, n_distinct real, correlation real)",
+	},
 }
 
 func TestCollectionFindHelper(t *testing.T) {
@@ -73,6 +80,11 @@ func TestCollectionFindHelper(t *testing.T) {
 		t.Fatalf("Could not load get_stat_statements helper: %s", err)
 	}
 
+	_, err = db.Exec(util.GetColumnStatsHelper)
+	if err != nil {
+		t.Fatalf("Could not load get_column_stats helper: %s", err)
+	}
+
 	ctx := context.Background()
 	logger := &util.Logger{}
 	server := &state.Server{}

Diff for: runner/generate_helper_sql.go

@@ -11,33 +11,6 @@ import (
 	"github.com/pganalyze/collector/util"
 )
 
-var statsHelpers = []string{
-	// Column stats
-	`DROP FUNCTION IF EXISTS pganalyze.get_column_stats;
-CREATE FUNCTION pganalyze.get_column_stats() RETURNS TABLE(
-  schemaname name, tablename name, attname name, inherited bool, null_frac real, avg_width int, n_distinct real, correlation real
-) AS $$
-  /* pganalyze-collector */
-  SELECT schemaname, tablename, attname, inherited, null_frac, avg_width, n_distinct, correlation
-  FROM pg_catalog.pg_stats
-  WHERE schemaname NOT IN ('pg_catalog', 'information_schema') AND tablename <> 'pg_subscription';
-$$ LANGUAGE sql VOLATILE SECURITY DEFINER;`,
-
-	// Extended stats
-	`DROP FUNCTION IF EXISTS pganalyze.get_relation_stats_ext;
-CREATE FUNCTION pganalyze.get_relation_stats_ext() RETURNS TABLE(
-  statistics_schemaname text, statistics_name text,
-  inherited boolean, n_distinct pg_ndistinct, dependencies pg_dependencies,
-  most_common_val_nulls boolean[], most_common_freqs float8[], most_common_base_freqs float8[]
-) AS
-$$
-  /* pganalyze-collector */ SELECT statistics_schemaname::text, statistics_name::text,
-  (row_to_json(se.*)::jsonb ->> 'inherited')::boolean AS inherited, n_distinct, dependencies,
-  most_common_val_nulls, most_common_freqs, most_common_base_freqs
-  FROM pg_catalog.pg_stats_ext se
-  WHERE schemaname NOT IN ('pg_catalog', 'information_schema') AND tablename <> 'pg_subscription';
-$$ LANGUAGE sql VOLATILE SECURITY DEFINER;`}
-
 func GenerateStatsHelperSql(ctx context.Context, server *state.Server, opts state.CollectionOpts, logger *util.Logger) (string, error) {
 	db, err := postgres.EstablishConnection(ctx, server, logger, opts, "")
 	if err != nil {
@@ -55,9 +28,8 @@ func GenerateStatsHelperSql(ctx context.Context, server *state.Server, opts stat
 		output.WriteString(fmt.Sprintf("\\c %s\n", pq.QuoteIdentifier(dbName)))
 		output.WriteString("CREATE SCHEMA IF NOT EXISTS pganalyze;\n")
 		output.WriteString(fmt.Sprintf("GRANT USAGE ON SCHEMA pganalyze TO %s;\n", server.Config.GetDbUsername()))
-		for _, helper := range statsHelpers {
-			output.WriteString(helper + "\n")
-		}
+		output.WriteString(util.GetColumnStatsHelper + "\n")
+		output.WriteString(util.GetRelationStatsExtHelper + "\n")
 		output.WriteString("\n")
 	}
 

Diff for: util/helpers/get_column_stats.sql

@@ -0,0 +1,9 @@
+DROP FUNCTION IF EXISTS pganalyze.get_column_stats;
+CREATE FUNCTION pganalyze.get_column_stats() RETURNS TABLE(
+  schemaname name, tablename name, attname name, inherited bool, null_frac real, avg_width int, n_distinct real, correlation real
+) AS $$
+  /* pganalyze-collector */
+  SELECT schemaname, tablename, attname, inherited, null_frac, avg_width, n_distinct, correlation
+  FROM pg_catalog.pg_stats
+  WHERE schemaname NOT IN ('pg_catalog', 'information_schema') AND tablename <> 'pg_subscription';
+$$ LANGUAGE sql VOLATILE SECURITY DEFINER;

Diff for: util/helpers/get_relation_stats_ext.sql

@@ -0,0 +1,13 @@
+DROP FUNCTION IF EXISTS pganalyze.get_relation_stats_ext;
+CREATE FUNCTION pganalyze.get_relation_stats_ext() RETURNS TABLE(
+  statistics_schemaname text, statistics_name text,
+  inherited boolean, n_distinct pg_ndistinct, dependencies pg_dependencies,
+  most_common_val_nulls boolean[], most_common_freqs float8[], most_common_base_freqs float8[]
+) AS
+$$
+  /* pganalyze-collector */ SELECT statistics_schemaname::text, statistics_name::text,
+  (row_to_json(se.*)::jsonb ->> 'inherited')::boolean AS inherited, n_distinct, dependencies,
+  most_common_val_nulls, most_common_freqs, most_common_base_freqs
+  FROM pg_catalog.pg_stats_ext se
+  WHERE schemaname NOT IN ('pg_catalog', 'information_schema') AND tablename <> 'pg_subscription';
+$$ LANGUAGE sql VOLATILE SECURITY DEFINER;

Diff for: util/helpers/get_stat_statements.sql

@@ -1,4 +1,5 @@
-CREATE OR REPLACE FUNCTION pganalyze.get_stat_statements(showtext boolean = true) RETURNS SETOF pg_stat_statements AS
+DROP FUNCTION IF EXISTS pganalyze.get_stat_statements;
+CREATE FUNCTION pganalyze.get_stat_statements(showtext boolean = true) RETURNS SETOF pg_stat_statements AS
 $$
     /* pganalyze-collector */ SELECT * FROM public.pg_stat_statements(showtext);
 $$ LANGUAGE sql VOLATILE SECURITY DEFINER;

Diff for: util/sql_helpers.go

@@ -9,3 +9,9 @@ var ExplainAnalyzeHelper string
 
 //go:embed helpers/get_stat_statements.sql
 var GetStatStatementsHelper string
+
+//go:embed helpers/get_column_stats.sql
+var GetColumnStatsHelper string
+
+//go:embed helpers/get_relation_stats_ext.sql
+var GetRelationStatsExtHelper string