You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by do...@apache.org on 2012/04/19 01:48:41 UTC
svn commit: r1327735 - in
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity: jdbc/SqlJdbcUtil.java
model/ModelViewEntity.java
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
Re: svn commit: r1327735 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity:
jdbc/SqlJdbcUtil.java model/ModelViewEntity.java
Posted by Adam Heath <do...@brainfood.com>.
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, doogie@apache.org 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
>>
>>
>
Re: svn commit: r1327735 - in /ofbiz/trunk/framework/entity/src/org/ofbiz/entity: jdbc/SqlJdbcUtil.java model/ModelViewEntity.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
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, doogie@apache.org 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
>
>