Opened 13 years ago
Last modified 13 years ago
#719 new defect
Defect in filter parsing
Reported by: | maland | Owned by: | romicadascalescu |
---|---|---|---|
Priority: | major | Milestone: | 3.6.0 |
Component: | ArcSDE Provider | Version: | 3.5.0 |
Severity: | 3 | Keywords: | |
Cc: | maland | External ID: |
Description
There is a defect in the ProcessBinaryLogicalOperator function in ArcSDEFilterToSql.cpp. It happens when both operands are "attribute"-style filters. The parenthesis matching is broken.
If you set a filter like this: YROW = 0 AND ( XROW = 0 OR XROW = 1)
Then the generated SQL will be: YROW = 0 AND ( ( ( (XROW) = (0) ( OR ( (XROW) = (1) ) ) )
This is obviously wrong since there is 9 left parenthesis and 7 right parenthesis.
FIX: I've not spent enough time on this to understand the point of the mUseNesting variable, but the following fix works good for my case.
Original code:
else if ((leftOpType == ArcSDEFilterType_Attribute) && (rightOpType == ArcSDEFilterType_Attribute)) { // Both operands are "attribute"-style filters, process them both (append both to query string): if (!mFilterAnalyzed) AppendString (OPEN_PAREN); else if ((mFilterAnalyzed) && (mUseNesting)) AppendString (OPEN_PAREN); HandleFilter (FdoPtr<FdoFilter>(filter.GetLeftOperand ())); if (!mFilterAnalyzed) AppendString (OPEN_PAREN); else if ((mFilterAnalyzed) && (mUseNesting)) AppendString (OPEN_PAREN); switch (filter.GetOperation ()) { case FdoBinaryLogicalOperations_And: AppendString (LOGICAL_AND); break; case FdoBinaryLogicalOperations_Or: AppendString (LOGICAL_OR); break; default: throw FdoFilterException::Create (NlsMsgGet(ARCSDE_UNSUPPORTED_BINARY_LOGICAL_OPERATOR, "The given binary logical operator is not supported.")); } if ((mFilterAnalyzed) && (mUseNesting)) AppendString (OPEN_PAREN); HandleFilter (FdoPtr<FdoFilter>(filter.GetRightOperand ())); if ((mFilterAnalyzed) && (mUseNesting)) AppendString (CLOSE_PAREN); }
Modified code:
else if ((leftOpType == ArcSDEFilterType_Attribute) && (rightOpType == ArcSDEFilterType_Attribute)) { // Both operands are "attribute"-style filters, process them both (append both to query string): AppendString (OPEN_PAREN); HandleFilter (FdoPtr<FdoFilter>(filter.GetLeftOperand ())); switch (filter.GetOperation ()) { case FdoBinaryLogicalOperations_And: AppendString (LOGICAL_AND); break; case FdoBinaryLogicalOperations_Or: AppendString (LOGICAL_OR); break; default: throw FdoFilterException::Create (NlsMsgGet(ARCSDE_UNSUPPORTED_BINARY_LOGICAL_OPERATOR, "The given binary logical operator is not supported.")); } HandleFilter (FdoPtr<FdoFilter>(filter.GetRightOperand ())); AppendString (CLOSE_PAREN); }
Attachments (1)
Change History (4)
by , 13 years ago
Attachment: | ArcSDE.patch added |
---|
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Owner: | changed from | to
---|
comment:3 by , 13 years ago
Cc: | added |
---|
Hi I agree that your proposed change will simplify the code. I have applied the patch locally and tested with our application. The fix works good. Thank you!
Regards, Henning
Hi Henning,
I had a look at you change and it's OK, however I would propose some additional changes to remove class FilterAnalyzer and flags mFilterAnalyzed, mUseNesting. This will simplify the code and will make possible use less brackets in a filter. The same method is used in FDO & SQLite filter ToString in order to avoid too many brackets. Could you please have a loot at the code change proposed and test it if possible and let me know your opinion?
Regards, Romy.