feat(indexer): postgres schema creation + CI config #20701
Conversation
|
|
|
|
||
| // CreateTableSql generates a CREATE TABLE statement for the object type. | ||
| func (tm *TableManager) CreateTableSql(writer io.Writer) error { | ||
| _, err := fmt.Fprintf(writer, "CREATE TABLE IF NOT EXISTS %q (\n\t", tm.TableName()) |
There was a problem hiding this comment.
Shall we use ? for avoiding requiring to sanitize table name and module names?
There was a problem hiding this comment.
Can placeholders bind identifiers? I thought it was only parameters. The downside would be that it's hard to inspect the actual SQL in tests and logs
There was a problem hiding this comment.
I see, everything goes to a writer, cannot be a prepared statement indeed. We should probably sanitize the inputs then.
There was a problem hiding this comment.
Schema validation enforces use of the NameFormat regex first, then second %q quotes and escapes these
You're right that manager is non intuitive because they don't actually manage modules. How about calling these "module indexer" and "object indexer" instead of manager? Applied this renaming in the latest commit. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
indexer/postgres/go.mod (1)
1-5: Update to Go module declaration and notes.The module declaration and the note about staying on an earlier version of Go are clear and well-documented. However, consider updating to a more recent version of Go if possible to benefit from the latest features and security updates.
indexer/postgres/tests/testdata/init_schema_no_retain_delete.txt (1)
7-35: Table creation fortest_all_kinds.The SQL statements for creating the table
test_all_kindsare correct. However, consider the following improvements:
- The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
- Add a comma for clarity.
- "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos")) STORED, + "time" TIMESTAMPTZ ALWAYS GENERATED AS (nanos_to_timestamptz("time_nanos")) STORED, - "float64" DOUBLE PRECISION NOT NULL + "float64" DOUBLE PRECISION NOT NULL,Tools
LanguageTool
[style] ~25-~25: The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
Context: ..." BOOLEAN NOT NULL, "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos"))...(ADVERB_WORD_ORDER)
[typographical] ~29-~29: Consider adding a comma here.
Context: ...oat32" REAL NOT NULL, "float64" DOUBLE PRECISION NOT NULL, "bech32address" TEXT NOT NUL...(NN_NOT_NN_COMMA)
indexer/postgres/tests/testdata/init_schema.txt (1)
8-35: Improve table creation fortest_all_kinds.The SQL statement for creating the table
test_all_kindsis mostly correct. However, consider the following improvements:
- The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
- Add a comma for clarity.
- "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos")) STORED, + "time" TIMESTAMPTZ ALWAYS GENERATED AS (nanos_to_timestamptz("time_nanos")) STORED, - "float64" DOUBLE PRECISION NOT NULL + "float64" DOUBLE PRECISION NOT NULL,Tools
LanguageTool
[style] ~25-~25: The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
Context: ..." BOOLEAN NOT NULL, "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos"))...(ADVERB_WORD_ORDER)
[typographical] ~29-~29: Consider adding a comma here.
Context: ...oat32" REAL NOT NULL, "float64" DOUBLE PRECISION NOT NULL, "bech32address" TEXT NOT NUL...(NN_NOT_NN_COMMA)
indexer/postgres/module.go (1)
31-55: Improve error messages inInitializeSchema.The
InitializeSchemamethod is mostly correct. However, consider adding more context to error messages for better debugging.- return err + return fmt.Errorf("failed to create enum types for fields in module %s: %w", m.moduleName, err)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
indexer/postgres/go.sumis excluded by!**/*.sumindexer/postgres/tests/go.sumis excluded by!**/*.sum
Files selected for processing (15)
- indexer/postgres/README.md (1 hunks)
- indexer/postgres/column.go (1 hunks)
- indexer/postgres/create_table.go (1 hunks)
- indexer/postgres/create_table_test.go (1 hunks)
- indexer/postgres/enum.go (1 hunks)
- indexer/postgres/go.mod (1 hunks)
- indexer/postgres/indexer.go (1 hunks)
- indexer/postgres/internal/testdata/example_schema.go (1 hunks)
- indexer/postgres/module.go (1 hunks)
- indexer/postgres/object.go (1 hunks)
- indexer/postgres/options.go (1 hunks)
- indexer/postgres/tests/go.mod (1 hunks)
- indexer/postgres/tests/init_schema_test.go (1 hunks)
- indexer/postgres/tests/testdata/init_schema.txt (1 hunks)
- indexer/postgres/tests/testdata/init_schema_no_retain_delete.txt (1 hunks)
Files skipped from review due to trivial changes (1)
- indexer/postgres/create_table.go
Files skipped from review as they are similar to previous changes (8)
- indexer/postgres/README.md
- indexer/postgres/column.go
- indexer/postgres/create_table_test.go
- indexer/postgres/enum.go
- indexer/postgres/indexer.go
- indexer/postgres/internal/testdata/example_schema.go
- indexer/postgres/options.go
- indexer/postgres/tests/init_schema_test.go
Additional context used
Path-based instructions (2)
indexer/postgres/object.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.indexer/postgres/module.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
LanguageTool
indexer/postgres/tests/testdata/init_schema_no_retain_delete.txt
[style] ~25-~25: The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
Context: ..." BOOLEAN NOT NULL, "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos"))...(ADVERB_WORD_ORDER)
[typographical] ~29-~29: Consider adding a comma here.
Context: ...oat32" REAL NOT NULL, "float64" DOUBLE PRECISION NOT NULL, "bech32address" TEXT NOT NUL...(NN_NOT_NN_COMMA)
indexer/postgres/tests/testdata/init_schema.txt
[style] ~25-~25: The adverb ‘ALWAYS’ is usually put before the verb ‘GENERATED’.
Context: ..." BOOLEAN NOT NULL, "time" TIMESTAMPTZ GENERATED ALWAYS AS (nanos_to_timestamptz("time_nanos"))...(ADVERB_WORD_ORDER)
[typographical] ~29-~29: Consider adding a comma here.
Context: ...oat32" REAL NOT NULL, "float64" DOUBLE PRECISION NOT NULL, "bech32address" TEXT NOT NUL...(NN_NOT_NN_COMMA)
Additional comments not posted (18)
indexer/postgres/go.mod (1)
7-11: Dependency management.The dependency on
cosmossdk.io/schemais correctly specified. Ensure that this dependency is compatible with the rest of your codebase.indexer/postgres/object.go (4)
1-7: Package and imports.The package declaration and imports are appropriate. Ensure that all imported packages are necessary and used within the code.
9-16: ObjectIndexer struct definition.The
ObjectIndexerstruct is well-defined with clear field names. Ensure that theOptionstype is correctly defined and used.
18-39: NewObjectIndexer function.The
NewObjectIndexerfunction correctly initializes theObjectIndexerstruct. The use of maps forallFieldsandvalueFieldsis efficient. Ensure that theOptionsparameter is correctly utilized within the struct.
41-44: TableName method.The
TableNamemethod correctly formats the table name using the module name and object type name. The use offmt.Sprintfis appropriate.indexer/postgres/tests/go.mod (3)
1-11: Module declaration and dependencies.The module declaration and dependencies are appropriate for testing. Ensure that all dependencies are necessary and compatible with the rest of your codebase.
13-29: Indirect dependencies.The indirect dependencies are correctly specified. Ensure that these dependencies do not introduce any conflicts or issues in your codebase.
31-33: Replace directive and Go version.The replace directive correctly points to the local module. The Go version is appropriately set to 1.22.
indexer/postgres/tests/testdata/init_schema_no_retain_delete.txt (4)
1-2: Enum type creation.The SQL statement for creating the
test_my_enumenum type is correct.
4-5: Enum type creation.The SQL statement for creating the
test_vote_typeenum type is correct.
37-45: Table creation fortest_singleton.The SQL statements for creating the table
test_singletonare correct.
47-54: Table creation fortest_vote.The SQL statements for creating the table
test_voteare correct.indexer/postgres/tests/testdata/init_schema.txt (4)
2-2: Enum creation fortest_my_enumlooks good.The SQL statement for creating the enum type
test_my_enumis correct.
5-5: Enum creation fortest_vote_typelooks good.The SQL statement for creating the enum type
test_vote_typeis correct.
38-45: Table creation fortest_singletonlooks good.The SQL statement for creating the table
test_singletonis correct.
48-55: Table creation fortest_votelooks good.The SQL statement for creating the table
test_voteis correct.indexer/postgres/module.go (2)
20-28: Constructor forModuleIndexerlooks good.The
NewModuleIndexermethod correctly initializes a newModuleIndexerwith the provided parameters.
59-61: Getter forObjectIndexerslooks good.The
ObjectIndexersmethod correctly returns the object indexers for the module.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (7)
- .github/dependabot.yml (1 hunks)
- .github/pr_labeler.yml (1 hunks)
- .github/workflows/test.yml (1 hunks)
- indexer/postgres/create_table_test.go (1 hunks)
- indexer/postgres/tests/init_schema_test.go (1 hunks)
- schema/field.go (3 hunks)
- schema/object_type.go (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- .github/dependabot.yml
- .github/pr_labeler.yml
- .github/workflows/test.yml
- indexer/postgres/tests/init_schema_test.go
Additional context used
Path-based instructions (3)
schema/field.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.indexer/postgres/create_table_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"schema/object_type.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (11)
schema/field.go (3)
36-36: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.
49-49: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.
70-70: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.indexer/postgres/create_table_test.go (5)
10-41: Approved: Test function for creating SQL table with all kinds of fields.The function is well-structured and includes expected output comments for validation.
43-54: Approved: Test function for creating SQL table for a singleton object.The function is well-structured and includes expected output comments for validation.
56-67: Approved: Test function for creating SQL table for a vote object.The function is well-structured and includes expected output comments for validation.
69-79: Approved: Test function for creating SQL table for a vote object without retaining deletions.The function is well-structured and includes expected output comments for validation.
81-94: Approved: Helper functions for creating SQL tables.The functions are well-structured and handle error checking appropriately.
schema/object_type.go (3)
46-46: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.
65-65: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.
92-92: Approved: Change in error message formatting.The change from
%wto%vin the error message formatting is appropriate to address the false positive linting issue.
Description
This PR adds:
cosmossdk.io/indexer/postgresfor the postgres indexercosmossdk.io/indexer/postgres/testsfor postgres indexer integration tests (to keep the dependencies on pgx and embedded postgres out of indexer/postgres)cosmossdk.io/schemamodule schemas with testsAuthor Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Chores