Add support for SAP HANA. - #81
Conversation
WalkthroughAdds SAP HANA support by introducing a new hdb package and routing scheme "hdb" in database Connect; updates module dependencies; connector validation now performs a real DB PingContext; removes explicit MySQL pool configuration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant DB as pkg/database.Connect
participant HDB as pkg/database/hdb.Connect
participant SQL as database/sql.Open
Caller->>DB: Connect(ctx, dsn "hdb://...")
DB->>DB: parse scheme == "hdb"
DB->>HDB: Connect(ctx, dsn)
HDB->>SQL: sql.Open("hdb", dsn)
SQL-->>HDB: (*sql.DB or error)
HDB-->>DB: (*sql.DB, nil) or (nil, err)
alt success
DB-->>Caller: (*sql.DB, HDB, nil)
else error
DB-->>Caller: (nil, HDB, err)
end
sequenceDiagram
autonumber
actor Service
participant Connector as pkg/connector.Validate
participant DB as *sql.DB
Service->>Connector: Validate(ctx)
Connector->>DB: db.PingContext(ctx)
alt ping ok
DB-->>Connector: nil
Connector-->>Service: nil
else ping fail
DB-->>Connector: error
Connector-->>Service: error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![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
🧹 Nitpick comments (1)
pkg/database/hdb/hdb.go (1)
17-27: Use the context to validate the HANA connection
Connectignoresctxand will happily return a pool even if the DSN is wrong or the host is unavailable. Ping with the supplied context (and close on failure) so we fail fast and respect cancellation.func Connect(ctx context.Context, dsn string) (*sql.DB, error) { db, err := sql.Open("hdb", dsn) if err != nil { return nil, err } db.SetMaxOpenConns(MaxOpenConns) db.SetMaxIdleConns(MaxIdleConns) db.SetConnMaxLifetime(MaxConnLifetime) + + if err := db.PingContext(ctx); err != nil { + _ = db.Close() + return nil, err + } return db, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (86)
go.sumis excluded by!**/*.sumand included by nonevendor/github.com/SAP/go-hdb/LICENSE.mdis excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/bytes.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/calldriver.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/conn.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/connector.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/convert.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/dbconn.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/dbconnectinfo.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/decimal.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/deprecated.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/dial/dialer.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/doc.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/driver.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/dsn.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/error.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/identifier.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/auth.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/certkey.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/jwt.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/list.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/scram.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/scrampbkdf2sha256.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/scramsha256.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/sessioncookie.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/auth/x509.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/convert.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/datatype.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/decode.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/decodeerror.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/dfv.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/doc.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/datetime.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/decimal.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/decode.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/doc.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/encode.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/encoding/field.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/error.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/fieldnames.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/functioncode.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/headers.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/init.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/julian/julian.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/keyvaluesparts.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/levenshtein/levenshtein.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/lob.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/messagetype.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/optionsparts.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/optiontype.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/parameter.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/partkind.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/parts.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/parts1.24.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/parts1.25.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/protocol.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/resizeslice.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/result.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/rowsaffected.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/simpleparts.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/typecode.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/x_generator.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/protocol/x_stringer.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/rand/alphanum/rand.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/internal/unsafe/unsafe.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/lob.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/metadata.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/metrics.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/result.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/scanner.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/session.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/sniffer.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/stats.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/stats.tmplis excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/statscfg.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/statscfg.jsonis excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/stmt.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/trace.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/unicode/cesu8/cesu8.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/unicode/cesu8/encoding.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/version.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/wgroup/wgroup1.24.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/wgroup/wgroup1.25.gois excluded by!vendor/**and included by nonevendor/github.com/SAP/go-hdb/driver/x_bstring_test.pyis excluded by!vendor/**and included by nonevendor/modules.txtis excluded by!vendor/**and included by none
📒 Files selected for processing (3)
go.mod(3 hunks)pkg/database/database.go(3 hunks)pkg/database/hdb/hdb.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Error handling: usefmt.Errorfwith contextual messages; check specific errors witherrors.Is
Organize imports: standard library first, then third-party, then project imports; alphabetize within each group
Naming: CamelCase for exported identifiers; camelCase for unexported; preserve acronyms like ID, URL, HTTP, API
Limit line length to a maximum of 200 characters
Comments for exported items must be complete sentences ending with periods
Do not uselog.Fatalorlog.Panic(ruleguard-enforced)
Files:
pkg/database/hdb/hdb.gopkg/database/database.go
🧬 Code graph analysis (2)
pkg/database/hdb/hdb.go (1)
pkg/database/database.go (1)
Connect(53-121)
pkg/database/database.go (1)
pkg/database/hdb/hdb.go (1)
Connect(17-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: test
- GitHub Check: go-lint
- GitHub Check: go-test (ubuntu-latest)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docker-compose-hanaexpress-test.ymlis excluded by none and included by noneexamples/sap-hana-test.ymlis excluded by none and included by nonetest/hanaexpress/password.jsonis excluded by none and included by none
📒 Files selected for processing (1)
pkg/connector/connector.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Error handling: usefmt.Errorfwith contextual messages; check specific errors witherrors.Is
Organize imports: standard library first, then third-party, then project imports; alphabetize within each group
Naming: CamelCase for exported identifiers; camelCase for unexported; preserve acronyms like ID, URL, HTTP, API
Limit line length to a maximum of 200 characters
Comments for exported items must be complete sentences ending with periods
Do not uselog.Fatalorlog.Panic(ruleguard-enforced)
Files:
pkg/connector/connector.go
🪛 GitHub Actions: ci
pkg/connector/connector.go
[error] 79-79: golangci-lint: (*database/sql.DB).Ping must not be called. use (*database/sql.DB).PingContext (noctx) at connector.go:79
🪛 GitHub Check: go-lint
pkg/connector/connector.go
[failure] 79-79:
(*database/sql.DB).Ping must not be called. use (*database/sql.DB).PingContext (noctx)
| err := c.db.Ping() | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Use PingContext instead of Ping and add error context.
The linter correctly flags that Ping() must not be called. Use PingContext(ctx) to properly propagate the context parameter. Additionally, per the coding guidelines, wrap the error with fmt.Errorf to provide contextual information.
Apply this diff:
- err := c.db.Ping()
+ err := c.db.PingContext(ctx)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("failed to ping database: %w", err)
}📝 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.
| err := c.db.Ping() | |
| if err != nil { | |
| return nil, err | |
| } | |
| err := c.db.PingContext(ctx) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to ping database: %w", err) | |
| } |
🧰 Tools
🪛 GitHub Actions: ci
[error] 79-79: golangci-lint: (*database/sql.DB).Ping must not be called. use (*database/sql.DB).PingContext (noctx) at connector.go:79
🪛 GitHub Check: go-lint
[failure] 79-79:
(*database/sql.DB).Ping must not be called. use (*database/sql.DB).PingContext (noctx)
🤖 Prompt for AI Agents
In pkg/connector/connector.go around lines 79 to 82, replace the blocking
c.db.Ping() call with c.db.PingContext(ctx) so the operation respects
cancellation/timeout, and wrap any returned error with fmt.Errorf to add context
(e.g. "pinging database: %w"); also ensure the function has access to a ctx
variable and add/import "fmt" if not already present.
…can't ping the database.
d9827f9 to
78c0a61
Compare
| const ( | ||
| MaxIdleConns = 10 | ||
| MaxOpenConns = 10 | ||
| MaxConnLifetime = 5 * time.Minute |
There was a problem hiding this comment.
im sligtly confused by these constants being here. i understand other packages might do this, but asking as we had more drivers.... seems like a wrong pattern hash-tag-bridge-too-far
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/database/hdb/hdb.go(1 hunks)pkg/database/mysql/mysql.go(0 hunks)
💤 Files with no reviewable changes (1)
- pkg/database/mysql/mysql.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Error handling: usefmt.Errorfwith contextual messages; check specific errors witherrors.Is
Organize imports: standard library first, then third-party, then project imports; alphabetize within each group
Naming: CamelCase for exported identifiers; camelCase for unexported; preserve acronyms like ID, URL, HTTP, API
Limit line length to a maximum of 200 characters
Comments for exported items must be complete sentences ending with periods
Do not uselog.Fatalorlog.Panic(ruleguard-enforced)
Files:
pkg/database/hdb/hdb.go
🧬 Code graph analysis (1)
pkg/database/hdb/hdb.go (2)
pkg/database/mysql/mysql.go (1)
Connect(47-59)pkg/database/database.go (1)
Connect(53-121)
🔇 Additional comments (2)
pkg/database/hdb/hdb.go (2)
1-8: LGTM! Import organization follows guidelines.The package declaration and imports are correctly organized with standard library imports first, followed by third-party imports. The blank import for the HDB driver registration is the standard pattern for
database/sqldrivers.
10-10: Context parameter is unused but consistent with other database packages.The
ctxparameter is declared but not used within the function body. Whilesql.Opendoes not accept a context, this signature is consistent with other database packageConnectfunctions in the codebase (e.g.,mysql.Connect,oracle.Connect). This appears to be an intentional design choice for API consistency and potential future use.If context usage is planned for future enhancements (e.g., connection timeouts, cancellation during driver initialization), consider documenting this intent in the function comment.
| func Connect(ctx context.Context, dsn string) (*sql.DB, error) { | ||
| db, err := sql.Open("hdb", dsn) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return db, nil | ||
| } |
There was a problem hiding this comment.
Add documentation for the exported function.
The exported Connect function is missing a documentation comment. Per coding guidelines, all exported items must have complete sentence comments ending with periods.
Apply this diff to add documentation:
+// Connect establishes a connection to a SAP HANA database using the provided DSN.
+// It returns a configured sql.DB instance or an error if the connection cannot be opened.
func Connect(ctx context.Context, dsn string) (*sql.DB, error) {
db, err := sql.Open("hdb", dsn)
if err != nil {
return nil, err
}
return db, nil
}📝 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.
| func Connect(ctx context.Context, dsn string) (*sql.DB, error) { | |
| db, err := sql.Open("hdb", dsn) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return db, nil | |
| } | |
| // Connect establishes a connection to a SAP HANA database using the provided DSN. | |
| // It returns a configured sql.DB instance or an error if the connection cannot be opened. | |
| func Connect(ctx context.Context, dsn string) (*sql.DB, error) { | |
| db, err := sql.Open("hdb", dsn) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return db, nil | |
| } |
🤖 Prompt for AI Agents
In pkg/database/hdb/hdb.go around lines 10 to 17, the exported Connect function
lacks a documentation comment; add a complete sentence comment immediately above
the function that describes what Connect does, mentions the ctx and dsn
parameters (context for cancellation/timeout and Data Source Name), and
summarizes the return values (a *sql.DB on success or an error on failure), and
ensure the sentence ends with a period.
2 participants
Description
Useful links:
Note
Adds SAP HANA support via
hdbdriver and connection ping validation, with test docker config, example config, and dependency updates.hdbengine integration.pkg/database/hdbwithConnectusingSAP/go-hdb.database.Connectto handlehdb://DSNs.Connector.Validatenow pings the DB (db.Ping()).docker-compose-hanaexpress-test.ymlandexamples/sap-hana-test.ymlfor local HANA testing.github.com/SAP/go-hdband updategolang.org/x/*versions.test/hanaexpress/password.json.Written by Cursor Bugbot for commit 78c0a61. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Chores