fix(postgresql): support schemas in base store#2273
Conversation
There was a problem hiding this comment.
Pull request overview
Adds optional PostgreSQL schema support to PostgresBaseStore while keeping the existing unqualified-table behavior when no schema is configured. This aligns PostgresBaseStore more closely with other PostgreSQL-backed stores in the extensions module and addresses #2192.
Changes:
- Add
PostgresBaseStore.Builder#schemaName(String)and build schema-qualified, quoted identifiers when configured. - Create the schema (if configured) before creating the table when
initializeSchema(true)is enabled. - Add tests for schema-qualified SQL, schema initialization, and invalid schema names.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| agentscope-extensions/agentscope-extensions-postgresql/src/main/java/io/agentscope/extensions/postgresql/store/PostgresBaseStore.java | Adds schema configuration, qualified table naming, schema creation on initialization, and shared identifier validation. |
| agentscope-extensions/agentscope-extensions-postgresql/src/test/java/io/agentscope/extensions/postgresql/store/PostgresBaseStoreTest.java | Adds unit tests covering schema-qualified SQL usage, schema creation, and schema name validation. |
| this.selectSql = String.format(SELECT_SQL, qualifiedTableName); | ||
| this.upsertSql = String.format(UPSERT_SQL, qualifiedTableName); | ||
| this.insertSql = String.format(INSERT_SQL, qualifiedTableName); |
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR adds optional PostgreSQL schema support to PostgresBaseStore while preserving backward compatibility when no schema is configured. The implementation is clean and well-structured: it introduces a schemaName builder parameter, properly validates identifiers to prevent SQL injection, and generates correctly-quoted schema-qualified table names ("schema"."table"). The test coverage is good, covering schema-qualified queries, schema initialization, and invalid identifier rejection. However, there's one concern about the ON CONFLICT SET clause behavior with schema-qualified names that should be verified against a real PostgreSQL database, as the current tests use mocks.
(inline comments could not be attached — line numbers fell outside PR hunks. See archived report.)
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR adds optional PostgreSQL schema support to PostgresBaseStore while preserving backward compatibility when no schema is configured. The implementation is clean and well-structured: it introduces a schemaName builder parameter, properly validates identifiers to prevent SQL injection, and generates correctly-quoted schema-qualified table names ("schema"."table"). The test coverage is good, covering schema-qualified queries, schema initialization, and invalid identifier rejection. However, there's one concern about the ON CONFLICT SET clause behavior with schema-qualified names that should be verified against a real PostgreSQL database, as the current tests use mocks.
(inline comments could not be attached — line numbers fell outside PR hunks. See archived report.)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
This PR contains 205 lines of changes. Review in progress.
Automated review by github-manager-bot
AgentScope-Java Version
2.0.1-SNAPSHOT
Description
Closes #2192.
PostgresBaseStorepreviously only supported unqualified table names. This PR adds optional PostgreSQL schema support while preserving the existing behavior when no schema is configured.Changes:
PostgresBaseStore.Builder#schemaName(String).schema.tableidentifiers for all BaseStore SQL when a schema is configured.initializeSchema(true)is enabled.Testing:
mvn -q -pl agentscope-extensions/agentscope-extensions-postgresql -am -Dtest=PostgresBaseStoreTest -Dsurefire.failIfNoSpecifiedTests=false testChecklist
mvn test)