Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-5701

Bug SQL Count Distinct command in GenericDAO.java

    Details

    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      I have encounter the problem is: How can ofbiz framework count distinct for all selected fields in query function of delegator? Although EntityFindOptions.setDistinct(true) and the selected fields are all field, the ofbiz framework always count distinct follow only one field.

      I have resolved problem as my attact file

      1. GenericDAO.java.patch
        4 kB
        Jacques Le Roux
      2. GenericDAO.java.patch
        3 kB
        Jacques Le Roux
      3. SQLDistinctGenericDAO.diff
        3 kB
        kieuanhvu
      4. SQLDistinctGenericDAO.diff
        3 kB
        kieuanhvu

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi kieuanhvu,

          At revision: 1810054, I have reverted your patch (all branches) because it's not only a problem with Dynamic View Entities as reported by Pawan but also an issue with MSSQL as reported by Wei Zhang at OFBIZ-9676.

          Could you please see if you can provide a patch which is not an issue in these both cases?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi kieuanhvu, At revision: 1810054, I have reverted your patch (all branches) because it's not only a problem with Dynamic View Entities as reported by Pawan but also an issue with MSSQL as reported by Wei Zhang at OFBIZ-9676 . Could you please see if you can provide a patch which is not an issue in these both cases?
          Hide
          pawan.verma Pawan Verma added a comment -

          Hey Guys,
          I have tested this flow with both Static and Dynamic View Entity. It is working fine for the Static View(Tested for PartyAndPerson entity).
          But in case of the Dynamic View, it is not working. Previously it was not applying distinct but now method getResultsSizeAfterPartialList() always returns 1 in all the cases.
          Below is my testing View Entity:

          workEffortAndGoodStandard = new DynamicViewEntity();
          
          workEffortAndGoodStandard.addMemberEntity("WE", "WorkEffort");
          workEffortAndGoodStandard.addAlias("WE", "workEffortId",null, null, false, true, null);
          workEffortAndGoodStandard.addAlias("WE", "workEffortPurposeTypeId",null, null, false, true, null);
          
          workEffortAndGoodStandard.addMemberEntity("WEGS", "WorkEffortGoodStandard");
          workEffortAndGoodStandard.addAlias("WEGS", "workEffortId", null, null, false, true, null);
          workEffortAndGoodStandard.addAlias("WEGS", "productId", null, null, true, true, null);
          workEffortAndGoodStandard.addViewLink("WEGS", "WE", Boolean.FALSE, ModelKeyMap.makeKeyMapList("workEffortId", "workEffortId"));
          
          workEffortAndGoodStandardListItr = select("productId").from(workEffortAndGoodStandard).distinct().queryIterator();
          workEffortAndGoodStandardListSize = workEffortAndGoodStandardListItr.getResultsSizeAfterPartialList();
          
          Show
          pawan.verma Pawan Verma added a comment - Hey Guys, I have tested this flow with both Static and Dynamic View Entity. It is working fine for the Static View(Tested for PartyAndPerson entity). But in case of the Dynamic View, it is not working. Previously it was not applying distinct but now method getResultsSizeAfterPartialList() always returns 1 in all the cases. Below is my testing View Entity: workEffortAndGoodStandard = new DynamicViewEntity(); workEffortAndGoodStandard.addMemberEntity( "WE" , "WorkEffort" ); workEffortAndGoodStandard.addAlias( "WE" , "workEffortId" , null , null , false , true , null ); workEffortAndGoodStandard.addAlias( "WE" , "workEffortPurposeTypeId" , null , null , false , true , null ); workEffortAndGoodStandard.addMemberEntity( "WEGS" , "WorkEffortGoodStandard" ); workEffortAndGoodStandard.addAlias( "WEGS" , "workEffortId" , null , null , false , true , null ); workEffortAndGoodStandard.addAlias( "WEGS" , "productId" , null , null , true , true , null ); workEffortAndGoodStandard.addViewLink( "WEGS" , "WE" , Boolean .FALSE, ModelKeyMap.makeKeyMapList( "workEffortId" , "workEffortId" )); workEffortAndGoodStandardListItr = select( "productId" ).from(workEffortAndGoodStandard).distinct().queryIterator(); workEffortAndGoodStandardListSize = workEffortAndGoodStandardListItr.getResultsSizeAfterPartialList();
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks kieuanhvu,

          Your patch is in
          trunk r1804319
          R16.11 r1804320
          R15.12 r1804321
          R14.12 r1804322
          R13.07 r1804323

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks kieuanhvu, Your patch is in trunk r1804319 R16.11 r1804320 R15.12 r1804321 R14.12 r1804322 R13.07 r1804323
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Updated patch

          Show
          jacques.le.roux Jacques Le Roux added a comment - Updated patch
          Hide
          Renuka_Srishti Renuka Srishti added a comment -

          I think this issue exists in the case of View Entity, for more details you can refer OFBIZ-9428.

          Show
          Renuka_Srishti Renuka Srishti added a comment - I think this issue exists in the case of View Entity, for more details you can refer OFBIZ-9428 .
          Hide
          rishisolankii Rishi Solanki added a comment -

          kieuanhvu: Picked this one but could not understand what you wants to say, could you please explain it one more time in some other way which should be really easier to understand (may be you can use some example records to explain). From your last comment it looks like not setting the setDistinct will give you some expected results then what is the need to patch you have provided.
          In short if you could help to understand the problem better then we can help. Also if you are able to achieve your expected results by setting and not setting the distinct value then this should not be bug. So if no problem then this should be consider as invalid and recommending to close as invalid.

          Please provide more details if this is an valid issue in GenericDAO or it is something expected in code and not an real issue?

          Thanks!

          Show
          rishisolankii Rishi Solanki added a comment - kieuanhvu : Picked this one but could not understand what you wants to say, could you please explain it one more time in some other way which should be really easier to understand (may be you can use some example records to explain). From your last comment it looks like not setting the setDistinct will give you some expected results then what is the need to patch you have provided. In short if you could help to understand the problem better then we can help. Also if you are able to achieve your expected results by setting and not setting the distinct value then this should not be bug. So if no problem then this should be consider as invalid and recommending to close as invalid. Please provide more details if this is an valid issue in GenericDAO or it is something expected in code and not an real issue? Thanks!
          Hide
          kieuanhvu kieuanhvu added a comment - - edited

          Hi, Jacques Le Roux, my use case as following:
          EntityFindOptions options = new EntityFindOptions(); // Line 1
          options.setDistinct(true); // Line 2
          options.setMaxRows(viewSize * (viewIndex + 1));// viewSize = 10, viewIndex = 0
          lowIndex = viewIndex * viewSize + 1;
          highIndex = (viewIndex + 1) * viewSize;
          try {
          EntityListIterator partyListIt = delegator.find("PartyRoleAndPartyDetail",
          EntityCondition.makeCondition(conditions, EntityOperator.AND), null, null, UtilMisc.toList("createdDate DESC"), options);
          List<GenericValue> partyList = partyListIt.getPartialList(lowIndex, viewSize); // Line 9
          int partyListSize = partyListIt.getResultsSizeAfterPartialList(); // Line 10
          partyListIt.close();
          if(highIndex > partyListSize)

          { highIndex = partyListSize; }

          /...../
          }catch(GenericEntityException e){
          }

          If I delete line 2 in my code, the partyListSize variable will have result as my expected (In my specific case, partyListSize = 19)
          If I don't delete line 2, the partyListSize variable will equals 1, but size of partyList, in line 9, is 10 (because the viewSize = 10).

          Show
          kieuanhvu kieuanhvu added a comment - - edited Hi, Jacques Le Roux, my use case as following: EntityFindOptions options = new EntityFindOptions(); // Line 1 options.setDistinct(true); // Line 2 options.setMaxRows(viewSize * (viewIndex + 1));// viewSize = 10, viewIndex = 0 lowIndex = viewIndex * viewSize + 1; highIndex = (viewIndex + 1) * viewSize; try { EntityListIterator partyListIt = delegator.find("PartyRoleAndPartyDetail", EntityCondition.makeCondition(conditions, EntityOperator.AND), null, null, UtilMisc.toList("createdDate DESC"), options); List<GenericValue> partyList = partyListIt.getPartialList(lowIndex, viewSize); // Line 9 int partyListSize = partyListIt.getResultsSizeAfterPartialList(); // Line 10 partyListIt.close(); if(highIndex > partyListSize) { highIndex = partyListSize; } / ..... / }catch(GenericEntityException e){ } If I delete line 2 in my code, the partyListSize variable will have result as my expected (In my specific case, partyListSize = 19) If I don't delete line 2, the partyListSize variable will equals 1, but size of partyList, in line 9, is 10 (because the viewSize = 10).
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          OK, I will test the feature. Would you have an easy use case for me to test?

          Show
          jacques.le.roux Jacques Le Roux added a comment - OK, I will test the feature. Would you have an easy use case for me to test?
          Hide
          kieuanhvu kieuanhvu added a comment -

          Hi Jacques Le Roux, I've tested your path, it's same as my patch!

          Show
          kieuanhvu kieuanhvu added a comment - Hi Jacques Le Roux, I've tested your path, it's same as my patch!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi kieuanhvu, did you get a chance to test my patch?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi kieuanhvu, did you get a chance to test my patch?
          Hide
          jacques.le.roux Jacques Le Roux added a comment - - edited

          If you and I have done things correctly this would be your patch. Just C/Pasted your patch and done by hand, no tests nor any thinking so far...

          Show
          jacques.le.roux Jacques Le Roux added a comment - - edited If you and I have done things correctly this would be your patch. Just C/Pasted your patch and done by hand, no tests nor any thinking so far...
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Are you really reading what I'm writing? Did you try with a fresh checkout?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Are you really reading what I'm writing? Did you try with a fresh checkout?
          Hide
          kieuanhvu kieuanhvu added a comment -

          Hi, Jacques Le Roux, I have modified tabs by 4 spaces instead of 3 spaces!!!!!

          Show
          kieuanhvu kieuanhvu added a comment - Hi, Jacques Le Roux, I have modified tabs by 4 spaces instead of 3 spaces!!!!!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          It seems you did not get my point. For an unknow reason (to me) this patch is not valid. It seems so far you only replaced tabs by 4 spaces but did not fix the reason applying the patch does not work. I still get the same issue than above. You can try the same by checking out a fresh copy of the trunk and applying your patch on it usin "patch -p1 < SQLDistinctGenericDAO.diff"

          Show
          jacques.le.roux Jacques Le Roux added a comment - It seems you did not get my point. For an unknow reason (to me) this patch is not valid. It seems so far you only replaced tabs by 4 spaces but did not fix the reason applying the patch does not work. I still get the same issue than above. You can try the same by checking out a fresh copy of the trunk and applying your patch on it usin "patch -p1 < SQLDistinctGenericDAO.diff"
          Hide
          kieuanhvu kieuanhvu added a comment -

          Sorry, I have modified the path file!!!

          Show
          kieuanhvu kieuanhvu added a comment - Sorry, I have modified the path file!!!
          Hide
          jacques.le.roux Jacques Le Roux added a comment - - edited

          Sorry but these files are same, apart that the last one has only 1 tab in it !

          Show
          jacques.le.roux Jacques Le Roux added a comment - - edited Sorry but these files are same, apart that the last one has only 1 tab in it !
          Hide
          kieuanhvu kieuanhvu added a comment -

          Hi, Jacques Le Roux
          I have reattach my patch file,
          Thanks for your response!!

          Show
          kieuanhvu kieuanhvu added a comment - Hi, Jacques Le Roux I have reattach my patch file, Thanks for your response!!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          This is not a valid patch

          patch -p1 <patch.patch
          patching file framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java
          patch: **** malformed patch at line 12: modelViewEntity = (ModelViewEntity) modelEntity;

          Would be better if you could provide a patch in svn format. Also replace tabs by 4 spaces

          Thanks

          Show
          jacques.le.roux Jacques Le Roux added a comment - This is not a valid patch patch -p1 <patch.patch patching file framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java patch: **** malformed patch at line 12: modelViewEntity = (ModelViewEntity) modelEntity; Would be better if you could provide a patch in svn format. Also replace tabs by 4 spaces Thanks

            People

            • Assignee:
              Unassigned
              Reporter:
              kieuanhvu kieuanhvu
            • Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:

                Development

                  Agile