Juan Hernandez has posted comments on this change. Change subject: fixing integer out of range exception in search ......................................................................
Patch Set 5: (4 comments) http://gerrit.ovirt.org/#/c/23488/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/SearchParameters.java: Line 8: private static final long serialVersionUID = 2275481072329075722L; Line 9: Line 10: private String _searchPattern; Line 11: private SearchType _searchType; Line 12: private long _maxCount; I think that this isn't necessary. We need to use larger integers for intermediate calculations, but not for this parameter. Line 13: private long searchFrom; Line 14: private boolean caseSensitive; Line 15: Line 16: public SearchParameters() { http://gerrit.ovirt.org/#/c/23488/5/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java File backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java: Line 935: Line 936: protected String getPagePhrase(SyntaxContainer syntax, String pageNumber) { Line 937: String result = ""; Line 938: // Do not add paging syntax if query attempts to get all records Line 939: if (syntax.getMaxCount() < Integer.MAX_VALUE) { I thought that -1 was the value that means "get all the values", at least it is the value used in the RESTAPI. Has that changed? Line 940: Integer page = IntegerCompat.tryParse(pageNumber); Line 941: if (page == null) { Line 942: page = 1; Line 943: } Line 954: case Offset: Line 955: result = Line 956: StringFormat.format(pagingSyntax, Line 957: (page - 1) * syntax.getMaxCount() + 1, Line 958: syntax.getMaxCount()); Here is where we should convert to longer integers, just in order to do the calculations. Long should be long enough for this, but I would suggest to use BigInteger arithmetic, that way you don't have to worry about overflows. Line 959: break; Line 960: } Line 961: } Line 962: } http://gerrit.ovirt.org/#/c/23488/5/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxContainer.java File backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxContainer.java: Line 13: private static final String LINE_SEPARATOR = "\n"; Line 14: private boolean mValid = false; Line 15: private SyntaxError mError = SyntaxError.NO_ERROR; Line 16: private final int[] mErrorPos = new int[2]; Line 17: private long privateMaxCount; I think we don't need to change the type of this parameter, only the types of variables used for intermediate calculations. Line 18: private long searchFrom = 0; Line 19: private boolean caseSensitive=true; Line 20: Line 21: public long getMaxCount() { -- To view, visit http://gerrit.ovirt.org/23488 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I518dfdfff93dde28d9741d5e333c695239b7e7d2 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
