Skip to content

Commit

Permalink
Improve support for EDB Postgres Advanced Server (EPAS) (#601)
Browse files Browse the repository at this point in the history
Previously the collector would emit system catalogs that are only existing
on EPAS, causing unnecessary information to show in the pganalyze dashboard.

Additionally, EPAS allows function signatures to be re-used between functions
and procedures (presumably this was allowed before upstream Postgres added
CREATE PROCEDURE), which causes a server-side error with pganalyze. Since this
is an edge case and there are no intents to support this upstream, ignore
duplicate functions/procedures that have the same signature.
  • Loading branch information
lfittl authored Oct 3, 2024
1 parent 750cdf8 commit 03ebfd1
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 29 deletions.
61 changes: 51 additions & 10 deletions input/postgres/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import (

"github.com/guregu/null"
"github.com/pganalyze/collector/state"
"github.com/pganalyze/collector/util"
)

const functionsSQLDefaultKindFields = "CASE WHEN pp.proisagg THEN 'a' WHEN pp.proiswindow THEN 'w' ELSE 'f' END AS prokind"
const functionsSQLpg11KindFields = "pp.prokind"

const functionsSQL string = `
SELECT pp.oid,
pn.nspname,
n.nspname,
pp.proname,
pl.lanname,
pp.prosrc,
Expand All @@ -29,31 +30,44 @@ SELECT pp.oid,
pp.proretset,
pp.provolatile
FROM pg_catalog.pg_proc pp
INNER JOIN pg_catalog.pg_namespace pn ON (pp.pronamespace = pn.oid)
INNER JOIN pg_catalog.pg_namespace n ON (pp.pronamespace = n.oid)
INNER JOIN pg_catalog.pg_language pl ON (pp.prolang = pl.oid)
WHERE pn.nspname NOT IN ('pg_catalog', 'information_schema')
WHERE %s
AND pp.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_proc'::regclass)
AND ($1 = '' OR (pn.nspname || '.' || pp.proname) !~* $1)`
AND ($1 = '' OR (n.nspname || '.' || pp.proname) !~* $1)`

const functionStatsSQL string = `
SELECT funcid, calls, total_time, self_time
FROM pg_stat_user_functions psuf
INNER JOIN pg_catalog.pg_proc pp ON (psuf.funcid = pp.oid)
INNER JOIN pg_catalog.pg_namespace pn ON (pp.pronamespace = pn.oid)
WHERE pn.nspname NOT IN ('pg_catalog', 'information_schema')
INNER JOIN pg_catalog.pg_namespace n ON (pp.pronamespace = n.oid)
WHERE %s
AND pp.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_proc'::regclass)
AND ($1 = '' OR (pn.nspname || '.' || pp.proname) !~* $1)`
AND ($1 = '' OR (n.nspname || '.' || pp.proname) !~* $1)`

func GetFunctions(ctx context.Context, db *sql.DB, postgresVersion state.PostgresVersion, currentDatabaseOid state.Oid, ignoreRegexp string) ([]state.PostgresFunction, error) {
type FunctionSignature struct {
SchemaName string
FunctionName string
Arguments string
}

func GetFunctions(ctx context.Context, logger *util.Logger, db *sql.DB, postgresVersion state.PostgresVersion, currentDatabaseOid state.Oid, ignoreRegexp string) ([]state.PostgresFunction, error) {
var kindFields string
var systemCatalogFilter string

if postgresVersion.Numeric >= state.PostgresVersion11 {
kindFields = functionsSQLpg11KindFields
} else {
kindFields = functionsSQLDefaultKindFields
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(functionsSQL, kindFields))
if postgresVersion.IsEPAS {
systemCatalogFilter = relationSQLEPASSystemCatalogFilter
} else {
systemCatalogFilter = relationSQLdefaultSystemCatalogFilter
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(functionsSQL, kindFields, systemCatalogFilter))
if err != nil {
return nil, err
}
Expand All @@ -68,6 +82,11 @@ func GetFunctions(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
defer rows.Close()

var functions []state.PostgresFunction
var functionSignatures map[FunctionSignature]struct{}

if postgresVersion.IsEPAS {
functionSignatures = make(map[FunctionSignature]struct{})
}

for rows.Next() {
var row state.PostgresFunction
Expand All @@ -80,6 +99,20 @@ func GetFunctions(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
return nil, err
}

if postgresVersion.IsEPAS {
// EDB Postgres Advanced Server allows creating functions and procedures that share a signature
// (functions take precedence when calling), something which regular Postgres does not allow.
//
// For now we ignore them here to avoid server side errors, but we could revise this in the future.
signature := FunctionSignature{SchemaName: row.SchemaName, FunctionName: row.FunctionName, Arguments: row.Arguments}
if _, ok := functionSignatures[signature]; ok {
logger.PrintVerbose("Ignoring duplicate function signature for %s.%s(%s)", signature.SchemaName, signature.FunctionName, signature.Arguments)
continue
} else {
functionSignatures[signature] = struct{}{}
}
}

row.DatabaseOid = currentDatabaseOid
row.Config = unpackPostgresStringArray(config)

Expand All @@ -94,7 +127,15 @@ func GetFunctions(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
}

func GetFunctionStats(ctx context.Context, db *sql.DB, postgresVersion state.PostgresVersion, ignoreRegexp string) (functionStats state.PostgresFunctionStatsMap, err error) {
stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+functionStatsSQL)
var systemCatalogFilter string

if postgresVersion.IsEPAS {
systemCatalogFilter = relationSQLEPASSystemCatalogFilter
} else {
systemCatalogFilter = relationSQLdefaultSystemCatalogFilter
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(functionStatsSQL, systemCatalogFilter))
if err != nil {
err = fmt.Errorf("FunctionStats/Prepare: %s", err)
return
Expand Down
11 changes: 9 additions & 2 deletions input/postgres/relation_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ SELECT c.oid,
AND c.relkind IN ('r','v','m','p')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND ($1 = '' OR (n.nspname || '.' || c.relname) !~* $1)
UNION ALL
SELECT relid,
Expand Down Expand Up @@ -153,14 +153,21 @@ SELECT relid,

func GetRelationStats(ctx context.Context, db *sql.DB, postgresVersion state.PostgresVersion, server *state.Server) (relStats state.PostgresRelationStatsMap, err error) {
var insertsSinceVacuumField string
var systemCatalogFilter string

if postgresVersion.Numeric >= state.PostgresVersion13 {
insertsSinceVacuumField = relationStatsSQLInsertsSinceVacuumFieldPg13
} else {
insertsSinceVacuumField = relationStatsSQLInsertsSinceVacuumFieldDefault
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(relationStatsSQL, insertsSinceVacuumField))
if postgresVersion.IsEPAS {
systemCatalogFilter = relationSQLEPASSystemCatalogFilter
} else {
systemCatalogFilter = relationSQLdefaultSystemCatalogFilter
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(relationStatsSQL, insertsSinceVacuumField, systemCatalogFilter))
if err != nil {
err = fmt.Errorf("RelationStats/Prepare: %s", err)
return
Expand Down
30 changes: 20 additions & 10 deletions input/postgres/relations.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/pganalyze/collector/state"
)

const relationSQLdefaultSystemCatalogFilter = "n.nspname NOT IN ('pg_catalog', 'pg_toast', 'information_schema')"
const relationSQLEPASSystemCatalogFilter = "n.nspname NOT IN ('pg_catalog', 'pg_toast', 'information_schema', 'sys') AND n.nspparent <> 'sys'::regnamespace"

const relationsSQLOidField = "c.relhasoids AS relation_has_oids"
const relationsSQLpg12OidField = "false AS relation_has_oids"

Expand Down Expand Up @@ -41,7 +44,7 @@ const relationsSQL string = `
WHERE c.relkind IN ('r','v','m','p')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND ($1 = '' OR (n.nspname || '.' || c.relname) !~* $1)`

const columnsSQL string = `
Expand All @@ -64,7 +67,7 @@ const columnsSQL string = `
WHERE c.relkind IN ('r','v','m','p')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND a.attnum > 0
AND NOT a.attisdropped
AND c.oid NOT IN (SELECT relid FROM locked_relids)
Expand Down Expand Up @@ -104,7 +107,7 @@ SELECT c.oid,
WHERE c.relkind IN ('r','v','m','p')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND c.oid NOT IN (SELECT relid FROM locked_relids)
AND c2.oid NOT IN (SELECT relid FROM locked_relids)
AND ($1 = '' OR (n.nspname || '.' || c.relname) !~* $1)
Expand Down Expand Up @@ -143,7 +146,7 @@ SELECT c.oid,
WHERE c.relkind IN ('r','v','m','p')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND c.oid NOT IN (SELECT relid FROM locked_relids)
AND ($1 = '' OR (n.nspname || '.' || c.relname) !~* $1)
UNION ALL
Expand Down Expand Up @@ -171,7 +174,7 @@ SELECT c.oid,
WHERE c.relkind IN ('v','m')
AND c.relpersistence <> 't'
AND c.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_class'::regclass)
AND n.nspname NOT IN ('pg_catalog','pg_toast','information_schema')
AND %s
AND c.oid NOT IN (SELECT relid FROM locked_relids)
AND ($1 = '' OR (n.nspname || '.' || c.relname) !~* $1)
UNION ALL
Expand All @@ -184,6 +187,13 @@ SELECT relid,
func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.PostgresVersion, currentDatabaseOid state.Oid, ignoreRegexp string) ([]state.PostgresRelation, error) {
relations := make(map[state.Oid]state.PostgresRelation, 0)

var systemCatalogFilter string
if postgresVersion.IsEPAS {
systemCatalogFilter = relationSQLEPASSystemCatalogFilter
} else {
systemCatalogFilter = relationSQLdefaultSystemCatalogFilter
}

// Relations
var oidField string

Expand All @@ -193,7 +203,7 @@ func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
oidField = relationsSQLOidField
}

rows, err := db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(relationsSQL, oidField), ignoreRegexp)
rows, err := db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(relationsSQL, oidField, systemCatalogFilter), ignoreRegexp)
if err != nil {
err = fmt.Errorf("Relations/Query: %s", err)
return nil, err
Expand Down Expand Up @@ -241,7 +251,7 @@ func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
}

// Columns
rows, err = db.QueryContext(ctx, QueryMarkerSQL+columnsSQL, ignoreRegexp)
rows, err = db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(columnsSQL, systemCatalogFilter), ignoreRegexp)
if err != nil {
err = fmt.Errorf("Columns/Query: %s", err)
return nil, err
Expand Down Expand Up @@ -280,7 +290,7 @@ func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
}

// Indices
rows, err = db.QueryContext(ctx, QueryMarkerSQL+indicesSQL, ignoreRegexp)
rows, err = db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(indicesSQL, systemCatalogFilter), ignoreRegexp)
if err != nil {
err = fmt.Errorf("Indices/Query: %s", err)
return nil, err
Expand Down Expand Up @@ -335,7 +345,7 @@ func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
}

// Constraints
rows, err = db.QueryContext(ctx, QueryMarkerSQL+constraintsSQL, ignoreRegexp)
rows, err = db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(constraintsSQL, systemCatalogFilter), ignoreRegexp)
if err != nil {
err = fmt.Errorf("Constraints/Query: %s", err)
return nil, err
Expand Down Expand Up @@ -399,7 +409,7 @@ func GetRelations(ctx context.Context, db *sql.DB, postgresVersion state.Postgre
}

// View definitions
rows, err = db.QueryContext(ctx, QueryMarkerSQL+viewDefinitionSQL, ignoreRegexp)
rows, err = db.QueryContext(ctx, QueryMarkerSQL+fmt.Sprintf(viewDefinitionSQL, systemCatalogFilter), ignoreRegexp)
if err != nil {
err = fmt.Errorf("Views/Prepare: %s", err)
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion input/postgres/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func collectSchemaData(ctx context.Context, collectionOpts state.CollectionOpts,
}

if collectionOpts.CollectPostgresFunctions {
newFunctions, err := GetFunctions(ctx, db, postgresVersion, databaseOid, server.Config.IgnoreSchemaRegexp)
newFunctions, err := GetFunctions(ctx, logger, db, postgresVersion, databaseOid, server.Config.IgnoreSchemaRegexp)
if err != nil {
return ps, ts, fmt.Errorf("error collecting stored procedure metadata: %s", err)
}
Expand Down
14 changes: 10 additions & 4 deletions input/postgres/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"encoding/json"
"fmt"

"github.com/pganalyze/collector/state"
)
Expand Down Expand Up @@ -33,13 +34,18 @@ SELECT t.oid,
AND (t.typrelid = 0 OR (SELECT c.relkind = 'c' FROM pg_catalog.pg_class c WHERE c.oid = t.typrelid))
AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_type el WHERE el.oid = t.typelem AND el.typarray = t.oid)
AND t.oid NOT IN (SELECT pd.objid FROM pg_catalog.pg_depend pd WHERE pd.deptype = 'e' AND pd.classid = 'pg_catalog.pg_type'::regclass)
AND n.nspname <> 'pg_catalog'
AND n.nspname <> 'information_schema'
AND n.nspname !~ '^pg_toast'
AND %s
`

func GetTypes(ctx context.Context, db *sql.DB, postgresVersion state.PostgresVersion, currentDatabaseOid state.Oid) ([]state.PostgresType, error) {
stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+typesSQL)
var systemCatalogFilter string
if postgresVersion.IsEPAS {
systemCatalogFilter = relationSQLEPASSystemCatalogFilter
} else {
systemCatalogFilter = relationSQLdefaultSystemCatalogFilter
}

stmt, err := db.PrepareContext(ctx, QueryMarkerSQL+fmt.Sprintf(typesSQL, systemCatalogFilter))
if err != nil {
return nil, err
}
Expand Down
2 changes: 2 additions & 0 deletions input/postgres/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package postgres
import (
"context"
"database/sql"
"strings"

"github.com/pganalyze/collector/state"
"github.com/pganalyze/collector/util"
Expand All @@ -14,6 +15,7 @@ func GetPostgresVersion(ctx context.Context, logger *util.Logger, db *sql.DB) (v
if err != nil {
return
}
version.IsEPAS = strings.Contains(version.Full, "EnterpriseDB Advanced Server")

err = db.QueryRowContext(ctx, QueryMarkerSQL+"SHOW server_version").Scan(&version.Short)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions state/postgres_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type PostgresVersion struct {
Numeric int `json:"numeric"` // e.g. 90501

// For collector use only, to avoid calling functions that don't work
IsAwsAurora bool
IsCitus bool
IsAwsAurora bool // Amazon Aurora
IsCitus bool // Citus extension (e.g. with Azure CosmosDB for PostgreSQL)
IsEPAS bool // EnterpriseDB Advanced Server
}

0 comments on commit 03ebfd1

Please sign in to comment.