mUseNesting is not right

[ChristineBao]

Report from Autodesk QA:

During selection in ODBC provider, it generate filter like this:

(ID_ae=2 AND ID=18) OR (ID_ae=3 AND ID=20) OR (ID_ae=4 AND ID=28) OR (ID_ae=5 AND ID=26) OR (ID_ae=7 AND ID=24) OR (ID_ae=9 AND ID=13) OR (ID_ae=10 AND ID=73) OR (ID_ae=12 AND ID=34) OR (ID_ae=13 AND ID=26) OR (ID_ae=14 AND ID=24) OR (ID_ae=15 AND ID=34) OR (ID_ae=17 AND ID=16) OR (ID_ae=18 AND ID=20) OR (ID_ae=19 AND ID=26) OR (ID_ae=20 AND ID=5) OR (ID_ae=21 AND ID=26) OR (ID_ae=23 AND ID=34) OR (ID_ae=25 AND ID=34) OR (ID_ae=26 AND ID=8) OR (ID_ae=28 AND ID=26) OR (ID_ae=29 AND ID=26) OR (ID_ae=30 AND ID=73) OR (ID_ae=31 AND ID=30) OR (ID_ae=32 AND ID=29) OR (ID_ae=33 AND ID=26) OR (ID_ae=34 AND ID=11) OR (ID_ae=35 AND ID=34) OR (ID_ae=41 AND ID=30) OR (ID_ae=42 AND ID=26) OR (ID_ae=43 AND ID=75) OR (ID_ae=48 AND ID=8) OR (ID_ae=50 AND ID=73) OR (ID_ae=51 AND ID=38) OR (ID_ae=52 AND ID=73) OR (ID_ae=53 AND ID=20) OR (ID_ae=54 AND ID=38) OR (ID_ae=55 AND ID=30) OR (ID_ae=56 AND ID=18) OR (ID_ae=57 AND ID=47) OR (ID_ae=58 AND ID=48) OR (ID_ae=60 AND ID=17) OR (ID_ae=61 AND ID=74) OR (ID_ae=62 AND ID=71) OR (ID_ae=63 AND ID=59) OR (ID_ae=64 AND ID=29) OR (ID_ae=65 AND ID=61) OR (ID_ae=66 AND ID=71) OR (ID_ae=69 AND ID=61) OR (ID_ae=70 AND ID=64) OR (ID_ae=72 AND ID=64) OR (ID_ae=73 AND ID=70) OR (ID_ae=79 AND ID=71) OR (ID_ae=80 AND ID=70) OR (ID_ae=81 AND ID=58) OR (ID_ae=82 AND ID=59) OR (ID_ae=83 AND ID=64) OR (ID_ae=84 AND ID=65) OR (ID_ae=85 AND ID=58) OR (ID_ae=86 AND ID=59) OR (ID_ae=87 AND ID=60) OR (ID_ae=88 AND ID=65) OR (ID_ae=89 AND ID=53) OR (ID_ae=90 AND ID=64) OR (ID_ae=92 AND ID=69) OR (ID_ae=93 AND ID=60) OR (ID_ae=94 AND ID=65) OR (ID_ae=95 AND ID=65) OR (ID_ae=96 AND ID=65) OR (ID_ae=97 AND ID=65) OR (ID_ae=98 AND ID=65) OR (ID_ae=99 AND ID=59) OR (ID_ae=100 AND ID=63) OR (ID_ae=101 AND ID=58) OR (ID_ae=102 AND ID=53) OR (ID_ae=103 AND ID=65) OR (ID_ae=104 AND ID=55) OR (ID_ae=105 AND ID=67) OR (ID_ae=106 AND ID=68) OR (ID_ae=107 AND ID=71) OR (ID_ae=110 AND ID=65) OR (ID_ae=111 AND ID=71) OR (ID_ae=112 AND ID=5) OR (ID_ae=113 AND ID=53)

However after processing by FdoRdbmsFilterProcessor, it's appended a lot of ( at first of the filter string, and cause exception when executing the query.

RDBMS: [Microsoft][ODBC Microsoft Access Driver] Expression too complex in query expression '(  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  ( B.[ID_a'.

The generated filter string is:

(  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  (  ( B."ID_ae" = 2 )  AND  ( B."ID" = 18 )  )  OR  (  ( B."ID_ae" = 3 )  AND  ( B."ID" = 20 )  )  )  OR  (  ( B."ID_ae" = 4 )  AND  ( B."ID" = 28 )  )  )  OR  (  ( B."ID_ae" = 5 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 7 )  AND  ( B."ID" = 24 )  )  )  OR  (  ( B."ID_ae" = 9 )  AND  ( B."ID" = 13 )  )  )  OR  (  ( B."ID_ae" = 10 )  AND  ( B."ID" = 73 )  )  )  OR  (  ( B."ID_ae" = 12 )  AND  ( B."ID" = 34 )  )  )  OR  (  ( B."ID_ae" = 13 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 14 )  AND  ( B."ID" = 24 )  )  )  OR  (  ( B."ID_ae" = 15 )  AND  ( B."ID" = 34 )  )  )  OR  (  ( B."ID_ae" = 17 )  AND  ( B."ID" = 16 )  )  )  OR  (  ( B."ID_ae" = 18 )  AND  ( B."ID" = 20 )  )  )  OR  (  ( B."ID_ae" = 19 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 20 )  AND  ( B."ID" = 5 )  )  )  OR  (  ( B."ID_ae" = 21 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 23 )  AND  ( B."ID" = 34 )  )  )  OR  (  ( B."ID_ae" = 25 )  AND  ( B."ID" = 34 )  )  )  OR  (  ( B."ID_ae" = 26 )  AND  ( B."ID" = 8 )  )  )  OR  (  ( B."ID_ae" = 28 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 29 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 30 )  AND  ( B."ID" = 73 )  )  )  OR  (  ( B."ID_ae" = 31 )  AND  ( B."ID" = 30 )  )  )  OR  (  ( B."ID_ae" = 32 )  AND  ( B."ID" = 29 )  )  )  OR  (  ( B."ID_ae" = 33 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 34 )  AND  ( B."ID" = 11 )  )  )  OR  (  ( B."ID_ae" = 35 )  AND  ( B."ID" = 34 )  )  )  OR  (  ( B."ID_ae" = 41 )  AND  ( B."ID" = 30 )  )  )  OR  (  ( B."ID_ae" = 42 )  AND  ( B."ID" = 26 )  )  )  OR  (  ( B."ID_ae" = 43 )  AND  ( B."ID" = 75 )  )  )  OR  (  ( B."ID_ae" = 48 )  AND  ( B."ID" = 8 )  )  )  OR  (  ( B."ID_ae" = 50 )  AND  ( B."ID" = 73 )  )  )  OR  (  ( B."ID_ae" = 51 )  AND  ( B."ID" = 38 )  )  )  OR  (  ( B."ID_ae" = 52 )  AND  ( B."ID" = 73 )  )  )  OR  (  ( B."ID_ae" = 53 )  AND  ( B."ID" = 20 )  )  )  OR  (  ( B."ID_ae" = 54 )  AND  ( B."ID" = 38 )  )  )  OR  (  ( B."ID_ae" = 55 )  AND  ( B."ID" = 30 )  )  )  OR  (  ( B."ID_ae" = 56 )  AND  ( B."ID" = 18 )  )  )  OR  (  ( B."ID_ae" = 57 )  AND  ( B."ID" = 47 )  )  )  OR  (  ( B."ID_ae" = 58 )  AND  ( B."ID" = 48 )  )  )  OR  (  ( B."ID_ae" = 60 )  AND  ( B."ID" = 17 )  )  )  OR  (  ( B."ID_ae" = 61 )  AND  ( B."ID" = 74 )  )  )  OR  (  ( B."ID_ae" = 62 )  AND  ( B."ID" = 71 )  )  )  OR  (  ( B."ID_ae" = 63 )  AND  ( B."ID" = 59 )  )  )  OR  (  ( B."ID_ae" = 64 )  AND  ( B."ID" = 29 )  )  )  OR  (  ( B."ID_ae" = 65 )  AND  ( B."ID" = 61 )  )  )  OR  (  ( B."ID_ae" = 66 )  AND  ( B."ID" = 71 )  )  )  OR  (  ( B."ID_ae" = 69 )  AND  ( B."ID" = 61 )  )  )  OR  (  ( B."ID_ae" = 70 )  AND  ( B."ID" = 64 )  )  )  OR  (  ( B."ID_ae" = 72 )  AND  ( B."ID" = 64 )  )  )  OR  (  ( B."ID_ae" = 73 )  AND  ( B."ID" = 70 )  )  )  OR  (  ( B."ID_ae" = 79 )  AND  ( B."ID" = 71 )  )  )  OR  (  ( B."ID_ae" = 80 )  AND  ( B."ID" = 70 )  )  )  OR  (  ( B."ID_ae" = 81 )  AND  ( B."ID" = 58 )  )  )  OR  (  ( B."ID_ae" = 82 )  AND  ( B."ID" = 59 )  )  )  OR  (  ( B."ID_ae" = 83 )  AND  ( B."ID" = 64 )  )  )  OR  (  ( B."ID_ae" = 84 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 85 )  AND  ( B."ID" = 58 )  )  )  OR  (  ( B."ID_ae" = 86 )  AND  ( B."ID" = 59 )  )  )  OR  (  ( B."ID_ae" = 87 )  AND  ( B."ID" = 60 )  )  )  OR  (  ( B."ID_ae" = 88 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 89 )  AND  ( B."ID" = 53 )  )  )  OR  (  ( B."ID_ae" = 90 )  AND  ( B."ID" = 64 )  )  )  OR  (  ( B."ID_ae" = 92 )  AND  ( B."ID" = 69 )  )  )  OR  (  ( B."ID_ae" = 93 )  AND  ( B."ID" = 60 )  )  )  OR  (  ( B."ID_ae" = 94 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 95 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 96 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 97 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 98 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 99 )  AND  ( B."ID" = 59 )  )  )  OR  (  ( B."ID_ae" = 100 )  AND  ( B."ID" = 63 )  )  )  OR  (  ( B."ID_ae" = 101 )  AND  ( B."ID" = 58 )  )  )  OR  (  ( B."ID_ae" = 102 )  AND  ( B."ID" = 53 )  )  )  OR  (  ( B."ID_ae" = 103 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 104 )  AND  ( B."ID" = 55 )  )  )  OR  (  ( B."ID_ae" = 105 )  AND  ( B."ID" = 67 )  )  )  OR  (  ( B."ID_ae" = 106 )  AND  ( B."ID" = 68 )  )  )  OR  (  ( B."ID_ae" = 107 )  AND  ( B."ID" = 71 )  )  )  OR  (  ( B."ID_ae" = 110 )  AND  ( B."ID" = 65 )  )  )  OR  (  ( B."ID_ae" = 111 )  AND  ( B."ID" = 71 )  )  )  OR  (  ( B."ID_ae" = 112 )  AND  ( B."ID" = 5 )  )  )  OR  (  ( B."ID_ae" = 113 )  AND  ( B."ID" = 53 )  )  ) 

