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.

Sign up for GitHub

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

Jump to bottom

Add delayLoadingLobs #1336

Merged

rene-ye merged 12 commits into microsoft:dev from rene-ye:blob

May 26, 2020

Member

rene-ye commented May 14, 2020 •

edited

Loading

Adds a property delayLoadingLobs which will cause all LOB objects retrieved from the ResultSet to not be streamed.

The driver does this by setting a boolean on every LOB object when retrieved via a getter. This boolean is then used by the LOB object when users call stream getters.

rene-ye and others added 3 commits

May 11, 2020 08:58


          Add property

13fc69a


          Add resource

e994670


          Fix nullptr

4c1e3ef

rene-ye added the Public API label

rene-ye changed the title ~~Add loadLoargeObjects~~ Add loadLargeObjects

rene-ye added this to the 8.3.1 milestone

rene-ye added 4 commits

May 21, 2020 14:30


          change loadLargeObjects to delayLoadingLobs

8708f10


          add tests

8aaeff9


          quick fix

7d26080


          Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into blob

2493e81

# Conflicts:
#	src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java Outdated Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java Outdated Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java Outdated Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/LobsTest.java Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/LobsTest.java Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/LobsTest.java Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/LobsTest.java Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java Outdated Show resolved Hide resolved

TarasTielkes commented May 22, 2020

(I'm adding this comment as a user that follows the project, but does not actively participate - if this is not the right place for this comment, please let me know)

It would be good to document the intent of this API. Why would I, as a user of the driver, want to set the loadLargeObjects property? What are the benefits, and what are the downsides? Interacting with large volumes of large BLOBs is a tricky area in my experience, so a clear description of what this API intends to facilitate would be a good idea, methinks.

I do agree with the comments from @peterbae regarding the naming, FWIW.

rene-ye changed the title ~~Add loadLargeObjects~~ Add delayLoadingLobs


          address comments

b64fa12

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerLob.java Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java Outdated Show resolved Hide resolved

peterbae reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/unit/lobs/LobsTest.java Show resolved Hide resolved


          format

5f1b0f7

peterbae previously approved these changes

View reviewed changes

ulvii previously approved these changes

View reviewed changes

lilgreenbird reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBlob.java Show resolved Hide resolved

rene-ye added 2 commits

May 26, 2020 09:45


          small test fix

89a6a48


          Merge branch 'blob' of https://github.com/rene-ye/mssql-jdbc into blob

f79f76d

rene-ye dismissed stale reviews from ulvii and peterbae via

f79f76d

May 26, 2020 16:46

lilgreenbird previously approved these changes

View reviewed changes

ulvii approved these changes

View reviewed changes

ulvii suggested changes

View reviewed changes

Contributor

ulvii left a comment

There are conflicts in the PR.


          Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into blob

4f856a2

# Conflicts:
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java

rene-ye dismissed lilgreenbird’s stale review via

4f856a2

May 26, 2020 17:57

peterbae approved these changes

View reviewed changes

ulvii approved these changes

View reviewed changes

lilgreenbird approved these changes

View reviewed changes

rene-ye merged commit 54c6748 into microsoft:dev

Contributor

ulvii commented May 27, 2020

Hi @TarasTielkes ,
Thanks for the suggestions. We will definitely document the property on MSDN once it is in production release. To give you a brief idea about it, as of 7.0.0 version of the driver, we introduced some changes to the way the driver handles LOB streaming and the new property is meant to provide backward compatibility with 6.4.0 version of the driver for some edge cases.

lilgreenbird added this to Closed/Merged PRs in MSSQL JDBC

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