-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(indexer/postgres)!: add basic support for header, txs and events #22695
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant changes across multiple files, primarily focusing on enhancing error handling, event processing, and dependency management. Key modifications include improvements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
app.logger.Error("Commit listening hook failed", "height", blockHeight, "err", err) | ||
if app.streamingManager.StopNodeOnErr { | ||
err = fmt.Errorf("Commit listening hook failed: %w", err) | ||
if blockHeight == 1 { | ||
// can't rollback to height 0, so just return the error | ||
return nil, fmt.Errorf("failed to commit block 1, can't automatically rollback: %w", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change potentially affects state.
Call sequence:
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).Commit (baseapp/abci.go:970)
@facundomedica your pull request is missing a changelog! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
baseapp/streaming.go (2)
5-5
: Remove unnecessary import ofencoding/json
The import of
"encoding/json"
is redundant if not used elsewhere in the code. Ensure that all imported packages are utilized to maintain a clean codebase.Apply this diff to remove the unused import:
import ( - "encoding/json" "fmt" "sort"
Line range hint
149-162
: Update function documentation to reflect new parameterThe
eventToAppDataEvent
function now accepts an additionalheight
parameter. Update the function's documentation comment to reflect this change for better clarity.Apply this diff to update the function documentation:
-func eventToAppDataEvent(event abci.Event) (appdata.Event, error) { +// eventToAppDataEvent converts an abci.Event to an appdata.Event, including the block height. +func eventToAppDataEvent(event abci.Event, height int64) (appdata.Event, error) {server/v2/cometbft/oe/optimistic_execution_test.go (2)
8-10
: Organize imports according to Go conventionGroup standard library imports separately from third-party imports, and sort them alphabetically within their groups to enhance readability, as per Go conventions.
Apply this diff to reorganize imports:
import ( "context" "errors" "testing" + "github.com/stretchr/testify/assert" + + abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" - abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" - "github.com/stretchr/testify/assert" "cosmossdk.io/core/server" "cosmossdk.io/core/store" "cosmossdk.io/core/transaction" "cosmossdk.io/log" )
Line range hint
25-27
: Check for potential data races in test executionThe test may not be thread-safe if accessed concurrently. Ensure that shared resources are properly synchronized to avoid data races during parallel test runs.
Consider using sync mechanisms like mutexes if the test suite allows for concurrent execution.
schema/decoding/middleware.go (1)
43-44
: Consider logging when skipping modules without codecsWhile silently continuing when a codec is not found makes the system more resilient, it might hide potential issues from operators. Consider adding debug-level logging to inform about skipped modules.
- // we don't have a codec for this module so continue - continue + // we don't have a codec for this module so skip it + if a.logger.IsDebug() { + a.logger.Debug("skipping module without codec", "module", moduleName) + } + continueruntime/v2/go.mod (1)
Line range hint
3-3
: Update Go version to a released versionThe module specifies Go 1.23 which hasn't been released yet. This could cause compatibility issues.
Update to the latest stable version:
-go 1.23 +go 1.22x/accounts/keeper.go (1)
65-72
: LGTM: Enhanced map key naming improves code clarity.The addition of named keys for collections.Map instances improves code readability and maintainability. Consider adding a comment block explaining the purpose of each named key for better documentation.
Add documentation like this:
+// AccountsByType maps account addresses to their implementation types: +// - address: the account's address in bytes +// - type: the account's implementation type as a string AccountsByType: collections.NewMap(sb, AccountTypeKeyPrefix, "accounts_by_type", collections.BytesKey.WithName("address"), collections.StringValue.WithName("type")),
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
go.sum
is excluded by!**/*.sum
server/v2/stf/go.sum
is excluded by!**/*.sum
simapp/v2/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (27)
baseapp/abci.go
(1 hunks)baseapp/streaming.go
(7 hunks)codec/collections.go
(1 hunks)go.mod
(1 hunks)indexer/postgres/base_sql.go
(1 hunks)indexer/postgres/listener.go
(2 hunks)runtime/v2/app.go
(1 hunks)runtime/v2/go.mod
(1 hunks)schema/appdata/data.go
(2 hunks)schema/decoding/middleware.go
(1 hunks)server/v2/cometbft/abci.go
(3 hunks)server/v2/cometbft/abci_test.go
(3 hunks)server/v2/cometbft/go.mod
(1 hunks)server/v2/cometbft/oe/optimistic_execution_test.go
(1 hunks)server/v2/cometbft/server.go
(1 hunks)server/v2/cometbft/streaming.go
(3 hunks)server/v2/go.mod
(1 hunks)server/v2/stf/go.mod
(1 hunks)server/v2/stf/stf.go
(5 hunks)simapp/go.mod
(1 hunks)simapp/v2/app_di.go
(1 hunks)simapp/v2/go.mod
(3 hunks)tests/go.mod
(1 hunks)x/accounts/go.mod
(1 hunks)x/accounts/keeper.go
(1 hunks)x/accounts/module.go
(2 hunks)x/distribution/go.mod
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- server/v2/cometbft/server.go
- server/v2/stf/go.mod
🧰 Additional context used
📓 Path-based instructions (17)
runtime/v2/app.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
schema/decoding/middleware.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/accounts/module.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
tests/go.mod (1)
Pattern tests/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
codec/collections.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/stf/stf.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/cometbft/oe/optimistic_execution_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/appdata/data.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
baseapp/abci.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/cometbft/streaming.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/cometbft/abci_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"
server/v2/cometbft/abci.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
indexer/postgres/base_sql.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
indexer/postgres/listener.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/accounts/keeper.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
baseapp/streaming.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/app_di.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🔇 Additional comments (24)
indexer/postgres/listener.go (1)
85-87
: Consider splitting listener methods into separate files
To enhance code organization and maintainability, consider breaking out the OnTx
and OnEvent
implementations into separate files like tx.go
and event.go
.
baseapp/streaming.go (1)
232-243
: Handle transaction decoding errors appropriately
When decoding transactions, logging the error and proceeding might mask underlying issues. Consider handling the error explicitly, possibly by recording failed transactions separately or by failing the process if decoding is critical.
Provide a verification script to check how often transaction decoding fails:
server/v2/cometbft/abci.go (1)
541-543
: Pass all relevant transaction data to streamDeliverBlockChanges
Ensure that the decodedTxs
variable is correctly populated and passed to streamDeliverBlockChanges
to prevent potential nil pointer dereferences or missing transaction data in downstream processing.
Review the code to confirm that decodedTxs
is not nil and contains all necessary transaction information.
indexer/postgres/base_sql.go (2)
20-21
: Ensure consistency of NOT NULL
constraints in tx
table
The data
column is now NULL
, but it might be essential for certain queries. Verify if allowing NULL
values is intentional and won't impact data integrity.
32-34
: Clarify the necessity of allowing NULL
values in type
and data
columns
Allowing NULL
values in the type
and data
columns of the event
table may lead to incomplete event records. Confirm if this change is necessary and that it won't affect the integrity of event data.
x/accounts/module.go (1)
10-10
: LGTM: Required imports for schema handling
The new imports are necessary for implementing the schema.HasModuleCodec interface.
Also applies to: 14-14
server/v2/cometbft/streaming.go (1)
20-20
:
Verify length consistency between txs and decodedTxs
The function assumes that txs
and decodedTxs
slices have the same length, but this isn't validated.
Add length validation at the start of the function:
+if len(txs) != len(decodedTxs) {
+ return fmt.Errorf("mismatched lengths: txs=%d, decodedTxs=%d", len(txs), len(decodedTxs))
+}
schema/appdata/data.go (1)
34-35
: LGTM! Well-documented field additions
The BlockNumber field additions to both TxData and Event structs are well-documented and use appropriate types.
Also applies to: 59-60
server/v2/go.mod (1)
7-7
: LGTM: Schema module path correctly configured
The replace directive for the schema module is properly configured to use the local path.
simapp/v2/app_di.go (2)
7-7
: LGTM: PostgreSQL driver properly imported
The pgx driver is imported with a blank identifier, which is the correct pattern for SQL drivers.
13-13
: LGTM: PostgreSQL indexer registration
The postgres indexer is properly imported with a blank identifier for side-effect registration.
x/distribution/go.mod (1)
178-178
: LGTM: Schema module path correctly configured
The replace directive for the schema module is properly configured to use the local path, maintaining consistency with other modules.
server/v2/cometbft/go.mod (1)
6-6
: LGTM: Schema module replacement is correctly configured
The addition of cosmossdk.io/schema => ../../../schema
uses the correct relative path and maintains consistency with the module organization pattern.
go.mod (1)
189-189
: LGTM: Schema module replacement is properly configured at root level
The addition of cosmossdk.io/schema => ./schema
is correctly configured. However, note that this is part of the temporary replacements that will be removed once all modules have their own go.mods, as indicated by the TODO comment.
Let's verify the schema module's independence status:
✅ Verification successful
Schema module replacement is temporary but necessary
The schema module already has its own go.mod file, but it's intentionally kept dependency-free and on Go 1.12 for backward compatibility with all Cosmos SDK production releases. The root-level replacement is still needed temporarily to maintain build consistency while ensuring the module remains accessible for historical app patching.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the schema module has its own go.mod file
# Expected: Presence of go.mod would indicate progress toward module independence
fd -t f "^go.mod$" schema/
Length of output: 64
Script:
#!/bin/bash
# Let's check the content of schema/go.mod to verify if it's properly configured
cat schema/go.mod
Length of output: 300
codec/collections.go (1)
401-401
: LGTM: Setting BytesKind fields as nullable is correct.
This change ensures proper handling of byte fields in protobuf messages by marking them as nullable, aligning with protobuf's behavior where bytes can be nil/empty.
tests/go.mod (1)
290-290
: LGTM: Appropriate dependency management updates.
The addition of the schema module replace directive ensures consistent local development, while the keyring replacement aligns with the project's security practices.
simapp/go.mod (1)
255-256
: LGTM: Required dependencies for postgres indexer feature.
The addition of schema and postgres indexer replace directives properly supports the PR's objective of adding basic postgres indexer support.
Let's verify the postgres dependencies are properly configured:
✅ Verification successful
PostgreSQL dependencies are properly configured
The verification confirms that all required PostgreSQL dependencies are correctly set up:
- Core PostgreSQL driver (
github.com/jackc/pgx/v5
) is present in go.mod files - Required support packages (pgpassfile, pgservicefile) are properly included
- Driver is correctly imported with
_ "github.com/jackc/pgx/v5/stdlib"
in relevant files - Dependency versions are consistent across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify postgres-related dependencies
# Test: Check if all required postgres dependencies are present
rg -A 1 'github.com/jackc/pg'
Length of output: 5028
simapp/v2/go.mod (1)
303-303
: LGTM: Replace directives are correctly configured.
The replace directives for indexer/postgres
and schema
are properly set up to use local development paths, which is the expected configuration for development work.
Also applies to: 305-305
server/v2/stf/stf.go (2)
Line range hint 207-211
: LGTM: Consistent event enrichment in deliverTx.
The addition of BlockNumber and proper event staging in both validation and execution events improves traceability and debugging capabilities.
Also applies to: 218-221
378-381
: LGTM: Consistent block stage event handling.
The changes maintain consistency in setting BlockNumber and BlockStage across preBlock, beginBlock, and endBlock events, which enhances event tracking throughout the block lifecycle.
Also applies to: 396-399, 420-423
baseapp/abci.go (1)
1000-1003
: LGTM: Important safety check for block 1 commit failures
The added check prevents attempting to rollback to height 0 when a commit fails at block 1, which would be an invalid operation. This is a critical safety feature that ensures proper error handling during chain initialization.
x/accounts/go.mod (1)
181-181
: LGTM: Schema module replacement is correctly configured.
The replace directive for cosmossdk.io/schema
follows the established pattern for local module replacements.
server/v2/cometbft/abci_test.go (2)
14-14
: LGTM: Import organization is clean and clear.
The import statements are well-organized, with the new abci
import properly aliased and core imports appropriately positioned.
Also applies to: 22-22, 30-30
63-66
: LGTM: Generic type constraints are properly formatted.
The indentation of generic type constraints improves readability while maintaining the same functionality.
Also applies to: 93-96
attrsMap := map[string]interface{}{} | ||
for _, attr := range attrs { | ||
attrsMap[attr.Key] = attr.Value | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve all event attributes by handling duplicate keys appropriately
Currently, attributes with duplicate keys may overwrite previous entries due to the use of a map. This can lead to loss of data for events that have multiple attributes with the same key. To ensure all attributes are preserved, store them as a slice of key-value pairs instead.
Apply this diff to modify the handling of attributes:
-func (e *eventData) Attributes() ([]EventAttribute, error) {
- attrsMap := map[string]interface{}{}
- for _, attr := range attrs {
- attrsMap[attr.Key] = attr.Value
- }
- jsonAttributes, err = json.Marshal(attrsMap)
+func (e *eventData) Attributes() ([]EventAttribute, error) {
+ attrsSlice := []map[string]interface{}{}
+ for _, attr := range attrs {
+ attrsSlice = append(attrsSlice, map[string]interface{}{
+ "key": attr.Key,
+ "value": attr.Value,
+ })
+ }
+ jsonAttributes, err = json.Marshal(attrsSlice)
if err != nil {
return fmt.Errorf("failed to marshal event attributes: %w", err)
}
}
Committable suggestion skipped: line range outside the PR's diff.
decodedTxs, err := decodeTxs(c.logger, req.Txs, c.txCodec) | ||
if err != nil { | ||
return nil, nil, nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ineffective error handling in decodeTxs
function
The decodeTxs
function logs decoding errors but continues processing, which may lead to nil transactions being included in further processing, potentially causing panics. Ensure that the error handling either skips invalid transactions or fails gracefully.
Apply this diff to modify the error handling:
func decodeTxs[T transaction.Tx](logger log.Logger, rawTxs [][]byte, codec transaction.Codec[T]) ([]T, error) {
txs := make([]T, 0, len(rawTxs))
for _, rawTx := range rawTxs {
tx, err := codec.Decode(rawTx)
if err != nil {
- // do not return an error here, as we want to deliver the block even if some txs are invalid
- logger.Debug("failed to decode tx", "err", err)
+ // skip invalid transactions
+ logger.Error("failed to decode tx", "err", err)
+ continue
}
- txs[i] = tx
+ txs = append(txs, tx)
}
return txs, nil
}
|
||
for _, overrideKey := range a.config.OverrideStoreKeys { | ||
moduleSet[overrideKey.KvStoreKey] = moduleSet[overrideKey.ModuleName] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate module existence before applying overrides
The current implementation assumes that the module specified in overrideKey.ModuleName
exists in the moduleSet. We should validate this to prevent nil pointer dereferences.
for _, overrideKey := range a.config.OverrideStoreKeys {
+ module, exists := moduleSet[overrideKey.ModuleName]
+ if !exists {
+ return nil, fmt.Errorf("module %s not found for store key override %s",
+ overrideKey.ModuleName, overrideKey.KvStoreKey)
+ }
- moduleSet[overrideKey.KvStoreKey] = moduleSet[overrideKey.ModuleName]
+ moduleSet[overrideKey.KvStoreKey] = module
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
for _, overrideKey := range a.config.OverrideStoreKeys { | |
moduleSet[overrideKey.KvStoreKey] = moduleSet[overrideKey.ModuleName] | |
} | |
for _, overrideKey := range a.config.OverrideStoreKeys { | |
module, exists := moduleSet[overrideKey.ModuleName] | |
if !exists { | |
return nil, fmt.Errorf("module %s not found for store key override %s", | |
overrideKey.ModuleName, overrideKey.KvStoreKey) | |
} | |
moduleSet[overrideKey.KvStoreKey] = module | |
} |
// ModuleCodec implements `schema.HasModuleCodec` interface. | ||
// It allows the indexer to decode the module's KVPairUpdate. | ||
func (am AppModule) ModuleCodec() (schema.ModuleCodec, error) { | ||
return am.k.Schema.ModuleCodec(collections.IndexingOptions{}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding error handling for nil Schema
The current implementation assumes that the keeper's Schema is properly initialized. Consider adding a nil check to prevent potential panics.
func (am AppModule) ModuleCodec() (schema.ModuleCodec, error) {
+ if am.k.Schema == nil {
+ return schema.ModuleCodec{}, fmt.Errorf("schema not initialized for accounts module")
+ }
return am.k.Schema.ModuleCodec(collections.IndexingOptions{})
}
Committable suggestion skipped: line range outside the PR's diff.
BlockNumber: uint64(height), | ||
TxIndex: int32(i), | ||
Bytes: func() ([]byte, error) { return tx, nil }, | ||
JSON: func() (json.RawMessage, error) { | ||
return json.Marshal(decodedTxs[i]) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for JSON marshaling
The JSON marshaling operation could fail, but the error is not being handled properly. This could lead to silent failures.
Apply this fix:
JSON: func() (json.RawMessage, error) {
- return json.Marshal(decodedTxs[i])
+ if i >= len(decodedTxs) {
+ return nil, fmt.Errorf("index %d out of bounds for decodedTxs (len=%d)", i, len(decodedTxs))
+ }
+ return json.Marshal(decodedTxs[i])
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
BlockNumber: uint64(height), | |
TxIndex: int32(i), | |
Bytes: func() ([]byte, error) { return tx, nil }, | |
JSON: func() (json.RawMessage, error) { | |
return json.Marshal(decodedTxs[i]) | |
}, | |
BlockNumber: uint64(height), | |
TxIndex: int32(i), | |
Bytes: func() ([]byte, error) { return tx, nil }, | |
JSON: func() (json.RawMessage, error) { | |
if i >= len(decodedTxs) { | |
return nil, fmt.Errorf("index %d out of bounds for decodedTxs (len=%d)", i, len(decodedTxs)) | |
} | |
return json.Marshal(decodedTxs[i]) | |
}, |
@@ -43,9 +43,11 @@ require ( | |||
) | |||
|
|||
require ( | |||
cosmossdk.io/indexer/postgres v0.0.0-20241204160609-556102cfa046 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Critical: Update pgx dependency to patch security vulnerabilities
The codebase is using pgx v5.7.1 which is vulnerable to SQL injection attacks (CVE published on March 4, 2024). The current version is within the vulnerable range ">= 5.0.0, < 5.5.4".
- Update
github.com/jackc/pgx/v5
from v5.7.1 to at least v5.5.4 in:- simapp/go.mod
- simapp/v2/go.mod
- indexer/postgres/tests/go.mod
- tests/go.mod
This addresses two HIGH severity vulnerabilities:
- SQL Injection via Protocol Message Size Overflow
- SQL Injection via Line Comment Creation
🔗 Analysis chain
Verify PostgreSQL dependency versions for security vulnerabilities.
The PostgreSQL-related dependencies have been added:
github.com/jackc/pgx/v5
v5.7.1github.com/jackc/pgpassfile
v1.0.0github.com/jackc/pgservicefile
v0.0.0-20240606120523-5a60cdf6a761github.com/jackc/puddle/v2
v2.2.2
Also applies to: 50-50, 161-163
Let me verify the exact version of pgx being used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for security advisories in PostgreSQL-related dependencies
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/jackc/pgx") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 1001
Script:
#!/bin/bash
# Check the exact version of pgx in go.mod files
rg "github.com/jackc/pgx/v5" -A 2
Length of output: 3044
event_index BIGINT NOT NULL, | ||
type TEXT NULL, | ||
data JSONB NULL, | ||
attributes JSONB NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should store both data and attributes. I think if an event has Data
, we should use that. If it doesn't have data and just has Attributes
we should store that in data
as JSON
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically I think we just need one data
column, otherwise it's a lot of duplicated data in most cases
Description
Related to epic #20352
Missing:
Author 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.md
Reviewers 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
Improvements
Dependency Updates