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

don't attempt to read a SQLXML more than once (#964) #965

Merged
merged 5 commits into from Oct 25, 2017
Merged

don't attempt to read a SQLXML more than once (#964) #965

merged 5 commits into from Oct 25, 2017

Conversation

@bbodnar
Copy link
Contributor

@bbodnar bbodnar commented Sep 27, 2017

The case is clear: the specification says, calling getString() "consumes" the data and cannot be done multiple times on the same object.

Here a simple fix.

@davecramer
Copy link
Member

@davecramer davecramer commented Sep 27, 2017

Seems reasonable. I presume it fixes your problem ?

@bbodnar
Copy link
Contributor Author

@bbodnar bbodnar commented Sep 27, 2017

Yes, it fixes my problem.
I don't think it could have any negative side-effect.

@jorsol
Copy link
Member

@jorsol jorsol commented Sep 27, 2017

@bbodnar , please remove the Trailing whitespace at the end of xmlObject.getString(); and add spaces around "==" in (stringValue==null) to make Checkstyle happy.

@jorsol
jorsol approved these changes Sep 27, 2017
Copy link
Member

@jorsol jorsol left a comment

LGTM

Copy link
Member

@vlsi vlsi left a comment

Would you please add a test case?

@codecov-io
Copy link

@codecov-io codecov-io commented Oct 19, 2017

Codecov Report

Merging #965 into master will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master     #965      +/-   ##
============================================
+ Coverage     65.92%   65.93%   +0.01%     
- Complexity     3561     3568       +7     
============================================
  Files           166      167       +1     
  Lines         15238    15271      +33     
  Branches       2464     2470       +6     
============================================
+ Hits          10045    10069      +24     
- Misses         4022     4034      +12     
+ Partials       1171     1168       -3
@davecramer
Copy link
Member

@davecramer davecramer commented Oct 24, 2017

@bbodnar are you able to find time to add a test case ?

@bbodnar
Copy link
Contributor Author

@bbodnar bbodnar commented Oct 24, 2017

I will add a test case in the next days.

… multiple times)
@davecramer
Copy link
Member

@davecramer davecramer commented Oct 24, 2017

@bbodnar checkstyle is not happy:
[ERROR] /home/travis/build/pgjdbc/pgjdbc/pgjdbc/src/test/java/org/postgresql/test/jdbc4/XmlTest.java:265:15: WhitespaceAround: '=' is not preceded with whitespace. [WhitespaceAround]
[ERROR] /home/travis/build/pgjdbc/pgjdbc/pgjdbc/src/test/java/org/postgresql/test/jdbc4/XmlTest.java:265:16: WhitespaceAround: '=' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3) [WhitespaceAround]
[ERROR] /home/travis/build/pgjdbc/pgjdbc/pgjdbc/src/test/java/org/postgresql/test/jdbc4/XmlTest.java:312: Line matches the illegal pattern 'Trailing whitespace'. [Regexp]

@bbodnar
Copy link
Contributor Author

@bbodnar bbodnar commented Oct 25, 2017

Sorry. It's now fixed.

@vlsi vlsi added this to the 42.1.5 milestone Oct 25, 2017
@vlsi
vlsi approved these changes Oct 25, 2017
@vlsi vlsi merged commit 8f5e245 into pgjdbc:master Oct 25, 2017
2 checks passed
2 checks passed
codecov/project 65.93% (+0.01%) compared to fcb28c7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vlsi vlsi modified the milestones: 42.1.5, 42.2.0 Jan 8, 2018
rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018
rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants