Eli Mesika 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 inter
Done
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 i
No,
See SearchQuery code :

long maxValue = Integer.MAX_VALUE;
// If a number > maxValue is given then maxValue will be used
searchObj.setMaxCount(getParameters().getMaxCount() == -1 ? maxValue : 
Math.min(maxValue, getParameters().getMaxCount()));

So, the value is set to Integer.MAX_VALUE  before this is called
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
Done
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 
Done
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

Reply via email to