This ticket is related to: Related to ticket ​http://trac.osgeo.org/fdo/ticket/367.

[ChristineBao]

This defect is caused by the mUseNesting flag. It's set to true if the filter contains both AND and OR operation

FdoRdbmsFilterProcessor.cpp:

    // Initialize the member variables that are set by this routine. The default
    // value should reflect the current behavior.
    mUseNesting         = true;
    mUseGrouping        = false;
    mAddNegationBracket = false;

    // Analyze the filter.
    FilterAnalyzer filterAnalyzer;
    filter->Process(&filterAnalyzer);

    // Check the result of the analyzing process and set the corresponding
    // member variables that control the generation of the SQL statement
    // from the given filter.
    if ((filterAnalyzer.containsBinaryLogicalOperatorAnd) ||
        (filterAnalyzer.containsBinaryLogicalOperatorOr)     )
    {
        if (filterAnalyzer.isSpatialObjectFilter)
        {
            mUseNesting  = false;
            mUseGrouping = true;
        }
        else
        {
            mUseNesting = filterAnalyzer.containsBinaryLogicalOperatorAnd &&
                              filterAnalyzer.containsBinaryLogicalOperatorOr;
        }
        mAddNegationBracket =
                        !mUseNesting &&
                        filterAnalyzer.containsUnaryLogicalOperatorNot;
    }

And when the mUseNesting is true, ( is added again and again for the OR operator.

FdoRdbmsFilterProcessor.cpp:

    if (mUseNesting)
        AppendString (OPEN_PARENTH, 3);
    if( filter.GetOperation() == FdoBinaryLogicalOperations_And )
    {
        useGrouping  = mUseGrouping;
        mUseGrouping = false;
        if (useGrouping)
            AppendString (OPEN_PARENTH, 3);
        HandleFilter( leftOperand );
        if (useGrouping)
            AppendString (CLOSE_PARENTH, 3);
        AppendString( LOGICAL_AND );
        if (useGrouping)
            AppendString (OPEN_PARENTH, 3);
        HandleFilter( rightOperand );
        if (useGrouping)
            AppendString (CLOSE_PARENTH, 3);
    }
    else
    {
        mProcessingOrOperator = true;
        HandleFilter( leftOperand );
        AppendString (LOGICAL_OR, 4);
        HandleFilter( rightOperand );
		FdoSpatialCondition* leftSpCond = dynamic_cast<FdoSpatialCondition*>(leftOperand.p);
        FdoSpatialCondition* rightSpCond = dynamic_cast<FdoSpatialCondition*>(rightOperand.p);
        if ( !SupportsSpatialOrNonSpatialOperator() ) 
        {
            if ((leftSpCond || rightSpCond) && !(leftSpCond && rightSpCond))
                throw FdoCommandException::Create( NlsMsgGet(FDORDBMS_532, "OR not supported in a query when mixing property with spatial filters" ) );
        }
    }

    if (mUseNesting)
        AppendString (CLOSE_PARENTH, 3);

In this test case, there are a lot of OR operation, so too many ( is added into filter string and caused the error.

[ChristineBao]

The test code, config.xml and test data are attached.

[ChristineBao]

Attach fixing ​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802.patch for this ticket.

[ChristineBao]

Hi Thomas,

I read your fixing in ticket #367 (r4030) which fixed the nested "(" issue in some cases. I don't think the necessaries to add additional "(" in the filter, so I remove the mUseNesting flag. It's tested successfully in this test case, however I am not sure how it works for others. Would you please review it? This defect cause MapGuide cannot select features if use ODBC provider. It's appreciated that you can review it in this week.

Thanks & regards, Christine

[ChristineBao]

Attach new fix ​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802_2.patch.

[ChristineBao]

Previous fix was reviewed by Brent Robinson and he brought a very good question. If the filter is L"(COLOR = 652 or FEATID = 21587) and (FEATID = 20931 or COLOR = 256)", it should generate ( ( ( B.COLOR = 652 ) OR ( B.FEATID = 21587 ) ) AND ( ( B.FEATID = 20931 ) OR ( B.COLOR = 256 ) ) ), however the fix result is ( B.COLOR = 652 ) OR ( B.FEATID = 21587 ) AND ( B.FEATID = 20931 ) OR ( B.COLOR = 256 ). Obviously this is wrong.

Thanks Brent for bringing this up.

[ChristineBao]

The key point is () can only be ignored if OR is root operator. The problem happens here is when OR is root operator, it still add () to the filter. For example, consider the first round, it ends up: (A OR B) then second round, it ends up: ((A OR B) OR C) then third round, it ends up: (((A OR B) OR C) OR D) As you can see, the starting "(" gets more and more, and finally caused the trouble in ODBC execution. However the root OR does not need (). It has no difference of adding it or not.

[ChristineBao]

Attach a new fixing for this: ​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802.patch

It checks whether OR is the root operator. If yes, no do set mUseNesting for adding ().

Please review. Thanks.

[ChristineBao]

Attach new fix ​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802_3.patch.

[ChristineBao]

Brent brought a good question about Fix#802_2. It cannot work for: COLOR = 0 or (COLOR = 652 or FEATID = 21587) and (FEATID = 20931 or COLOR = 256)

Thanks Brent.

So I figure out the new rule is: if found a OR operator, check if AND operator has already been found -- this is because the process runs from root operator to child operator, if AND is found first, and then OR, it should add "()" around. Please review the new fixing: ​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802_3.patch.

[ChristineBao]

Sorry, the #3 fix is wrong in this case: ID_ae=0 AND (ID_ae=2 OR ID=18)

It's because the containsBinaryLogicalOperatorAnd is reset to false;

I attached a new fix #4 to remember the AND operator by hasBinaryLogicalOperatorAnd.

[ChristineBao]

​http://trac.osgeo.org/fdo/attachment/ticket/802/Fix%23802_4.patch is reviewed by Brent and it's OK. FDO is not code cutoff, and this fixing will be submit after Jan 4.

[ChristineBao]

Submit to trunk in r6348.