-
Notifications
You must be signed in to change notification settings - Fork 438
Use sys.all_objects for accurate function and procedure filtering #2705
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
Conversation
…give correct results post filtering
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2705 +/- ##
============================================
- Coverage 51.60% 51.56% -0.05%
- Complexity 4083 4091 +8
============================================
Files 149 149
Lines 34242 34268 +26
Branches 5719 5725 +6
============================================
- Hits 17672 17669 -3
- Misses 14104 14127 +23
- Partials 2466 2472 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDatabaseMetaData.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDatabaseMetaData.java
Outdated
Show resolved
Hide resolved
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.
Implementation looks good. Tests need a little tweaking, though. We don't need to print all that stuff to the logs.
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
Outdated
Show resolved
Hide resolved
PR approved. We can perform manual testing against a SQL DW instance using JDBC_DW. |
Description
This PR addresses GitHub reported issue : DatabaseMetadata.getProcedures and getFunctions return both functions and procedures #2690
Both
DatabaseMetadata.getProcedures
andDatabaseMetadata.getFunctions
were returning a result set that included both stored procedures and functions, instead of filtering to return only the relevant object type for each method. This caused confusion, as users expect the results of each method to be exclusive to procedures or functions.Currently, getFunctions() and getProcedures() both call the sp_stored_procedures. This issue originates from how the server handles sp_stored_procedures. It always returns the procedure type as 2 (sp-stored-procedures-transact-sql), regardless of whether the object is a function or a procedure.
Fix
The implementation has been updated so that
getProcedures()
andgetFunctions()
now usesys.all_objects
to accurately filter database objects. Additionally, for procedures type is returned as 1 and 2 for functions.Specifically, procedures are identified by object types
And functions are identified by
This ensures that each method returns only the objects relevant to its purpose, resolving the confusion caused by the previous behavior.
Testing
Manual testing by creating scripts to validate objects (functions and procedures) obtained from getFunctions() and getProcedures() using sys.all_objects matches the result obtained from sp_stored_procedures. This testing was done locally