From 384948b093b55e34d50be3873d241150a43d4e5c Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Mon, 15 May 2023 21:52:51 +0800 Subject: [PATCH 1/8] commit --- internal/handlers/pg/pgdb/indexes.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes.go b/internal/handlers/pg/pgdb/indexes.go index 0ca74fb8887b..53dfc03d804d 100644 --- a/internal/handlers/pg/pgdb/indexes.go +++ b/internal/handlers/pg/pgdb/indexes.go @@ -204,8 +204,14 @@ func createPgIndexIfNotExists(ctx context.Context, tx pgx.Tx, schema, table, ind return lazyerrors.Errorf("unknown sort order: %d", field.Order) } - // It's important to sanitize field.Field data here, as it's a user-provided value. - fieldsDef[i] = fmt.Sprintf(`((_jsonb->%s)) %s`, quoteString(field.Field), order) + // if the key is foo.bar, then need to modify it to foo -> bar + var transformedParts []string + fs := strings.Split(field.Field, ".") + for _, f := range fs { + // It's important to sanitize field.Field data here, as it's a user-provided value. + transformedParts = append(transformedParts, quoteString(f)) + } + fieldsDef[i] = fmt.Sprintf(`((_jsonb->%s)) %s`, strings.Join(transformedParts, " -> "), order) } sql := `CREATE` + unique + ` INDEX IF NOT EXISTS ` + pgx.Identifier{index}.Sanitize() + From 82de3f786cacc6277eb34eb5a45378e39c0962b7 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Mon, 15 May 2023 21:54:40 +0800 Subject: [PATCH 2/8] aa --- internal/handlers/pg/pgdb/indexes.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/handlers/pg/pgdb/indexes.go b/internal/handlers/pg/pgdb/indexes.go index 53dfc03d804d..0bd30cb12b12 100644 --- a/internal/handlers/pg/pgdb/indexes.go +++ b/internal/handlers/pg/pgdb/indexes.go @@ -207,6 +207,7 @@ func createPgIndexIfNotExists(ctx context.Context, tx pgx.Tx, schema, table, ind // if the key is foo.bar, then need to modify it to foo -> bar var transformedParts []string fs := strings.Split(field.Field, ".") + for _, f := range fs { // It's important to sanitize field.Field data here, as it's a user-provided value. transformedParts = append(transformedParts, quoteString(f)) From 2a2d2fdc289534b9a4708a8095de70ea6d56eb06 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Tue, 16 May 2023 22:53:46 +0800 Subject: [PATCH 3/8] caaa --- internal/handlers/pg/pgdb/indexes_test.go | 39 +++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes_test.go b/internal/handlers/pg/pgdb/indexes_test.go index a4431bfa0d65..cb58da691fce 100644 --- a/internal/handlers/pg/pgdb/indexes_test.go +++ b/internal/handlers/pg/pgdb/indexes_test.go @@ -39,11 +39,11 @@ func TestCreateIndexIfNotExists(t *testing.T) { collectionName := testutil.CollectionName(t) setupDatabase(ctx, t, pool, databaseName) - indexName := "test" + indexName := "test_nested_field" err := pool.InTransaction(ctx, func(tx pgx.Tx) error { idx := Index{ Name: indexName, - Key: []IndexKeyPair{{Field: "foo", Order: types.Ascending}, {Field: "bar", Order: types.Descending}}, + Key: []IndexKeyPair{{Field: "foo.bar", Order: types.Ascending}}, } return CreateIndexIfNotExists(ctx, tx, databaseName, collectionName, &idx) }) @@ -61,6 +61,32 @@ func TestCreateIndexIfNotExists(t *testing.T) { require.NoError(t, err) expectedIndexdef := fmt.Sprintf( + "CREATE INDEX %s ON \"%s\".%s USING btree ((((_jsonb -> 'foo'::text) -> 'bar'::text)))", + pgIndexName, databaseName, tableName, + ) + assert.Equal(t, expectedIndexdef, indexdef) + + indexName = "test" + err = pool.InTransaction(ctx, func(tx pgx.Tx) error { + idx := Index{ + Name: indexName, + Key: []IndexKeyPair{{Field: "foo", Order: types.Ascending}, {Field: "bar", Order: types.Descending}}, + } + return CreateIndexIfNotExists(ctx, tx, databaseName, collectionName, &idx) + }) + require.NoError(t, err) + + tableName = collectionNameToTableName(collectionName) + pgIndexName = indexNameToPgIndexName(collectionName, indexName) + + err = pool.p.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + databaseName, tableName, pgIndexName, + ).Scan(&indexdef) + require.NoError(t, err) + + expectedIndexdef = fmt.Sprintf( "CREATE INDEX %s ON \"%s\".%s USING btree (((_jsonb -> 'foo'::text)), ((_jsonb -> 'bar'::text)) DESC)", pgIndexName, databaseName, tableName, ) @@ -113,6 +139,15 @@ func TestDropIndexes(t *testing.T) { {Name: "_id_", Key: []IndexKeyPair{{Field: "_id", Order: types.Ascending}}, Unique: true}, }, }, + "DropNestedField": { + toCreate: []Index{ + {Name: "foo_1", Key: []IndexKeyPair{{Field: "foo.bar", Order: types.Ascending}}}, + }, + toDrop: []Index{{Key: []IndexKeyPair{{Field: "foo.bar", Order: types.Ascending}}}}, + expected: []Index{ + {Name: "_id_", Key: []IndexKeyPair{{Field: "_id", Order: types.Ascending}}, Unique: true}, + }, + }, "DropOneFromTheBeginning": { toCreate: []Index{ {Name: "foo_1", Key: []IndexKeyPair{{Field: "foo", Order: types.Ascending}}}, From 4ecf2007f92263eb1a0ce5d2034a4f1b7da8ea35 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Wed, 17 May 2023 10:41:49 +0800 Subject: [PATCH 4/8] aa --- internal/handlers/pg/pgdb/indexes_test.go | 112 ++++++++++++---------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes_test.go b/internal/handlers/pg/pgdb/indexes_test.go index cb58da691fce..71dcdd52bf75 100644 --- a/internal/handlers/pg/pgdb/indexes_test.go +++ b/internal/handlers/pg/pgdb/indexes_test.go @@ -39,58 +39,68 @@ func TestCreateIndexIfNotExists(t *testing.T) { collectionName := testutil.CollectionName(t) setupDatabase(ctx, t, pool, databaseName) - indexName := "test_nested_field" - err := pool.InTransaction(ctx, func(tx pgx.Tx) error { - idx := Index{ - Name: indexName, - Key: []IndexKeyPair{{Field: "foo.bar", Order: types.Ascending}}, - } - return CreateIndexIfNotExists(ctx, tx, databaseName, collectionName, &idx) - }) - require.NoError(t, err) - - tableName := collectionNameToTableName(collectionName) - pgIndexName := indexNameToPgIndexName(collectionName, indexName) - - var indexdef string - err = pool.p.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - databaseName, tableName, pgIndexName, - ).Scan(&indexdef) - require.NoError(t, err) - - expectedIndexdef := fmt.Sprintf( - "CREATE INDEX %s ON \"%s\".%s USING btree ((((_jsonb -> 'foo'::text) -> 'bar'::text)))", - pgIndexName, databaseName, tableName, - ) - assert.Equal(t, expectedIndexdef, indexdef) - - indexName = "test" - err = pool.InTransaction(ctx, func(tx pgx.Tx) error { - idx := Index{ - Name: indexName, - Key: []IndexKeyPair{{Field: "foo", Order: types.Ascending}, {Field: "bar", Order: types.Descending}}, - } - return CreateIndexIfNotExists(ctx, tx, databaseName, collectionName, &idx) - }) - require.NoError(t, err) - - tableName = collectionNameToTableName(collectionName) - pgIndexName = indexNameToPgIndexName(collectionName, indexName) - - err = pool.p.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - databaseName, tableName, pgIndexName, - ).Scan(&indexdef) - require.NoError(t, err) + for name, tc := range map[string]struct { + index Index // the index to create + expecteDefinition string // the expected index definition in postgresql + }{ + "keyWithoutNestedField": { + index: Index{ + Name: "foo_and_bar", + Key: []IndexKeyPair{ + {Field: "foo", Order: types.Ascending}, + {Field: "bar", Order: types.Descending}, + }, + }, + expecteDefinition: "((_jsonb -> 'foo'::text)), ((_jsonb -> 'bar'::text)) DESC", + }, + "keyWithNestedField_level1": { + index: Index{ + Name: "foo_dot_bar", + Key: []IndexKeyPair{ + {Field: "foo.bar", Order: types.Ascending}, + }, + }, + expecteDefinition: "(((_jsonb -> 'foo'::text) -> 'bar'::text))", + }, + "keyWithNestedField_level2": { + index: Index{ + Name: "foo_dot_bar_dot_c", + Key: []IndexKeyPair{ + {Field: "foo.bar.c", Order: types.Ascending}, + }, + }, + expecteDefinition: "((((_jsonb -> 'foo'::text) -> 'bar'::text) -> 'c'::text))", + }, + } { + // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#how-to-solve-this + tc := tc + t.Run(name, func(t *testing.T) { + t.Helper() + err := pool.InTransaction(ctx, func(tx pgx.Tx) error { + if err := CreateIndexIfNotExists(ctx, tx, databaseName, collectionName, &tc.index); err != nil { + return err + } + return nil + }) + require.NoError(t, err) + tableName := collectionNameToTableName(collectionName) + pgIndexName := indexNameToPgIndexName(collectionName, tc.index.Name) + + var indexdef string + err = pool.p.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + databaseName, tableName, pgIndexName, + ).Scan(&indexdef) + require.NoError(t, err) - expectedIndexdef = fmt.Sprintf( - "CREATE INDEX %s ON \"%s\".%s USING btree (((_jsonb -> 'foo'::text)), ((_jsonb -> 'bar'::text)) DESC)", - pgIndexName, databaseName, tableName, - ) - assert.Equal(t, expectedIndexdef, indexdef) + expectedIndexdef := fmt.Sprintf( + "CREATE INDEX %s ON \"%s\".%s USING btree (%s)", + pgIndexName, databaseName, tableName, tc.expecteDefinition, + ) + assert.Equal(t, expectedIndexdef, indexdef) + }) + } } // TestDropIndexes checks that we correctly drop indexes for various combination of existing indexes. From 097aa884d6d0ab6a98c26a37c7d45dd4f25f2a01 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Wed, 17 May 2023 10:44:11 +0800 Subject: [PATCH 5/8] aa --- internal/handlers/pg/pgdb/indexes_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/handlers/pg/pgdb/indexes_test.go b/internal/handlers/pg/pgdb/indexes_test.go index 71dcdd52bf75..ed3b1b1f0b2f 100644 --- a/internal/handlers/pg/pgdb/indexes_test.go +++ b/internal/handlers/pg/pgdb/indexes_test.go @@ -40,8 +40,8 @@ func TestCreateIndexIfNotExists(t *testing.T) { setupDatabase(ctx, t, pool, databaseName) for name, tc := range map[string]struct { - index Index // the index to create expecteDefinition string // the expected index definition in postgresql + index Index // the index to create }{ "keyWithoutNestedField": { index: Index{ @@ -74,6 +74,7 @@ func TestCreateIndexIfNotExists(t *testing.T) { } { // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#how-to-solve-this tc := tc + t.Run(name, func(t *testing.T) { t.Helper() err := pool.InTransaction(ctx, func(tx pgx.Tx) error { From e23de450835c7f9b3ca34073a9c08812b3417ef8 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Thu, 18 May 2023 05:04:42 +0800 Subject: [PATCH 6/8] change to expectedDefinition --- internal/handlers/pg/pgdb/indexes_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes_test.go b/internal/handlers/pg/pgdb/indexes_test.go index ed3b1b1f0b2f..f16da198a5b1 100644 --- a/internal/handlers/pg/pgdb/indexes_test.go +++ b/internal/handlers/pg/pgdb/indexes_test.go @@ -40,8 +40,8 @@ func TestCreateIndexIfNotExists(t *testing.T) { setupDatabase(ctx, t, pool, databaseName) for name, tc := range map[string]struct { - expecteDefinition string // the expected index definition in postgresql - index Index // the index to create + expectedDefinition string // the expected index definition in postgresql + index Index // the index to create }{ "keyWithoutNestedField": { index: Index{ @@ -51,7 +51,7 @@ func TestCreateIndexIfNotExists(t *testing.T) { {Field: "bar", Order: types.Descending}, }, }, - expecteDefinition: "((_jsonb -> 'foo'::text)), ((_jsonb -> 'bar'::text)) DESC", + expectedDefinition: "((_jsonb -> 'foo'::text)), ((_jsonb -> 'bar'::text)) DESC", }, "keyWithNestedField_level1": { index: Index{ @@ -60,7 +60,7 @@ func TestCreateIndexIfNotExists(t *testing.T) { {Field: "foo.bar", Order: types.Ascending}, }, }, - expecteDefinition: "(((_jsonb -> 'foo'::text) -> 'bar'::text))", + expectedDefinition: "(((_jsonb -> 'foo'::text) -> 'bar'::text))", }, "keyWithNestedField_level2": { index: Index{ @@ -69,10 +69,9 @@ func TestCreateIndexIfNotExists(t *testing.T) { {Field: "foo.bar.c", Order: types.Ascending}, }, }, - expecteDefinition: "((((_jsonb -> 'foo'::text) -> 'bar'::text) -> 'c'::text))", + expectedDefinition: "((((_jsonb -> 'foo'::text) -> 'bar'::text) -> 'c'::text))", }, } { - // https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721#how-to-solve-this tc := tc t.Run(name, func(t *testing.T) { @@ -97,7 +96,7 @@ func TestCreateIndexIfNotExists(t *testing.T) { expectedIndexdef := fmt.Sprintf( "CREATE INDEX %s ON \"%s\".%s USING btree (%s)", - pgIndexName, databaseName, tableName, tc.expecteDefinition, + pgIndexName, databaseName, tableName, tc.expectedDefinition, ) assert.Equal(t, expectedIndexdef, indexdef) }) From 0f2904b9a1d88e6c1e3cc611164ccbb90378b774 Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Thu, 18 May 2023 05:11:25 +0800 Subject: [PATCH 7/8] aaa --- internal/handlers/pg/pgdb/indexes.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes.go b/internal/handlers/pg/pgdb/indexes.go index 0bd30cb12b12..7e555a6a3710 100644 --- a/internal/handlers/pg/pgdb/indexes.go +++ b/internal/handlers/pg/pgdb/indexes.go @@ -205,12 +205,12 @@ func createPgIndexIfNotExists(ctx context.Context, tx pgx.Tx, schema, table, ind } // if the key is foo.bar, then need to modify it to foo -> bar - var transformedParts []string fs := strings.Split(field.Field, ".") + transformedParts := make([]string, len(fs)) - for _, f := range fs { + for i, f := range fs { // It's important to sanitize field.Field data here, as it's a user-provided value. - transformedParts = append(transformedParts, quoteString(f)) + transformedParts[i] = quoteString(f) } fieldsDef[i] = fmt.Sprintf(`((_jsonb->%s)) %s`, strings.Join(transformedParts, " -> "), order) } From a15579bb2d63a007b41dd5bb253d53a46b7c04ba Mon Sep 17 00:00:00 2001 From: "Daniel.Wu" Date: Fri, 19 May 2023 14:35:38 +0800 Subject: [PATCH 8/8] aa --- internal/handlers/pg/pgdb/indexes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/handlers/pg/pgdb/indexes.go b/internal/handlers/pg/pgdb/indexes.go index 7e555a6a3710..6d83f51b48c3 100644 --- a/internal/handlers/pg/pgdb/indexes.go +++ b/internal/handlers/pg/pgdb/indexes.go @@ -208,9 +208,9 @@ func createPgIndexIfNotExists(ctx context.Context, tx pgx.Tx, schema, table, ind fs := strings.Split(field.Field, ".") transformedParts := make([]string, len(fs)) - for i, f := range fs { + for j, f := range fs { // It's important to sanitize field.Field data here, as it's a user-provided value. - transformedParts[i] = quoteString(f) + transformedParts[j] = quoteString(f) } fieldsDef[i] = fmt.Sprintf(`((_jsonb->%s)) %s`, strings.Join(transformedParts, " -> "), order) }