Re: svn commit: r1327735 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity: jdbc/SqlJdbcUtil.java model/ModelViewEntity.java

[Scott Gray-2]

Nice, I ran into a problem related to this very recently.  Thanks for taking care of it.

Regards
Scott

On 19/04/2012, at 11:48 AM, [hidden email] wrote:

> Author: doogie
> Date: Wed Apr 18 23:48:40 2012
> New Revision: 1327735
>
> URL: http://svn.apache.org/viewvc?rev=1327735&view=rev
> Log:
> FEATURE/FIX: <entity-condition> on views are now done correctly.  When a
> view was previously joined to another view, the conditions on the inner
> view were added to the outer-most WHERE clause.  This caused multiple
> problems.  One, if the join was optional, the generated sql actually made
> it required(because it was in the WHERE, and not in the ON clause).
> Second, the outer WHERE may reference a field that was not available *at
> all* in the generated nested select(view table).
>
> The fix is to *not* recurse thru all view member entities when finding
> conditions, and instead attach them to a new WHERE clause on the
> generated inner SELECT(view table).
>
> Modified:
>    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=1327735&r1=1327734&r2=1327735&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java Wed Apr 18 23:48:40 2012
> @@ -40,6 +40,7 @@ import java.util.TreeSet;
> import javax.sql.rowset.serial.SerialBlob;
> import javax.sql.rowset.serial.SerialClob;
>
> +import javolution.util.FastList;
> import javolution.util.FastMap;
>
> import org.ofbiz.base.util.Debug;
> @@ -54,6 +55,7 @@ import org.ofbiz.entity.GenericNotImplem
> import org.ofbiz.entity.GenericValue;
> import org.ofbiz.entity.condition.EntityCondition;
> import org.ofbiz.entity.condition.EntityConditionParam;
> +import org.ofbiz.entity.condition.EntityOperator;
> import org.ofbiz.entity.condition.OrderByList;
> import org.ofbiz.entity.config.DatasourceInfo;
> import org.ofbiz.entity.jdbc.JdbcValueHandler;
> @@ -420,11 +422,31 @@ public class SqlJdbcUtil {
>             }
>             sql.append(makeFromClause(modelEntity, modelFieldTypeReader, datasourceInfo));
>             String viewWhereClause = makeViewWhereClause(modelEntity, datasourceInfo.joinStyle);
> -            if (UtilValidate.isNotEmpty(viewWhereClause)) {
> +            ModelViewEntity modelViewEntity = (ModelViewEntity)modelEntity;
> +            List<EntityCondition> whereConditions = FastList.newInstance();
> +            List<EntityCondition> havingConditions = FastList.newInstance();
> +            List<String> orderByList = FastList.newInstance();
> +
> +            modelViewEntity.populateViewEntityConditionInformation(modelFieldTypeReader, whereConditions, havingConditions, orderByList, null);
> +            String viewConditionClause;
> +            if (!whereConditions.isEmpty()) {
> +                viewConditionClause = EntityCondition.makeCondition(whereConditions, EntityOperator.AND).makeWhereString(modelViewEntity,  null, datasourceInfo);
> +            } else {
> +                viewConditionClause = null;
> +            }
> +            if (UtilValidate.isNotEmpty(viewWhereClause) || UtilValidate.isNotEmpty(viewConditionClause)) {
>                 sql.append(" WHERE ");
> -                sql.append(viewWhereClause);
> +                if (UtilValidate.isNotEmpty(viewWhereClause)) {
> +                    sql.append("(").append(viewWhereClause).append(")");
> +                    if (UtilValidate.isNotEmpty(viewConditionClause)) {
> +                        sql.append(" AND ");
> +                    }
> +                }
> +                if (UtilValidate.isNotEmpty(viewConditionClause)) {
> +                    sql.append("(").append(viewConditionClause).append(")");
> +                }
>             }
> -            ModelViewEntity modelViewEntity = (ModelViewEntity)modelEntity;
> +            // FIXME: handling HAVING, don't need ORDER BY for nested views
>             modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), sql, " GROUP BY ", ", ", "", false);
>
>             sql.append(")");
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=1327735&r1=1327734&r2=1327735&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Wed Apr 18 23:48:40 2012
> @@ -315,16 +315,6 @@ public class ModelViewEntity extends Mod
>                 orderByList.addAll(currentOrderByList);
>             }
>         }
> -
> -        for (Map.Entry<String, String> memberEntityEntry: this.memberModelEntities.entrySet()) {
> -            ModelEntity modelEntity = this.getModelReader().getModelEntityNoCheck(memberEntityEntry.getValue());
> -            if (modelEntity instanceof ModelViewEntity) {
> -                ModelViewEntity memberViewEntity = (ModelViewEntity) modelEntity;
> -                entityAliasStack.add(memberEntityEntry.getKey());
> -                memberViewEntity.populateViewEntityConditionInformation(modelFieldTypeReader, whereConditions, havingConditions, orderByList, entityAliasStack);
> -                entityAliasStack.remove(entityAliasStack.size() - 1);
> -            }
> -        }
>     }
>
>     @Deprecated @Override
>
>

[Adam Heath-2]

On 04/18/2012 09:14 PM, Scott Gray wrote:
> Nice, I ran into a problem related to this very recently.  Thanks for taking care of it.

<view-entity>
  <member-entity entity-name="Foo">
<view-entity>

<view-entity entity-name="Foo">
  <member-entity alias="A" entity-name="Bar">
  <member-entity alias="B" entity-name="Baz">
  <view-link entity-alias="A" rel-entity-alias="B">
    <view-condition/>
  </view-link>
</view-entity>

<entity entity-name="Bar"/>

<view-entity entity-name="Baz">
  <view-condition/>
</view-entity>

This now works.

>
> Regards
> Scott
>
> On 19/04/2012, at 11:48 AM, [hidden email] wrote:
>
>> Author: doogie
>> Date: Wed Apr 18 23:48:40 2012
>> New Revision: 1327735
>>
>> URL: http://svn.apache.org/viewvc?rev=1327735&view=rev
>> Log:
>> FEATURE/FIX: <entity-condition> on views are now done correctly.  When a
>> view was previously joined to another view, the conditions on the inner
>> view were added to the outer-most WHERE clause.  This caused multiple
>> problems.  One, if the join was optional, the generated sql actually made
>> it required(because it was in the WHERE, and not in the ON clause).
>> Second, the outer WHERE may reference a field that was not available *at
>> all* in the generated nested select(view table).
>>
>> The fix is to *not* recurse thru all view member entities when finding
>> conditions, and instead attach them to a new WHERE clause on the
>> generated inner SELECT(view table).
>>
>> Modified:
>>    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>>    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>>
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java?rev=1327735&r1=1327734&r2=1327735&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java (original)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java Wed Apr 18 23:48:40 2012
>> @@ -40,6 +40,7 @@ import java.util.TreeSet;
>> import javax.sql.rowset.serial.SerialBlob;
>> import javax.sql.rowset.serial.SerialClob;
>>
>> +import javolution.util.FastList;
>> import javolution.util.FastMap;
>>
>> import org.ofbiz.base.util.Debug;
>> @@ -54,6 +55,7 @@ import org.ofbiz.entity.GenericNotImplem
>> import org.ofbiz.entity.GenericValue;
>> import org.ofbiz.entity.condition.EntityCondition;
>> import org.ofbiz.entity.condition.EntityConditionParam;
>> +import org.ofbiz.entity.condition.EntityOperator;
>> import org.ofbiz.entity.condition.OrderByList;
>> import org.ofbiz.entity.config.DatasourceInfo;
>> import org.ofbiz.entity.jdbc.JdbcValueHandler;
>> @@ -420,11 +422,31 @@ public class SqlJdbcUtil {
>>             }
>>             sql.append(makeFromClause(modelEntity, modelFieldTypeReader, datasourceInfo));
>>             String viewWhereClause = makeViewWhereClause(modelEntity, datasourceInfo.joinStyle);
>> -            if (UtilValidate.isNotEmpty(viewWhereClause)) {
>> +            ModelViewEntity modelViewEntity = (ModelViewEntity)modelEntity;
>> +            List<EntityCondition> whereConditions = FastList.newInstance();
>> +            List<EntityCondition> havingConditions = FastList.newInstance();
>> +            List<String> orderByList = FastList.newInstance();
>> +
>> +            modelViewEntity.populateViewEntityConditionInformation(modelFieldTypeReader, whereConditions, havingConditions, orderByList, null);
>> +            String viewConditionClause;
>> +            if (!whereConditions.isEmpty()) {
>> +                viewConditionClause = EntityCondition.makeCondition(whereConditions, EntityOperator.AND).makeWhereString(modelViewEntity,  null, datasourceInfo);
>> +            } else {
>> +                viewConditionClause = null;
>> +            }
>> +            if (UtilValidate.isNotEmpty(viewWhereClause) || UtilValidate.isNotEmpty(viewConditionClause)) {
>>                 sql.append(" WHERE ");
>> -                sql.append(viewWhereClause);
>> +                if (UtilValidate.isNotEmpty(viewWhereClause)) {
>> +                    sql.append("(").append(viewWhereClause).append(")");
>> +                    if (UtilValidate.isNotEmpty(viewConditionClause)) {
>> +                        sql.append(" AND ");
>> +                    }
>> +                }
>> +                if (UtilValidate.isNotEmpty(viewConditionClause)) {
>> +                    sql.append("(").append(viewConditionClause).append(")");
>> +                }
>>             }
>> -            ModelViewEntity modelViewEntity = (ModelViewEntity)modelEntity;
>> +            // FIXME: handling HAVING, don't need ORDER BY for nested views
>>             modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), sql, " GROUP BY ", ", ", "", false);
>>
>>             sql.append(")");
>>
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=1327735&r1=1327734&r2=1327735&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Wed Apr 18 23:48:40 2012
>> @@ -315,16 +315,6 @@ public class ModelViewEntity extends Mod
>>                 orderByList.addAll(currentOrderByList);
>>             }
>>         }
>> -
>> -        for (Map.Entry<String, String> memberEntityEntry: this.memberModelEntities.entrySet()) {
>> -            ModelEntity modelEntity = this.getModelReader().getModelEntityNoCheck(memberEntityEntry.getValue());
>> -            if (modelEntity instanceof ModelViewEntity) {
>> -                ModelViewEntity memberViewEntity = (ModelViewEntity) modelEntity;
>> -                entityAliasStack.add(memberEntityEntry.getKey());
>> -                memberViewEntity.populateViewEntityConditionInformation(modelFieldTypeReader, whereConditions, havingConditions, orderByList, entityAliasStack);
>> -                entityAliasStack.remove(entityAliasStack.size() - 1);
>> -            }
>> -        }
>>     }
>>
>>     @Deprecated @Override
>>
>>
>