Skip to content

Commit 7d0f15e

Browse files
fridgepoetfridgepoet
authored
database_observability: fix schema_details collector to fetch column definitions with case sensitive table names (#4872)
1 parent 6f2396e commit 7d0f15e

File tree

5 files changed

+44
-32
lines changed

5 files changed

+44
-32
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ Main (unreleased)
2222

2323
- `loki.source.api` no longer drops request when relabel rules drops a specific stream. (@kalleep)
2424

25+
- (_Public Preview_) Additions to `database_observability.postgres` component:
26+
- fixes collection of Postgres schema details for mixed case table names (@fridgepoet)
27+
2528
v1.12.0-rc.0
2629
-----------------
2730

‎internal/component/database_observability/postgres/collector/connection_info_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import (
1515
)
1616

1717
func TestConnectionInfo(t *testing.T) {
18-
defer goleak.VerifyNone(t)
18+
// The goroutine which deletes expired entries runs indefinitely,
19+
// see https://github.com/hashicorp/golang-lru/blob/v2.0.7/expirable/expirable_lru.go#L79-L80
20+
defer goleak.VerifyNone(t, goleak.IgnoreTopFunction("github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1"))
1921

2022
const baseExpectedMetrics = `
2123
# HELP database_observability_connection_info Information about the connection

‎internal/component/database_observability/postgres/collector/query_details_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import (
1717
)
1818

1919
func TestQueryDetails(t *testing.T) {
20-
defer goleak.VerifyNone(t)
20+
// The goroutine which deletes expired entries runs indefinitely,
21+
// see https://github.com/hashicorp/golang-lru/blob/v2.0.7/expirable/expirable_lru.go#L79-L80
22+
defer goleak.VerifyNone(t, goleak.IgnoreTopFunction("github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1"))
2123

2224
testcases := []struct {
2325
name string
@@ -468,7 +470,9 @@ func TestQueryDetails(t *testing.T) {
468470
}
469471

470472
func TestQueryDetails_SQLDriverErrors(t *testing.T) {
471-
defer goleak.VerifyNone(t)
473+
// The goroutine which deletes expired entries runs indefinitely,
474+
// see https://github.com/hashicorp/golang-lru/blob/v2.0.7/expirable/expirable_lru.go#L79-L80
475+
defer goleak.VerifyNone(t, goleak.IgnoreTopFunction("github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1"))
472476

473477
t.Run("recoverable sql error in result set", func(t *testing.T) {
474478
t.Parallel()

‎internal/component/database_observability/postgres/collector/schema_details.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ const (
7070
pg_attribute: stores column information
7171
pg_attrdef: stores default values for columns
7272
pg_constraint: stores primary key information
73-
attr.attrelid = $1::regclass -- filter by the table name
73+
JOIN pg_class to filter by table name
74+
JOIN pg_namespace to filter by schema name
7475
AND attr.attnum > 0 -- no system columns
7576
AND NOT attr.attisdropped -- no dropped columns`
7677
*/
@@ -87,10 +88,13 @@ const (
8788
END as is_primary_key
8889
FROM
8990
pg_attribute attr
91+
JOIN pg_catalog.pg_class pg_class ON attr.attrelid = pg_class.oid
92+
JOIN pg_catalog.pg_namespace pg_namespace ON pg_class.relnamespace = pg_namespace.oid
9093
LEFT JOIN pg_catalog.pg_attrdef def ON attr.attrelid = def.adrelid AND attr.attnum = def.adnum
9194
LEFT JOIN pg_catalog.pg_constraint constraint_pk ON attr.attrelid = constraint_pk.conrelid AND attr.attnum = ANY(constraint_pk.conkey) AND constraint_pk.contype = 'p'
9295
WHERE
93-
attr.attrelid = $1::regclass
96+
pg_namespace.nspname = $1
97+
AND pg_class.relname = $2
9498
AND attr.attnum > 0
9599
AND NOT attr.attisdropped`
96100

@@ -580,8 +584,7 @@ func (c *SchemaDetails) fetchTableDefinitions(ctx context.Context, table *tableI
580584
}
581585

582586
func (c *SchemaDetails) fetchColumnsDefinitions(ctx context.Context, databaseName database, schemaName schema, tableName table, dbConnection *sql.DB) (*tableSpec, error) {
583-
qualifiedTableName := fmt.Sprintf("%s.%s", schemaName, tableName)
584-
colRS, err := dbConnection.QueryContext(ctx, selectColumnNames, qualifiedTableName)
587+
colRS, err := dbConnection.QueryContext(ctx, selectColumnNames, schemaName, tableName)
585588
if err != nil {
586589
level.Error(c.logger).Log("msg", "failed to query table columns", "datname", databaseName, "schema", schemaName, "table", tableName, "err", err)
587590
return nil, err

‎internal/component/database_observability/postgres/collector/schema_details_test.go‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
7373
}).AddRow("authors"),
7474
)
7575

76-
mock.ExpectQuery(selectColumnNames).WithArgs("public.authors").RowsWillBeClosed().
76+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "authors").RowsWillBeClosed().
7777
WillReturnRows(
7878
sqlmock.NewRows([]string{
7979
"column_name",
@@ -189,7 +189,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
189189
}).AddRow("spatial_ref_sys"),
190190
)
191191

192-
mock.ExpectQuery(selectColumnNames).WithArgs("public.authors").RowsWillBeClosed().
192+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "authors").RowsWillBeClosed().
193193
WillReturnRows(
194194
sqlmock.NewRows([]string{
195195
"column_name",
@@ -223,7 +223,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
223223
}),
224224
)
225225

226-
mock.ExpectQuery(selectColumnNames).WithArgs("public.categories").RowsWillBeClosed().
226+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "categories").RowsWillBeClosed().
227227
WillReturnRows(
228228
sqlmock.NewRows([]string{
229229
"column_name",
@@ -257,7 +257,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
257257
}),
258258
)
259259

260-
mock.ExpectQuery(selectColumnNames).WithArgs("postgis.spatial_ref_sys").RowsWillBeClosed().
260+
mock.ExpectQuery(selectColumnNames).WithArgs("postgis", "spatial_ref_sys").RowsWillBeClosed().
261261
WillReturnRows(
262262
sqlmock.NewRows([]string{
263263
"column_name",
@@ -383,7 +383,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
383383
WillReturnRows(sqlmock.NewRows([]string{"schema_name"}).AddRow("public"))
384384
db1Mock.ExpectQuery(selectTableNames).WithArgs("public").RowsWillBeClosed().
385385
WillReturnRows(sqlmock.NewRows([]string{"table_name"}).AddRow("users"))
386-
db1Mock.ExpectQuery(selectColumnNames).WithArgs("public.users").RowsWillBeClosed().
386+
db1Mock.ExpectQuery(selectColumnNames).WithArgs("public", "users").RowsWillBeClosed().
387387
WillReturnRows(
388388
sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
389389
AddRow("id", "integer", true, nil, "", true),
@@ -397,7 +397,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
397397
WillReturnRows(sqlmock.NewRows([]string{"schema_name"}).AddRow("public"))
398398
db2Mock.ExpectQuery(selectTableNames).WithArgs("public").RowsWillBeClosed().
399399
WillReturnRows(sqlmock.NewRows([]string{"table_name"}).AddRow("metrics"))
400-
db2Mock.ExpectQuery(selectColumnNames).WithArgs("public.metrics").RowsWillBeClosed().
400+
db2Mock.ExpectQuery(selectColumnNames).WithArgs("public", "metrics").RowsWillBeClosed().
401401
WillReturnRows(
402402
sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
403403
AddRow("id", "bigint", true, nil, "", true),
@@ -485,7 +485,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
485485
}).AddRow("users"),
486486
)
487487

488-
mock.ExpectQuery(selectColumnNames).WithArgs("public.users").RowsWillBeClosed().
488+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "users").RowsWillBeClosed().
489489
WillReturnRows(
490490
sqlmock.NewRows([]string{
491491
"column_name",
@@ -654,7 +654,7 @@ func Test_Postgres_SchemaDetails(t *testing.T) {
654654
}).AddRow("test_table"),
655655
)
656656

657-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
657+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
658658
WillReturnRows(
659659
sqlmock.NewRows([]string{
660660
"column_name",
@@ -765,7 +765,7 @@ func Test_Postgres_SchemaDetails_collector_detects_auto_increment_column(t *test
765765
}).AddRow("users"),
766766
)
767767

768-
mock.ExpectQuery(selectColumnNames).WithArgs("public.users").RowsWillBeClosed().
768+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "users").RowsWillBeClosed().
769769
WillReturnRows(
770770
sqlmock.NewRows([]string{
771771
"column_name",
@@ -871,7 +871,7 @@ func Test_Postgres_SchemaDetails_collector_detects_auto_increment_column(t *test
871871
}).AddRow("products"),
872872
)
873873

874-
mock.ExpectQuery(selectColumnNames).WithArgs("public.products").RowsWillBeClosed().
874+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "products").RowsWillBeClosed().
875875
WillReturnRows(
876876
sqlmock.NewRows([]string{
877877
"column_name",
@@ -978,7 +978,7 @@ func Test_Postgres_SchemaDetails_collector_detects_auto_increment_column(t *test
978978
}).AddRow("books"),
979979
)
980980

981-
mock.ExpectQuery(selectColumnNames).WithArgs("public.books").RowsWillBeClosed().
981+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "books").RowsWillBeClosed().
982982
WillReturnRows(
983983
sqlmock.NewRows([]string{
984984
"column_name",
@@ -1085,7 +1085,7 @@ func Test_Postgres_SchemaDetails_caching(t *testing.T) {
10851085
)
10861086

10871087
// selectColumnNames, selectIndexes, selectForeignKeys called only first run
1088-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1088+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
10891089
WillReturnRows(
10901090
sqlmock.NewRows([]string{
10911091
"column_name",
@@ -1215,7 +1215,7 @@ func Test_Postgres_SchemaDetails_caching(t *testing.T) {
12151215
}).AddRow("test_table"),
12161216
)
12171217

1218-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1218+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
12191219
WillReturnRows(
12201220
sqlmock.NewRows([]string{
12211221
"column_name",
@@ -1322,7 +1322,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
13221322
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
13231323
require.NoError(t, err)
13241324
defer db.Close()
1325-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").
1325+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").
13261326
WillReturnError(fmt.Errorf("column query error"))
13271327

13281328
collector := &SchemaDetails{logger: log.NewNopLogger()}
@@ -1347,7 +1347,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
13471347
logger: log.NewNopLogger(),
13481348
}
13491349

1350-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").
1350+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").
13511351
WillReturnError(fmt.Errorf("column query error"))
13521352

13531353
_, err = collector.fetchColumnsDefinitions(context.Background(), "testdb", "public", "test_table", db)
@@ -1367,7 +1367,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
13671367
logger: log.NewNopLogger(),
13681368
}
13691369

1370-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1370+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
13711371
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation"}).
13721372
AddRow("id", "integer", true, nil, ""))
13731373

@@ -1383,7 +1383,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
13831383
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
13841384
require.NoError(t, err)
13851385
defer db.Close()
1386-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").
1386+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").
13871387
WillReturnRows(
13881388
sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
13891389
AddRow("id", "integer", true, nil, "", true).
@@ -1402,7 +1402,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
14021402
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
14031403
require.NoError(t, err)
14041404
defer db.Close()
1405-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1405+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
14061406
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
14071407
AddRow("id", "integer", true, nil, "", true))
14081408
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").
@@ -1421,7 +1421,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
14211421
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
14221422
require.NoError(t, err)
14231423
defer db.Close()
1424-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1424+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
14251425
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
14261426
AddRow("id", "integer", true, nil, "", true))
14271427
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").RowsWillBeClosed().
@@ -1441,7 +1441,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
14411441
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
14421442
require.NoError(t, err)
14431443
defer db.Close()
1444-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1444+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
14451445
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
14461446
AddRow("id", "integer", true, nil, "", true))
14471447
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").
@@ -1464,7 +1464,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
14641464
require.NoError(t, err)
14651465
defer db.Close()
14661466

1467-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1467+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
14681468
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
14691469
AddRow("id", "integer", true, nil, "", true))
14701470
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").RowsWillBeClosed().
@@ -1486,7 +1486,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
14861486
require.NoError(t, err)
14871487
defer db.Close()
14881488

1489-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1489+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
14901490
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
14911491
AddRow("id", "integer", true, nil, "", true))
14921492
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").RowsWillBeClosed().
@@ -1508,7 +1508,7 @@ func Test_Postgres_SchemaDetails_ErrorCases(t *testing.T) {
15081508
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual))
15091509
require.NoError(t, err)
15101510
defer db.Close()
1511-
mock.ExpectQuery(selectColumnNames).WithArgs("public.test_table").RowsWillBeClosed().
1511+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "test_table").RowsWillBeClosed().
15121512
WillReturnRows(sqlmock.NewRows([]string{"column_name", "column_type", "not_nullable", "column_default", "identity_generation", "is_primary_key"}).
15131513
AddRow("id", "integer", true, nil, "", true))
15141514
mock.ExpectQuery(selectIndexes).WithArgs("public", "test_table").RowsWillBeClosed().
@@ -1627,7 +1627,7 @@ func Test_SchemaDetails_populates_TableRegistry(t *testing.T) {
16271627
"table_name",
16281628
}).AddRow("users").AddRow("orders"),
16291629
)
1630-
mock.ExpectQuery(selectColumnNames).WithArgs("public.users").RowsWillBeClosed().
1630+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "users").RowsWillBeClosed().
16311631
WillReturnRows(
16321632
sqlmock.NewRows([]string{
16331633
"column_name",
@@ -1658,7 +1658,7 @@ func Test_SchemaDetails_populates_TableRegistry(t *testing.T) {
16581658
"referenced_column_name",
16591659
}),
16601660
)
1661-
mock.ExpectQuery(selectColumnNames).WithArgs("public.orders").RowsWillBeClosed().
1661+
mock.ExpectQuery(selectColumnNames).WithArgs("public", "orders").RowsWillBeClosed().
16621662
WillReturnRows(
16631663
sqlmock.NewRows([]string{
16641664
"column_name",

0 commit comments

Comments
 (0)