Skip to content

Document JDBC connection properties - #640

Merged
jonathanpeppers merged 5 commits into
mainfrom
jonathanpeppers-document-java-sql-apis-70f
Aug 18, 2026
Merged

Document JDBC connection properties#640
jonathanpeppers merged 5 commits into
mainfrom
jonathanpeppers-document-java-sql-apis-70f

Conversation

Refs #189
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI lite review requested due to automatic review settings August 17, 2026 13:33
@jonathanpeppers
jonathanpeppers enabled auto-merge (squash) August 17, 2026 13:33

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

@dalexsoto review

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two property contracts need correction:

  • TransactionIsolation conflates getter and setter domains. The getter may return TransactionNone, but assigning TRANSACTION_NONE is invalid and must throw SQLException; distinguish readable results from setter-valid values.
  • Holdability and TransactionIsolation use Java identifiers such as ResultSet.HOLD_CURSORS_OVER_COMMIT and Connection.TRANSACTION_READ_COMMITTED, which do not exist in the managed API. Replace them with managed references such as IResultSet.HoldCursorsOverCommit and IConnection.TransactionReadCommitted.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f7bb4d04-d38f-491b-bca9-805acbe0ccf3
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the review in 910fc74: Holdability and TransactionIsolation now link to the managed IResultSet/IConnection constants, and the transaction isolation docs distinguish getter results from setter-valid values (TransactionNone is getter-only and rejected by the setter).

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previously reported constant and setter-domain issues are fixed, but two managed-contract problems remain:

  • TypeMap still names Java's java.util.Map instead of the exposed managed IDictionary<string, Java.Lang.Class> type.
  • The five transaction-isolation altmember links near TransactionIsolation target obsolete Java.Sql.Connection constants that direct users to IConnection.

Please use the managed type/cref and point all related constants to Java.Sql.IConnection.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f7bb4d04-d38f-491b-bca9-805acbe0ccf3
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the remaining managed-contract feedback in dabfc95: TypeMap now links to and displays IDictionary<string, Java.Lang.Class>, and all five transaction-isolation altmember links now target Java.Sql.IConnection constants.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The managed TypeMap type and transaction constant links are fixed, but the TypeMap contract still implies the returned dictionary is directly associated with the connection. JDBC requires callers to assign the map back after modification because drivers may retain an internal copy. Please document that reassignment/setter requirement and link the setter source alongside the getter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f7bb4d04-d38f-491b-bca9-805acbe0ccf3
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the latest TypeMap contract feedback in ba1565d: the docs now require assigning a modified dictionary back to the property because drivers may retain an internal copy, and they link both the JDBC getter and setter sources.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TypeMap reassignment contract is fixed, but the metadata property now names Java's DatabaseMetaData, which does not exist in the managed API. The generated signature returns Java.Sql.IDatabaseMetaData?; please reference the managed interface, preferably with <see cref="T:Java.Sql.IDatabaseMetaData" />.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f7bb4d04-d38f-491b-bca9-805acbe0ccf3
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the metadata feedback in f7e5efd: the MetaData value documentation now references the managed Java.Sql.IDatabaseMetaData interface via a resolvable cref.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All JDBC property contracts now use valid managed types and constants, including TypeMap reassignment and IDatabaseMetaData.

@jonathanpeppers
jonathanpeppers merged commit 6d531ba into mainAug 18, 2026
3 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-document-java-sql-apis-70f branch August 18, 2026 17:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jonathanpeppers@dalexsoto