OpenJPA
  1. OpenJPA
  2. OPENJPA-1584

PreparedQuery gives wrong result if query has subquery and parameters are used in both main select and subselect

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.1.0
    • Component/s: performance
    • Labels:
      None

      Description

      a back-to-back of the following JPQL query providing different set of parameter values,
      the second execution gives wrong answer.

      "select o from OrderJPA o where o.OrderId in (select max(o1.OrderId) from OrderJPA o1 where ((o1.CustomerId = :customerId) and (o1.DistrictId = :districtId) and (o1.WarehouseId = :warehouseId))) and (o.CustomerId = :customerId) and (o.DistrictId = :districtId) and (o.WarehouseId = :warehouseId)"

      SQL trace shown the first time query execution, let say customerId=339, districtId=3, warehouseId=23, then query returns 1 row:

      the SQL trace looked fine:

      [3/16/10 17:40:36:831 CDT] 00000045 OpenJPA 3 openjpa.jdbc.SQL: Trace: <t 241897067, conn 1981117973> executing prepstmnt 1547852866 SELECT t0.O_D_ID, t0.O_ID, t0.O_W_ID, t0.VERSION, t0.O_ALL_LOCAL, t0.O_CARRIER_ID, t0.O_C_ID, t0.O_ENTRY_D, t0.O_OL_CNT FROM ORDERS t0 WHERE (t0.O_ID IN (SELECT MAX(t1.O_ID) FROM ORDERS t1 WHERE (t1.O_C_ID = ? AND t1.O_D_ID = ? AND t1.O_W_ID = ?) ) AND t0.O_C_ID = ? AND t0.O_D_ID = ? AND t0.O_W_ID = ?) optimize for 1 row [params=(short) 339, (short) 3, (short) 23, (short) 339, (short) 3, (short) 23]

      On the next execution of the same JPQL, the PreparedQueryImpl (which is cached before) gets reused.
      In processing user provided parameters, for example, customerId=2967, districtId=5, warehouseId=22,
      It is observed that the parameter values are incorrect: the last 3 values were incorrectly copied from the previously cached version.

      [3/16/10 17:45:42:411 CDT] 00000043 OpenJPA 3 openjpa.jdbc.SQL: Trace: <t 195496871, conn 1706649017> executing prepstmnt 1531796301 SELECT t0.O_D_ID, t0.O_ID, t0.O_W_ID, t0.VERSION, t0.O_ALL_LOCAL, t0.O_CARRIER_ID, t0.O_C_ID, t0.O_ENTRY_D, t0.O_OL_CNT FROM ORDERS t0 WHERE (t0.O_ID IN (SELECT MAX(t1.O_ID) FROM ORDERS t1 WHERE (t1.O_C_ID = ? AND t1.O_D_ID = ? AND t1.O_W_ID = ?) ) AND t0.O_C_ID = ? AND t0.O_D_ID = ? AND t0.O_W_ID = ?) optimize for 1 row [params=(short) 2967, (short) 5, (short) 22, (short) 339, (short) 3, (short) 23]

        Activity

        Hide
        Catalina Wei added a comment -

        Resolved in OPENJPA-1719 and OPENJPA-1738

        Show
        Catalina Wei added a comment - Resolved in OPENJPA-1719 and OPENJPA-1738
        Hide
        Donald Woods added a comment -

        Additional tests are always welcomed

        Show
        Donald Woods added a comment - Additional tests are always welcomed
        Hide
        Martin Dirichs added a comment -

        A modification to SQLBuffer has been committed in revision 925036 which supposedly corrected this issue. However, the committed change also has problems. The problem arises during the construction of a prepared statement when sub-selects are added to the main statement. In particular, the indexes of user parameters have to be adapted to their new positions within the resulting statement (which consists of the main statement and the sub-select). File org.apache.openjpa.jdbc.sql.SQLBuffer contains the following lines, which were committed with revision 925036 (lines 170++):

        // modify this buffer's user parameter index
        int otherSize = buf._userIndex.size()/2;
        for (int i = 0; i < _userIndex.size(); i+=2)

        { int newIndex = ((Integer)_userIndex.get(i)).intValue() + otherSize; _userIndex.set(i, newIndex); }

        // append the other buffer's user parameters to this one
        for (int i = 0; i < buf._userIndex.size(); i+=2)

        { Object otherIndex = buf._userIndex.get(i); Object otherParam = buf._userIndex.get(i+1); _userIndex.add(otherIndex); _userIndex.add(otherParam); }

        This code assumes that the sub-select user parameters, which are stored in buf._userIndex, always reside at positions in the combined statement before all the parameters of the main statement. If this is the case, the above code works (almost) correctly: All user parameters of the main statement get their indexes adapted, all sub-select user parameters are just copied without change. The code is only almost correct in this case, because the index offset calculation done with
        int otherSize = buf._userIndex.size()/2;
        is misguided - the offset should really be the number of all parameters (including non-user parameters) of the sub-select, not only the user parameters.

        The correct code is given below:

        // modify this buffer's user parameter index for all parameters
        // at or behind paramIndex, which is the insertion position
        int otherSize = buf._params.size();
        for (int i = 0; i < _userIndex.size(); i+=2)

        { int oldIndex = ((Integer)_userIndex.get(i)).intValue(); if (oldIndex >= paramIndex) _userIndex.set(i, oldIndex + otherSize); }

        // append the other buffer's user parameters to this one, their
        // position adapted according to the insertion position
        for (int i = 0; i < buf._userIndex.size(); i+=2)

        { int otherIndex = ((Integer) buf._userIndex.get(i)).intValue(); Object otherParam = buf._userIndex.get(i+1); _userIndex.add(otherIndex + paramIndex); _userIndex.add(otherParam); }

        Here, the insertion position at where the sub-select parameters get inserted into the main statement is propertly taken into account (paramIndex). User parameters of the main statement which reside before the insertion position remain unchanged, the position of all others gets offset by the number of parameters of the sub-select. The sub-select user parameters in turn are moved by an amount equal to the insertion position.

        A corresponding patch OpenJPA-2.0.0_OJ1584.patch containing the correct version of the code is attached. It would be better though to also add a new test method to TestPreparedQueryCache. If my help is welcome, I can create such a test case and supply it as well.

        Show
        Martin Dirichs added a comment - A modification to SQLBuffer has been committed in revision 925036 which supposedly corrected this issue. However, the committed change also has problems. The problem arises during the construction of a prepared statement when sub-selects are added to the main statement. In particular, the indexes of user parameters have to be adapted to their new positions within the resulting statement (which consists of the main statement and the sub-select). File org.apache.openjpa.jdbc.sql.SQLBuffer contains the following lines, which were committed with revision 925036 (lines 170++): // modify this buffer's user parameter index int otherSize = buf._userIndex.size()/2; for (int i = 0; i < _userIndex.size(); i+=2) { int newIndex = ((Integer)_userIndex.get(i)).intValue() + otherSize; _userIndex.set(i, newIndex); } // append the other buffer's user parameters to this one for (int i = 0; i < buf._userIndex.size(); i+=2) { Object otherIndex = buf._userIndex.get(i); Object otherParam = buf._userIndex.get(i+1); _userIndex.add(otherIndex); _userIndex.add(otherParam); } This code assumes that the sub-select user parameters, which are stored in buf._userIndex, always reside at positions in the combined statement before all the parameters of the main statement. If this is the case, the above code works (almost) correctly: All user parameters of the main statement get their indexes adapted, all sub-select user parameters are just copied without change. The code is only almost correct in this case, because the index offset calculation done with int otherSize = buf._userIndex.size()/2; is misguided - the offset should really be the number of all parameters (including non-user parameters) of the sub-select, not only the user parameters. The correct code is given below: // modify this buffer's user parameter index for all parameters // at or behind paramIndex, which is the insertion position int otherSize = buf._params.size(); for (int i = 0; i < _userIndex.size(); i+=2) { int oldIndex = ((Integer)_userIndex.get(i)).intValue(); if (oldIndex >= paramIndex) _userIndex.set(i, oldIndex + otherSize); } // append the other buffer's user parameters to this one, their // position adapted according to the insertion position for (int i = 0; i < buf._userIndex.size(); i+=2) { int otherIndex = ((Integer) buf._userIndex.get(i)).intValue(); Object otherParam = buf._userIndex.get(i+1); _userIndex.add(otherIndex + paramIndex); _userIndex.add(otherParam); } Here, the insertion position at where the sub-select parameters get inserted into the main statement is propertly taken into account (paramIndex). User parameters of the main statement which reside before the insertion position remain unchanged, the position of all others gets offset by the number of parameters of the sub-select. The sub-select user parameters in turn are moved by an amount equal to the insertion position. A corresponding patch OpenJPA-2.0.0_OJ1584.patch containing the correct version of the code is attached. It would be better though to also add a new test method to TestPreparedQueryCache. If my help is welcome, I can create such a test case and supply it as well.

          People

          • Assignee:
            Catalina Wei
            Reporter:
            Catalina Wei
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development