Bug 231030 - Make usage of scrollable cursors configurable
Make usage of scrollable cursors configurable
Status: RESOLVED FIXED
Product: db
Classification: Unclassified
Component: Show Data
7.4
All All
: P1 (vote)
: 7.4
Assigned To: Jaroslav Havlin
issues@db
: API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-10 12:37 UTC by Jaroslav Havlin
Modified: 2013-06-27 02:20 UTC (History)
0 users

See Also:
Issue Type: DEFECT
:


Attachments
Proposed Patch (34.24 KB, patch)
2013-06-10 12:43 UTC, Jaroslav Havlin
Details | Diff
Proposed Patch - UI (18.63 KB, patch)
2013-06-10 18:04 UTC, Jaroslav Havlin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Havlin 2013-06-10 12:37:41 UTC
Scrollable cursors make SQL queries faster, but some drivers may start to fail. Use it by default only with well-known and tested drivers.

Users should be able to disable scrollable cursors if they encounter problems.
Comment 1 Jaroslav Havlin 2013-06-10 12:43:00 UTC
Created attachment 135585 [details]
Proposed Patch
Comment 2 Jaroslav Havlin 2013-06-10 18:04:01 UTC
Created attachment 135602 [details]
Proposed Patch - UI

I've split the patch into two parts. The first part, committed as
http://hg.netbeans.org/core-main/rev/832e3500091e, uses default list of drivers that can use scrollable cursors. This list can be customized using system property "db.scrollable.cursors.drivers".

The second (just attached) part contains the UI for enabling or disabling scrollable cursors for individual connections. This change needs to pass an API review.
Comment 3 Jaroslav Havlin 2013-06-10 18:10:28 UTC
Please review API change of class
org.netbeans.api.db.explorer.DatabaseConnection.

New methods:
boolean isUseScrollableCursors()
void    setUseScrollableCursors(boolean)

Thank you.
Comment 4 Quality Engineering 2013-06-12 02:05:17 UTC
Integrated into 'main-golden', will be available in build *201306112301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/832e3500091e
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #231030: Make usage of scrollable cursors configurable
Comment 5 matthias42 2013-06-19 11:16:38 UTC
Hey, I had a look at the changeset today (832e3500091e). A few remarks (I'm at work and have only limited access, so I'm just reading through the diff - please take the points with that in mind ;-))

a) getTotalCount - line 1279: This fails if the resultset contains less rows, than the limit, it also fails with the extended mysql limit syntax: "LIMIT offset, count"

b) getTotalCount - line 1324: Are you sure?! setFetchSize(20000) - if it is not ignored by the driver, it might very well blow the heap

c) The optimisations in getCountSQLQuery and getCountAsSubQuery look suspicious:

SELECT * FROM (
SELECT a, b, c, count(*) FROM TABLE2 GROUP BY 1, 2, 3 ORDER BY 1, 2, 3
) TABLE1
ORDER BY 2,1

would in case of getCountAsSubQuery lead to:

SELECT COUNT(*) FROM (SELECT * FROM (SELECT a, b, c, count(*) FROM TABLE 2 GROUP BY 1, 2, 3) C2668


Related: The mysql driver/server is utterly broken with respect to handling of large number of rows.

The simple sample (c is a connection to a mysql server):

Statement s = c.createStatement();
ResultSet rs = s.executeQuery("SELECT * FROM LARGE_TABLE");
for(int i = 0; i < 20 && rs.next(); i++) {
    System.out.println(rs.getString(1));
}
rs.close();
s.close();

Blows with a heap overflow...

You can:

a) Limit the absolute number of rows to fetch:

s.setMaxRows(30);

b) Switch to streaming mode (works sort of, still read _whole_ resultset):

s.setFetchSize(Integer.MIN_VALUE);

Both solutions are no-gos (both are broken in more than one way....
Comment 6 Jaroslav Havlin 2013-06-20 11:07:43 UTC
(In reply to comment #5)
> a) getTotalCount - line 1279: This fails if the resultset contains less rows,
> than the limit, it also fails with the extended mysql limit syntax: "LIMIT
> offset, count".
Good point. Removed.

> b) getTotalCount - line 1324: Are you sure?! setFetchSize(20000) 
> - if it is not ignored by the driver, it might very well blow the heap
I've just reverted the method from the original implementation. It's seems that this code wasn't invoked often. 
It's probably safer to show N/A if other ways to get total count fail. So I'm removing this part of code, too.

> c) The optimisations in getCountSQLQuery and getCountAsSubQuery look
> suspicious:
It seems to work fine. The method was in use for quite a long time, so it should work reliably.
Or why do you think it's suspicious?

Fixed in http://hg.netbeans.org/core-main/rev/5ad93a1b4bde
Thank you very much.
Comment 7 matthias42 2013-06-20 20:02:40 UTC
(In reply to comment #6)
> > c) The optimisations in getCountSQLQuery and getCountAsSubQuery look
> > suspicious:
> It seems to work fine. The method was in use for quite a long time, so it
> should work reliably.
> Or why do you think it's suspicious?

I did some paper calculation :-)

    static String getCountAsSubQuery(String queryString) {
        /* Input: 
         * SELECT * FROM (
         *     SELECT a, b, c FROM TABLE2 ORDER BY 1, 2, 3
         * ) TABLE1 WHERE a = 1
         */
        String[] splitByOrderBy = queryString.toUpperCase().split("ORDER BY"); // NOI18N
        /**
         * spitByOrderBy looks like this:
         * {
         * "SELECT * FROM (\n    SELECT A, B, C FROM TABLE2 ",
         * ") TABLE 1 WHERE A = 1"
         * }
         */
        queryString = queryString.substring(0, splitByOrderBy[0].length());
        /*
         * queryString = "SELECT * FROM ( SELECT a, b, c, count(*) FROM TABLE2 
         */
        return "SELECT COUNT(*) FROM (" + queryString + ") C2668"; // NOI18N
        /*
         * Result:
         * 
         * SELECT COUNT(*) FROM (SELECT * FROM ( SELECT a, b, c, count(*) FROM TABLE2) C2668
         */
    }

The result is wrong - even if you add the missing brace, the count will be way off.

While the above query was deliberately choosen, this could happen at any time. You could try to tackle this with a full blown SQL parser, but the extra time the one ORDER you optize out will most probably not be worth it.
Comment 8 Jaroslav Havlin 2013-06-21 08:43:35 UTC
> > > c) The optimisations in getCountSQLQuery and getCountAsSubQuery look
> > > suspicious:
> The result is wrong - even if you add the missing brace, the count will be way
> off.
You are right, thank you for explanation and the example.

> While the above query was deliberately choosen, this could happen at any time.
> You could try to tackle this with a full blown SQL parser, but the extra time
> the one ORDER you optize out will most probably not be worth it.
I originally thought that ORDER BY is removed because some DBs don't allow it in subqueries, but it is not probably the problem.
I agree it's not worth optimizing.
Fixed in http://hg.netbeans.org/core-main/rev/090decadd5e7
Thank you.
Comment 9 Jaroslav Havlin 2013-06-21 08:46:06 UTC
(In reply to comment #3)
> Please review API change of class
> org.netbeans.api.db.explorer.DatabaseConnection.
If there are no objections, I'll integrate the patch on Monday.
Thanks for reviewing.
Comment 10 Quality Engineering 2013-06-22 02:10:17 UTC
Integrated into 'main-golden', will be available in build *201306212301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/4db74340500c
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #231030: DataView pagination broken if scrollable cursors are disabled
Comment 11 Jaroslav Havlin 2013-06-24 08:59:06 UTC
(In reply to comment #9)
> If there are no objections, I'll integrate the patch on Monday.
http://hg.netbeans.org/core-main/rev/a31ab5991cd5

Removing apireviews@netbeans from the CC list, as future comments will not be probably related to the API change.
Comment 12 Jaroslav Havlin 2013-06-24 09:07:58 UTC
For reference, increasing priority.
A regression (broken pagination) was introduced by changeset
http://hg.netbeans.org/main-golden/rev/832e3500091e.
Fixed in http://hg.netbeans.org/main-golden/rev/4db74340500c.
Comment 13 Quality Engineering 2013-06-25 02:45:51 UTC
Integrated into 'main-golden', will be available in build *201306242301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/090decadd5e7
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #231030: Do not try to remove ORDER BY clause from queries for getting total count
Comment 14 Quality Engineering 2013-06-27 02:20:14 UTC
Integrated into 'main-golden', will be available in build *201306262301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/a31ab5991cd5
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #231030: Make usage of scrollable cursors configurable - UI


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo