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)

ArcSDE.patch (11.0 KB ) - added by romicadascalescu 13 years ago.

Download all attachments as: .zip

Change History (4)

by romicadascalescu, 13 years ago

Attachment: ArcSDE.patch added

comment:1 by romicadascalescu, 13 years ago

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.

comment:2 by gregboone, 13 years ago

Owner: changed from gregboone to romicadascalescu

comment:3 by maland, 13 years ago

Cc: maland 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

Note: See TracTickets for help on using tickets.