Why change it then? Why put the effort into it, and then require corresponding review effort?
Why comment "the +/- notation does not work well with PostgreSQL"?Either way I don't care a lot, as long as it's clear what is being fixed and what is just being changed because "I like my way better".
-David On Feb 3, 2007, at 2:51 AM, Leon Torres wrote:
Hi David,I was involved in fixing this issue. There was a genuine bug where the inventory reservation with FIFO method was ordered the wrong way, causing it to behave like LIFO. This commit fixes the issue by reversing the order for the FIFO case so it works correctly.So unless you want to break an important fix, it would be a bad idea to revert this.The ASC/DESC thing is just a clarification and a preference that was made in addition to the bug fix. This is *not* just a change of syntax commit. Furthermore, there is *no* bug with +- notation.We prefer to use ASC/DESC because it makes it clearer what's going on in this code, and we feel that the +- syntax is more ambiguous and most likely led to the incorrect implementation of FIFO in the first place.- Leon David E. Jones wrote:Hold on a minute there.... did you actually test and find this to be a problem? The +/- notation is an entity engine ONLY thing and should never make it to the database. This patch should be reverted and if +/- are making it to the database instead of being replaced with an ASC/DESC by the entity engine then THAT bug should be fixed. This is a slippery slope and we should backup to the top before it gets going...-David On Feb 2, 2007, at 6:16 PM, [EMAIL PROTECTED] wrote:Author: sichen Date: Fri Feb 2 17:16:36 2007 New Revision: 502824 URL: http://svn.apache.org/viewvc?view=rev&rev=502824 Log:Fix a pretty significant bug with sequence of inventory item reservations for orders: FIFO and LIFO were reversed (I checked this) and the +/- notation does not work well with PostgreSQLModified:ofbiz/trunk/applications/product/script/org/ofbiz/product/ inventory/InventoryReserveServices.xmlModified: ofbiz/trunk/applications/product/script/org/ofbiz/ product/inventory/InventoryReserveServices.xml URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ product/script/org/ofbiz/product/inventory/ InventoryReserveServices.xml? view=diff&rev=502824&r1=502823&r2=502824 ==================================================================== ========== --- ofbiz/trunk/applications/product/script/org/ofbiz/product/ inventory/InventoryReserveServices.xml (original) +++ ofbiz/trunk/applications/product/script/org/ofbiz/product/ inventory/InventoryReserveServices.xml Fri Feb 2 17:16:36 2007@@ -42,23 +42,26 @@<entity-one entity-name="OrderHeader" value- name="orderHeader"/><!-- before we do the find, put together the orderBy list based on which reserveOrderEnumId is specified --> + <!-- FIFO=first in first out, so it should be order by ASCending receive or expire date + LIFO=last in first out, so it means order by DESCending receive or expire date+ --><if-compare value="INVRO_GUNIT_COST" operator="equals" field-name="reserveOrderEnumId" map-name="parameters">- <set value="-unitCost" field="orderByString"/> + <set value="unitCost DESC" field="orderByString"/> <else><if-compare value="INVRO_LUNIT_COST" operator="equals" field-name="reserveOrderEnumId" map- name="parameters">- <set value="+unitCost" field="orderByString"/> + <set value="unitCost ASC" field="orderByString"/> <else><if-compare value="INVRO_FIFO_EXP" operator="equals" field-name="reserveOrderEnumId" map- name="parameters"> - <set value="+expireDate" field="orderByString"/> + <set value="expireDate ASC" field="orderByString"/><else><if-compare value="INVRO_LIFO_EXP" operator="equals" field-name="reserveOrderEnumId" map- name="parameters"> - <set value="-expireDate" field="orderByString"/> + <set value="expireDate DESC" field="orderByString"/><else><if-compare value="INVRO_LIFO_REC" operator="equals" field-name="reserveOrderEnumId" map- name="parameters"> - <set value="+datetimeReceived" field="orderByString"/> + <set value="datetimeReceived DESC" field="orderByString"/><else><!-- the default reserveOrderEnumId is INVRO_FIFO_REC, ie FIFO based on date received --> - <set value="-datetimeReceived" field="orderByString"/> + <set value="datetimeReceived ASC" field="orderByString"/> <set value="INVRO_FIFO_REC" field="parameters.reserveOrderEnumId"/></else> </if-compare>
smime.p7s
Description: S/MIME cryptographic signature
