Rework sql type gathering to use OID instead of typname. #1949
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There is a small fix required to appease checkstyle, otherwise looks fine. I would like a test where the a type of the same name is created in a different schema just for completeness |
MMeent
force-pushed
the
fix/issue-1948-off-path-type-discovery
branch
from
14fc3cc
to
c9e274e
Compare
|
I've cleaned up the code a bit, and made it into one commit that follows the commit style guide. |
Codecov Report
@@ Coverage Diff @@
## master #1949 +/- ##
============================================
+ Coverage 65.35% 69.33% +3.97%
- Complexity 3955 4226 +271
============================================
Files 197 197
Lines 18006 18012 +6
Branches 2919 2920 +1
============================================
+ Hits 11768 12488 +720
+ Misses 4878 4167 -711
+ Partials 1360 1357 -3 |
|
"Special has been given to the oid of Oid.UNSPECIFIED, as that needs to be mapped to Types.OTHER without needing to query the database." Can you fix this up ? |
|
What do you mean with 'fix up'? Should I remove it from the commit message, or did you mean something else? |
|
It doesn't read properly I presume you mean Special consideration ? |
…es not have the issue of name shadowing / qual-names, and has the added benefit of fixing pgjdbc#1948. Types that are not on the search path (e.g. they are shadowed, or in a schema that is not included in the search path) are stored in the caches as fully qualified type names and OIDs. As we cannot easily query the pg_type catalog using qualified type names, we replace the pgTypeName with the oid of the type name to query properties of the type. Testcases are added to improve coverage of correctly detecting SQL types that are not on the path, but are available through OID or qualified lookup. These types are stored internally as a fully qualified type, but we cannot use this name for lookup in pg_type. Special consideration has been given to Oid.UNSPECIFIED, as that needs to be mapped to Types.OTHER without first hitting the database. That mapping is static, but is not in `types` because it is not an actual type. Fixes pgjdbc#1948
MMeent
force-pushed
the
fix/issue-1948-off-path-type-discovery
branch
from
c9e274e
to
d56817f
Compare
|
Ah, yes, I indeed messed that up, and read over that mistake thrice already? I believe that's fixed. |
|
@davecramer I wouldn't want to come across as impatient, but at what timeframe could I expect this to be released? Is there some release schedule I could lookup for pgjdbc? |
|
I wouldn't expect to see a release before the end of the year. I know I'm going to be pinned, not sure about @vlsi ? |
davecramer
added a commit
to davecramer/pgjdbc
that referenced
this pull request
Jul 5, 2021
Fix: Rework sql type gathering to use OID instead of typname. This do… …es not have the issue of name shadowing / qual-names, and has the added benefit of fixing pgjdbc#1948. Types that are not on the search path (e.g. they are shadowed, or in a schema that is not included in the search path) are stored in the caches as fully qualified type names and OIDs. As we cannot easily query the pg_type catalog using qualified type names, we replace the pgTypeName with the oid of the type name to query properties of the type. Testcases are added to improve coverage of correctly detecting SQL types that are not on the path, but are available through OID or qualified lookup. These types are stored internally as a fully qualified type, but we cannot use this name for lookup in pg_type. Special consideration has been given to Oid.UNSPECIFIED, as that needs to be mapped to Types.OTHER without first hitting the database. That mapping is static, but is not in `types` because it is not an actual type. Fixes pgjdbc#1948
davecramer
added a commit
that referenced
this pull request
Jul 5, 2021
Fix: Rework sql type gathering to use OID instead of typname. This does not have the issue of name shadowing / qual-names, and has the added benefit of fixing #1948. Types that are not on the search path (e.g. they are shadowed, or in a schema that is not included in the search path) are stored in the caches as fully qualified type names and OIDs. As we cannot easily query the pg_type catalog using qualified type names, we replace the pgTypeName with the oid of the type name to query properties of the type. Testcases are added to improve coverage of correctly detecting SQL types that are not on the path, but are available through OID or qualified lookup. These types are stored internally as a fully qualified type, but we cannot use this name for lookup in pg_type. Special consideration has been given to Oid.UNSPECIFIED, as that needs to be mapped to Types.OTHER without first hitting the database. That mapping is static, but is not in `types` because it is not an actual type. Fixes #1948
6 tasks
|
So this has caused a fair bit of a regression as we are now querying the database for almost every type. Changes startup times considerably see issue #2236 |
davecramer
added a commit
to davecramer/pgjdbc
that referenced
this pull request
Sep 21, 2021
… all types by OID, we can return types for well known types
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is more stable, and has the added benefit of fixing #1948.
I'd like to remove the pgNameToSQLType map too, but that's used for the public api of getPGTypeNamesWithSQLTypes() - I'm not certain I can just remove that and be done with it...
All Submissions:
Changes to Existing Features: