Bug #5452
eliminate use of ResultSet.CONCUR_UPDATABLE?
100%
History
#1 Updated by Eric Faulhaber almost 3 years ago
Ovidiu, while debugging a few days ago, I came across this in SQLQuery.scroll(Session, int, boolean, List<RowStructure>)
:
int readOnlyMode = readOnly ? ResultSet.CONCUR_READ_ONLY : ResultSet.CONCUR_UPDATABLE;
I know we carried the readOnly
parameter over from the pre-4011 implementation, but this rarely will be set to true
, so we almost always will use CONCUR_UPDATABLE
. AFAIK, we never update records directly from a ResultSet
using the updateXXXX
methods, so I think we can drop the readOnly
parameter altogether and always make result sets CONCUR_READ_ONLY
.
I don't know if there's any practical performance benefit to this change, but it seems like it can't make it worse.
What do you think?
#2 Updated by Ovidiu Maxiniuc almost 3 years ago
You are right. Since we always fetch the data from the ResultSet
s and use it to hydrate our own Record
s there is no point in having the ResultSet
s updatable.
Also, I see that DatabaseManager.readOnly
is not populated any more so DatabaseManager.isReadOnly()
which is used to set the readonly
parameter is always returning false
.
#3 Updated by Alexandru Lungu about 1 year ago
- Assignee set to Ștefan Roman
#4 Updated by Alexandru Lungu about 1 year ago
Created 5452a from trunk/rev. 14560.
#5 Updated by Ștefan Roman about 1 year ago
- % Done changed from 0 to 50
- Status changed from New to WIP
#6 Updated by Ștefan Roman about 1 year ago
- % Done changed from 50 to 100
- Status changed from WIP to Review
Removed readOnly flag from SQLQuery, checked with HotelGUI and everything works fine.
Commited to 5452a revision 14561.
#7 Updated by Alexandru Lungu about 1 year ago
- % Done changed from 100 to 50
- Status changed from Review to WIP
I am OK with the current changes.
However, you still need to extend your removal of readOnly
. More exactly, there is still a Query.readOnly
member we no longer use, as the support for custom ResultSet
read-mode is removed.
For that matter, remove Query.setReadOnly
. This will require changes in Persistence.getQuery
(readOnly flag) and DefaultDirtyShareManager.list
(readOnly flag). These will cause changes in other files as well.
#8 Updated by Ștefan Roman about 1 year ago
- Status changed from WIP to Review
- % Done changed from 50 to 100
Removed readOnly flag and fixed persistence.list() and getQuery() new signatures, checked with a large project and it works normally.
Commited to 5452a revision 14562.
#9 Updated by Alexandru Lungu 9 months ago
I am OK with the changes.
I also committed 5452a rev. 14563 removing readOnly
from share manager and multiplexer classes.
I am rebasing now and going to merge in trunk.
#10 Updated by Alexandru Lungu 9 months ago
- Status changed from Review to Test
Branch 5452a was merged to trunk as rev 14704 and archived.
This can be closed.