Skip to content

fix: initialize catalog from connection properties#320

Merged
guanshengliang merged 2 commits into
mainfrom
fix/6980196872
May 15, 2026
Merged

fix: initialize catalog from connection properties#320
guanshengliang merged 2 commits into
mainfrom
fix/6980196872

Conversation

@sheyanjie-qq
Copy link
Copy Markdown
Contributor

@sheyanjie-qq sheyanjie-qq commented May 15, 2026

Description

initialize catalog from connection properties

Issue(s)

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 15, 2026 09:03
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request initializes the catalog field in the AbstractConnection constructor using the database name property and adds corresponding unit tests. The reviewer pointed out that this change introduces redundancy and potential state inconsistency because several subclasses maintain their own local database fields. It is recommended to refactor these subclasses to rely solely on the inherited catalog field from AbstractConnection.

Comment thread src/main/java/com/taosdata/jdbc/AbstractConnection.java
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR initializes Connection.getCatalog() from the incoming connection Properties (specifically TSDBDriver.PROPERTY_KEY_DBNAME) so a newly created connection reflects the configured database/catalog immediately.

Changes:

  • Set AbstractConnection.catalog from properties.getProperty(TSDBDriver.PROPERTY_KEY_DBNAME) during construction.
  • Refactor AbstractConnectionTest setup to use a helper constructor method.
  • Add unit tests verifying catalog initialization from properties and the null case when the property is absent.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/main/java/com/taosdata/jdbc/AbstractConnection.java Initialize catalog from connection properties during AbstractConnection construction.
src/test/java/com/taosdata/jdbc/AbstractConnectionTest.java Add tests ensuring getCatalog() reflects dbname from properties (or null when missing).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.13%. Comparing base (193a03f) to head (06cdf74).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #320      +/-   ##
============================================
- Coverage     80.18%   80.13%   -0.06%     
+ Complexity     4465     4463       -2     
============================================
  Files           227      227              
  Lines         13491    13492       +1     
  Branches       1797     1797              
============================================
- Hits          10818    10812       -6     
- Misses         1754     1759       +5     
- Partials        919      921       +2     
Flag Coverage Δ
unittests 80.13% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@zyyang90 zyyang90 left a comment

Choose a reason for hiding this comment

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

LGTM

@guanshengliang guanshengliang merged commit b16fb9f into main May 15, 2026
13 of 14 checks passed
Sign up for free to 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.

6 participants