Uploaded image for project: 'OpenJPA'
  1. OpenJPA
  2. OPENJPA-2534

A boolean is not converted correct when using the hint 'UseLiteralInSQL'.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2.1.1, 2.2.3, 2.3.0, 2.4.0
    • Fix Version/s: 2.1.2, 2.2.1.1, 2.2.3, 2.4.0
    • Component/s: sql
    • Labels:
      None

      Description

      The hint, 'openjpa.hint.UseLiteralInSQL', is not properly converting a boolean to an int. To illustrate this, lets take this entity:

      public class MyEntity implements Serializable {
      @Id
      private Integer pk;

      private boolean flag;
      ..........

      With this entity, lets take this test:

      Query q = em.createQuery("SELECT f FROM MyEntity f WHERE f.flag = true");
      q.setHint("openjpa.hint.UseLiteralInSQL", "false"); //false is the default
      q.getResultList();

      With this code, the following SQL will be generated:

      SELECT t0.pk, t0.code, t0.code2, t0.flag FROM MyEntity t0 WHERE (t0.flag = ?) [params=(int) 1]

      Next, lets take this code:

      Query q = em.createQuery("SELECT f FROM MyEntity f WHERE f.flag = true");
      q.setHint("openjpa.hint.UseLiteralInSQL", "true");
      q.getResultList();

      With this code, the following SQL will be generated:

      SELECT t0.pk, t0.code, t0.code2, t0.flag FROM MyEntity t0 WHERE (t0.flag = true)

      However, in the latter case where UseLiteralInSQL=true, the following SQL should be generated:

      SELECT t0.pk, t0.code, t0.code2, t0.flag FROM MyEntity t0 WHERE (t0.flag = '1')

      Thanks,

      Heath

      1. OPENJPA-2534.patch
        16 kB
        Mark Struberg
      2. OPENJPA-2534.patch
        4 kB
        Rick Curtis

        Issue Links

          Activity

          Hide
          jpaheath Heath Thomann added a comment -

          Attaching a test to replicate the issue described.

          Thanks,

          Heath

          Show
          jpaheath Heath Thomann added a comment - Attaching a test to replicate the issue described. Thanks, Heath
          Hide
          curtisr7 Rick Curtis added a comment -

          Attaching a test case and code fix for review.

          Show
          curtisr7 Rick Curtis added a comment - Attaching a test case and code fix for review.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1636464 from Heath Thomann in branch 'openjpa/branches/2.1.x'
          [ https://svn.apache.org/r1636464 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.1.x.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1636464 from Heath Thomann in branch 'openjpa/branches/2.1.x' [ https://svn.apache.org/r1636464 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.1.x.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1636472 from Heath Thomann in branch 'openjpa/branches/2.2.1.x'
          [ https://svn.apache.org/r1636472 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.2.1.x.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1636472 from Heath Thomann in branch 'openjpa/branches/2.2.1.x' [ https://svn.apache.org/r1636472 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.2.1.x.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1636473 from Heath Thomann in branch 'openjpa/branches/2.2.x'
          [ https://svn.apache.org/r1636473 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.2.x.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1636473 from Heath Thomann in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1636473 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to 2.2.x.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1636474 from Heath Thomann in branch 'openjpa/trunk'
          [ https://svn.apache.org/r1636474 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to trunk.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1636474 from Heath Thomann in branch 'openjpa/trunk' [ https://svn.apache.org/r1636474 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - applied Rick's patch to trunk.
          Hide
          struberg Mark Struberg added a comment -

          I actually find this very hint not really useful at all.

          I currently face a pretty similar issue. My customer doesn't have boolean values represented as NUMBER(1) in his database but as CHAR(1) containing '1' and '0'.

          Of course I also have seen 'Y'/'N' and 'T'/'F' as well already! The problem is that Oracle didn't define any native datatype for boolean for a very long time.

          I'm tempted to add it to DBDictionary and add an enum for those values

          BOOLEAN_INT_01
          BOOLEAN_STRING_01
          BOOLEAN_STRING_YN
          BOOLEAN_STRING_TF

          and use them in DBDictionary#setBoolean.
          Wdyt? Any better ideas?

          PS: imo we cannot just have an Object booleanTrueRepresentation and Object booleanFalseRepresentation as we really need to invoke different methods on the PreparedStatement.

          Show
          struberg Mark Struberg added a comment - I actually find this very hint not really useful at all. I currently face a pretty similar issue. My customer doesn't have boolean values represented as NUMBER(1) in his database but as CHAR(1) containing '1' and '0'. Of course I also have seen 'Y'/'N' and 'T'/'F' as well already! The problem is that Oracle didn't define any native datatype for boolean for a very long time. I'm tempted to add it to DBDictionary and add an enum for those values BOOLEAN_INT_01 BOOLEAN_STRING_01 BOOLEAN_STRING_YN BOOLEAN_STRING_TF and use them in DBDictionary#setBoolean. Wdyt? Any better ideas? PS: imo we cannot just have an Object booleanTrueRepresentation and Object booleanFalseRepresentation as we really need to invoke different methods on the PreparedStatement.
          Hide
          allee8285 Albert Lee added a comment -

          Rather than introducing the BOOLEAN_* type, have you considered using @Externalizer to attribute <-> db column conversion.

          http://ci.apache.org/projects/openjpa/trunk/docbook/manual.html#ref_guide_pc_extern

          Just a thought.

          Show
          allee8285 Albert Lee added a comment - Rather than introducing the BOOLEAN_* type, have you considered using @Externalizer to attribute <-> db column conversion. http://ci.apache.org/projects/openjpa/trunk/docbook/manual.html#ref_guide_pc_extern Just a thought.
          Hide
          struberg Mark Struberg added a comment -

          I know @Externalizer and also @ExternalValue and both are not an option. That would require us to annotate each and every boolean field. The point is that our current DBDictionary hardcoded uses (int) 0/1 and doesn't support native boolean at all. This is imo fundamentally false. Some databases which are SQL99 compatible support Boolean as own native type. For those we should imo use PreparedStatement.setBoolean() and not setInt() as we currently do! ExternalValue does not help much as Boolean always ends up as setInt atm.
          And for Oracle and others who don't know any native Boolean it is basically up to each installation on how they represent boolean in their db schema. I've seen the ones I mentioned above but I'm sure there are others as well.

          So I would probably also add BOOLEAN_BOOLEAN. This should of course be able to get changed via our normal ways: properties, specialized DBDictionary, etc.

          Show
          struberg Mark Struberg added a comment - I know @Externalizer and also @ExternalValue and both are not an option. That would require us to annotate each and every boolean field. The point is that our current DBDictionary hardcoded uses (int) 0/1 and doesn't support native boolean at all. This is imo fundamentally false. Some databases which are SQL99 compatible support Boolean as own native type. For those we should imo use PreparedStatement.setBoolean() and not setInt() as we currently do! ExternalValue does not help much as Boolean always ends up as setInt atm. And for Oracle and others who don't know any native Boolean it is basically up to each installation on how they represent boolean in their db schema. I've seen the ones I mentioned above but I'm sure there are others as well. So I would probably also add BOOLEAN_BOOLEAN. This should of course be able to get changed via our normal ways: properties, specialized DBDictionary, etc.
          Hide
          struberg Mark Struberg added a comment -

          Another small note:
          In Oracle it's even not that easy to see that you got hit by that. Usually Oracle parses the (int) 1 in the query and converts it automatically to the target column type.
          There are of course 2 very nasty issues with this:
          1.) It gets much slower. Despite having an Index correctly set up it will simply not be used. Because of the required toNumber(). Most times you don't even see this in the explain.
          2.) Some queries even blow up completely. Somehow Oracle manages the coercion halfway decent with normal queries - but if you use case statements it simply blows up complaining about a parameter mismatch.

          Show
          struberg Mark Struberg added a comment - Another small note: In Oracle it's even not that easy to see that you got hit by that. Usually Oracle parses the (int) 1 in the query and converts it automatically to the target column type. There are of course 2 very nasty issues with this: 1.) It gets much slower. Despite having an Index correctly set up it will simply not be used. Because of the required toNumber(). Most times you don't even see this in the explain. 2.) Some queries even blow up completely. Somehow Oracle manages the coercion halfway decent with normal queries - but if you use case statements it simply blows up complaining about a parameter mismatch.
          Hide
          struberg Mark Struberg added a comment -

          Oh and a clarification. When I said:
          > Of course I also have seen 'Y'/'N' and 'T'/'F' as well already!
          then I meant other installations / other customers. Usually a single installation only uses one of the possible representations. If they mix it that then they really need to use @ExternalValue (or map it in orm.xml).

          Show
          struberg Mark Struberg added a comment - Oh and a clarification. When I said: > Of course I also have seen 'Y'/'N' and 'T'/'F' as well already! then I meant other installations / other customers. Usually a single installation only uses one of the possible representations. If they mix it that then they really need to use @ExternalValue (or map it in orm.xml).
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1650957 from Mark Struberg in branch 'openjpa/trunk'
          [ https://svn.apache.org/r1650957 ]

          OPENJPA-2534 cleanup DBDictionary alias handling

          use class.getName() to prevent typos - no functional change yet.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1650957 from Mark Struberg in branch 'openjpa/trunk' [ https://svn.apache.org/r1650957 ] OPENJPA-2534 cleanup DBDictionary alias handling use class.getName() to prevent typos - no functional change yet.
          Hide
          struberg Mark Struberg added a comment -

          This patch implements an interface for a BooleanRepresentation + config.
          It also enhances the DBDictionary to make use of it.

          Tested with an external test app so far. Will write unit tests and commit all this stuff within the next days.

          Show
          struberg Mark Struberg added a comment - This patch implements an interface for a BooleanRepresentation + config. It also enhances the DBDictionary to make use of it. Tested with an external test app so far. Will write unit tests and commit all this stuff within the next days.
          Hide
          struberg Mark Struberg added a comment -

          I've created OPENJPA-2558 to handle the BooleanRepresentation in a separate report.

          Show
          struberg Mark Struberg added a comment - I've created OPENJPA-2558 to handle the BooleanRepresentation in a separate report.
          Hide
          jpaheath Heath Thomann added a comment -

          The issue described in this JIRA is fixed, as such I will close this JIRA. I should have done so sooner. Mark's idea's should be taken up in a different/new (but related?) JIRA. Sorry for not closing this sooner.

          Thanks,

          Heath

          Show
          jpaheath Heath Thomann added a comment - The issue described in this JIRA is fixed, as such I will close this JIRA. I should have done so sooner. Mark's idea's should be taken up in a different/new (but related?) JIRA. Sorry for not closing this sooner. Thanks, Heath
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1673491 from Heath Thomann in branch 'openjpa/branches/2.1.x'
          [ https://svn.apache.org/r1673491 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1673491 from Heath Thomann in branch 'openjpa/branches/2.1.x' [ https://svn.apache.org/r1673491 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1673495 from Heath Thomann in branch 'openjpa/branches/2.2.1.x'
          [ https://svn.apache.org/r1673495 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1673495 from Heath Thomann in branch 'openjpa/branches/2.2.1.x' [ https://svn.apache.org/r1673495 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1673502 from Heath Thomann in branch 'openjpa/branches/2.2.x'
          [ https://svn.apache.org/r1673502 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1673502 from Heath Thomann in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1673502 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1673509 from Heath Thomann in branch 'openjpa/trunk'
          [ https://svn.apache.org/r1673509 ]

          OPENJPA-2534: A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1673509 from Heath Thomann in branch 'openjpa/trunk' [ https://svn.apache.org/r1673509 ] OPENJPA-2534 : A boolean is not converted correct when using the hint 'UseLiteralInSQL' - made test update.

            People

            • Assignee:
              struberg Mark Struberg
              Reporter:
              jpaheath Heath Thomann
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development