Skip to content
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

fix: sort result set by sql type value in metadata typeinfo #910

Closed
wants to merge 3 commits into from

Conversation

zemian
Copy link
Contributor

@zemian zemian commented Aug 5, 2017

fixes #716

I don't see a good way to sort the type from backend query, so I provide a solution that sort the result after the query. Let me know if this is good for you or not.

@codecov-io
Copy link

codecov-io commented Aug 5, 2017

Codecov Report

Merging #910 into master will decrease coverage by 0.01%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master     #910      +/-   ##
============================================
- Coverage     65.85%   65.83%   -0.02%     
+ Complexity     3546     3545       -1     
============================================
  Files           166      166              
  Lines         15230    15236       +6     
  Branches       2470     2471       +1     
============================================
+ Hits          10029    10031       +2     
- Misses         4022     4026       +4     
  Partials       1179     1179

@Override
public int compare(byte[][] o1, byte[][] o2) {
try {
int i1 = Integer.parseInt(connection.getEncoding().decode(o1[1]));
Copy link
Member

Choose a reason for hiding this comment

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

Would you please perform parseInt/decode stuff outside of Comparator to avoid repeated decode/parse in the sort loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @vlsi, The decode part is needed in the loop because we need the byte[1] "sqlType" as input byte. Moving it outside means I have to create new list of byte array and associate the origingal sqlType int value as one element. And again to remove the int sqlType as final result after the sorting for return value. Can you please confirm is that what you like to see?

@GuntherRademacher
Copy link

This does not completely fix #716, because it does not honor the second criterion given in the spec (see my comment).

@davecramer
Copy link
Member

@GuntherRademacher so exactly what are the orderings then ? This seems overly pedantic to me

@GuntherRademacher
Copy link

Call it pedantic if you want. The ordering constraints are (listed here):

They are ordered by DATA_TYPE and then by how closely the data type maps to the corresponding JDBC SQL type.

The proposed fix ignores the second criterion, which is important, as it provides the ranking information that cannot be derived otherwise. Compared to that, the first criterion is trivial, as it can be restored from the result itself.

I stumbled on this while writing code to find the best fit for mapping JDBC types to vendor-specific type names, for different databases. AFAICS Oracle, MySQL, and Derby are doing it right. A corresponding SQL Server bug is described here.

@davecramer
Copy link
Member

davecramer commented Jan 5, 2018 via email

@yan-hic
Copy link

yan-hic commented Jun 14, 2019

Any update on this ?

Can a rank (set with CASE WHEN... THEN) not be used to order by sql.Type first, then by 'prefered' typname e.g. 12-VARCHAR before 12-NAME

davecramer added a commit to davecramer/pgjdbc that referenced this pull request Jun 17, 2019
…ed in Oracle documentation

this replaces PR pgjdbc#910 as it was stale and easier to replace than fix. Original work by @zemian
davecramer added a commit that referenced this pull request Nov 12, 2019
…ed in Oracle documentation (#1506)

this replaces PR #910 as it was stale and easier to replace than fix. Original work by @zemian
@davecramer
Copy link
Member

fixed in #1506

@davecramer davecramer closed this Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants