PATCH: Fix for ticket #864

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

PATCH: Fix for ticket #864

Jackie Ng
Hi All,

I've attached a patch for ticket 864 (864_sqlserver.patch)

http://trac.osgeo.org/fdo/ticket/864

It turns out the SQL Server provider uses its own FdoIFilterProcessor implementation (in addition/instead of the GenericRdbms one) which does not apply parentheses to the top-level operands of a binary condition that's connected by a logical AND operator.

Please review. Thanks.

- Jackie
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Fix for ticket #864 (and #889)

Jackie Ng
I've added a second version of the patch (864_sqlserver_v2.patch)

This should also fix ticket #889 as an analysis of the method shows that the addLeftBr variable was never set to true before I submitted the initial patch for review, meaning if the left operand contained a chain of OR conditions and is part of a AND binary condition, it was never properly parenthesised.

Since the area of code that affects #889 is the same area that affects #864, I've rolled both fixes into the same patch.

Please review. Thanks

- Jackie
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Fix for ticket #864 (and #889)

Romica Dascalescu
Hi Jackie,

Code change looks OK.

Thanks,
Romica
________________________________________
From: [hidden email] [[hidden email]] on behalf of Jackie Ng [[hidden email]]
Sent: Monday, May 12, 2014 10:37 AM
To: [hidden email]
Subject: Re: [fdo-internals] PATCH: Fix for ticket #864 (and #889)

I've added a second version of the patch (864_sqlserver_v2.patch)

This should also fix ticket #889 as an analysis of the method shows that the
addLeftBr variable was never set to true before I submitted the initial
patch for review, meaning if the left operand contained a chain of OR
conditions and is part of a AND binary condition, it was never properly
parenthesised.

Since the area of code that affects #889 is the same area that affects #864,
I've rolled both fixes into the same patch.

Please review. Thanks

- Jackie



--
View this message in context: http://osgeo-org.1560.x6.nabble.com/PATCH-Fix-for-ticket-864-tp5139747p5139787.html
Sent from the FDO Internals mailing list archive at Nabble.com.
_______________________________________________
fdo-internals mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/fdo-internals
_______________________________________________
fdo-internals mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/fdo-internals
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: Fix for ticket #864 (and #889)

Greg Boone
I should get an R3 posted by early next week.

-----Original Message-----
From: [hidden email] [mailto:[hidden email]] On Behalf Of Romica Dascalescu
Sent: Monday, May 12, 2014 10:44 AM
To: FDO Internals Mail List
Subject: Re: [fdo-internals] PATCH: Fix for ticket #864 (and #889)

Hi Jackie,

Code change looks OK.

Thanks,
Romica
________________________________________
From: [hidden email] [[hidden email]] on behalf of Jackie Ng [[hidden email]]
Sent: Monday, May 12, 2014 10:37 AM
To: [hidden email]
Subject: Re: [fdo-internals] PATCH: Fix for ticket #864 (and #889)

I've added a second version of the patch (864_sqlserver_v2.patch)

This should also fix ticket #889 as an analysis of the method shows that the addLeftBr variable was never set to true before I submitted the initial patch for review, meaning if the left operand contained a chain of OR conditions and is part of a AND binary condition, it was never properly parenthesised.

Since the area of code that affects #889 is the same area that affects #864, I've rolled both fixes into the same patch.

Please review. Thanks

- Jackie



--
View this message in context: http://osgeo-org.1560.x6.nabble.com/PATCH-Fix-for-ticket-864-tp5139747p5139787.html
Sent from the FDO Internals mailing list archive at Nabble.com.
_______________________________________________
fdo-internals mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/fdo-internals
_______________________________________________
fdo-internals mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/fdo-internals
_______________________________________________
fdo-internals mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/fdo-internals