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 issue #838 make sure we don't get columns that are dropped #840

Merged
merged 4 commits into from Jun 12, 2017

Conversation

Projects
None yet
4 participants
@davecramer
Member

davecramer commented Jun 7, 2017

No description provided.

@vlsi

This comment has been minimized.

Member

vlsi commented Jun 7, 2017

@davecramer , the key part is to add a test.

@davecramer

This comment has been minimized.

Member

davecramer commented Jun 7, 2017

@vlsi once confirmed that this works, I will add the test

apparently if causes many of the current tests to fail anyway :(

@vlsi

This comment has been minimized.

Member

vlsi commented Jun 7, 2017

once confirmed that this works, I will add the test

My initial thought was to trick @emesika into adding a test :)

@vlsi vlsi added this to the 42.1.2 milestone Jun 7, 2017

@codecov-io

This comment has been minimized.

codecov-io commented Jun 7, 2017

Codecov Report

Merging #840 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #840      +/-   ##
============================================
+ Coverage     65.56%   65.59%   +0.03%     
- Complexity     3545     3551       +6     
============================================
  Files           166      166              
  Lines         15250    15254       +4     
  Branches       2475     2475              
============================================
+ Hits           9998    10006       +8     
+ Misses         4074     4069       -5     
- Partials       1178     1179       +1
@emesika

This comment has been minimized.

emesika commented Jun 7, 2017

Hi
I cloned the project and tested the suggested fix that is targeted to 42.1.2
The fix solves the issue
I will wait for the official release ...
Thanks
Eli Mesika

@davecramer

This comment has been minimized.

Member

davecramer commented Jun 7, 2017

@emesika guess we can't convince you to create a test case for it ?

/* getFunctionColumns also has to be aware of dropped columns
add this in here to make sure it can deal with them
*/
rs = dbmd.getFunctionColumns(null, null, "f2", null);

This comment has been minimized.

@vlsi

vlsi Jun 8, 2017

Member

Does this reproduce the problem?
I thought the function should be somehow relate to the table.
As far as I see, f2 is defined as f2(a int, b varchar) RETURNS int, so I see no way it is related with TABLE metadatatest

@davecramer

This comment has been minimized.

Member

davecramer commented Jun 8, 2017

@davecramer davecramer merged commit 464a2d4 into pgjdbc:master Jun 12, 2017

2 checks passed

codecov/project 65.59% (+0.03%) compared to 2d0bfce
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

davecramer added a commit to davecramer/pgjdbc that referenced this pull request Sep 19, 2017

fix issue pgjdbc#838 make sure we don't get columns that are dropped (p…
…gjdbc#840)

* fix issue pgjdbc#838 make sure we don't get columns that are dropped

* added test case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment