Derby
  1. Derby
  2. DERBY-5501

Subquery is only allowed to return a single column - When using derby with hibernate (or JPA) queries are created per JPA spec. For tables with multi-column PK, subqueries are created with two columns in select clause.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.0.2.0, 10.0.2.1, 10.1.1.0, 10.1.2.1, 10.1.3.1, 10.2.1.6, 10.2.2.0, 10.3.1.4, 10.3.2.1, 10.3.3.0, 10.4.1.3, 10.4.2.0, 10.5.1.1, 10.5.2.0, 10.5.3.0, 10.6.1.0, 10.6.2.1, 10.7.1.1, 10.8.1.2, 10.8.2.2
    • Fix Version/s: 10.9.1.0
    • Component/s: SQL
    • Labels:
      None
    • Environment:
      Max/Linux
    • Issue & fix info:
      Repro attached
    • Bug behavior facts:
      Seen in production

      Description

      ERROR: Subquery is only allowed to return a single column.
      PROBLEM: When using derby with hibernate (or JPA) queries are created by the JPA engine per JPA spec. For tables with multi-column PK, subqueries are created with two columns in select clause (see select colofassig6_.activityID, colofassig6_.assigneeID from Assignment in the query below).
      Without this support, I can not use Derby with JPA.

      Hibernate: select distinct activitybe0_.activityID as activityID69_, activitybe0_.createdBy as createdBy69_, activitybe0_.createdOn as createdOn69_, activitybe0_.lastModifiedBy as lastModi4_69_, activitybe0_.lastModifiedOn as lastModi5_69_, activitybe0_.activityDate as activity6_69_, activitybe0_.activityTypeHierarchyID as activity7_69_, activitybe0_.activityTypeID as activity8_69_, activitybe0_.campaignID as campaignID69_, activitybe0_.comments as comments69_, activitybe0_.description as descrip11_69_, activitybe0_.inputID as inputID69_, activitybe0_.inputTypeID as inputTy13_69_, activitybe0_.name as name69_, activitybe0_.notes as notes69_, activitybe0_.organizationID as organiz16_69_, activitybe0_.parentActivityTypeID as parentA17_69_ from Activity activitybe0_, Activity activitybe1_ inner join ActivitySchedule colofactiv2_ on activitybe1_.activityID=colofactiv2_.activityID inner join ActivityScheduleStatus colofactiv3_ on colofactiv2_.activityScheduleID=colofactiv3_.activityScheduleID inner join ActivityScheduleStatusType activitysc4_ on colofactiv3_.activityScheduleStatusTypeID=activitysc4_.activityScheduleStatusTypeID, ActivityTypeHierarchy activityty5_ where activitybe0_.activityTypeHierarchyID=activityty5_.activityTypeHierarchyID and activityty5_.activityTypeHierarchyID=? and not (exists (select colofassig6_.activityID, colofassig6_.assigneeID from Assignment colofassig6_ where activitybe0_.activityID=colofassig6_.activityID)) and (activitybe0_.activityID<>activitybe1_.activityID or activitysc4_.name<>'Route')
      2011-11-14 11:41:13,413 ERROR [org.hibernate.util.JDBCExceptionReporter] (EJB-Timer-1321288405420[target=jboss.j2ee:ear=oecrm1.6.3RC1-derby-jboss.ear,jar=builder-ejb.jar,name=WorkflowActivatorBean,service=EJB3]) Subquery is only allowed to return a single column.

      1. derby-5501-1.diff
        3 kB
        Dag H. Wanvik
      2. derby-5501-1.stat
        0.2 kB
        Dag H. Wanvik
      3. derby-5501-2.diff
        11 kB
        Dag H. Wanvik
      4. derby-5501-2.diff
        11 kB
        Dag H. Wanvik
      5. derby-5501-3.diff
        12 kB
        Dag H. Wanvik
      6. derby-5501-3.stat
        0.5 kB
        Dag H. Wanvik
      7. derby-5501-repro.diff
        1 kB
        Dag H. Wanvik
      There are no Sub-Tasks for this issue.

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        [bulk update] Close all resolved issues that haven't been updated for more than one year.

        Show
        Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.
        Hide
        Dag H. Wanvik added a comment -

        I'm on board with that, Knut. I discussed briefly with Rick offline if we might need a way to call out improvement fixes from the "noise" of errors fixes in the release notes when they aren't quite new features, but we decided to keep things as they are, so adding it to the feature list is fine, although probably stretching the definition of feature. Still better than to complicate things further..

        Show
        Dag H. Wanvik added a comment - I'm on board with that, Knut. I discussed briefly with Rick offline if we might need a way to call out improvement fixes from the "noise" of errors fixes in the release notes when they aren't quite new features, but we decided to keep things as they are, so adding it to the feature list is fine, although probably stretching the definition of feature. Still better than to complicate things further..
        Hide
        Knut Anders Hatlen added a comment -

        Just to clarify my comment about the release note needed flag: I think it's fine to mention this improvement as a new feature in the release notes, but that's not what the release note needed flag is used for. It's used to indicate to the release note generator that it should look for a releaseNote.html file that describes compatibility issues caused by the changes. I did not mean to suggest that the issue type should be changed from improvement to bug.

        I've added this issue to the 10.9 feature list at http://wiki.apache.org/db-derby/DerbyTenNineOneRelease to make the release manager remember to mention it.

        Show
        Knut Anders Hatlen added a comment - Just to clarify my comment about the release note needed flag: I think it's fine to mention this improvement as a new feature in the release notes, but that's not what the release note needed flag is used for. It's used to indicate to the release note generator that it should look for a releaseNote.html file that describes compatibility issues caused by the changes. I did not mean to suggest that the issue type should be changed from improvement to bug. I've added this issue to the 10.9 feature list at http://wiki.apache.org/db-derby/DerbyTenNineOneRelease to make the release manager remember to mention it.
        Hide
        Dag H. Wanvik added a comment -

        Thanks, I'll mail you the URL.

        Show
        Dag H. Wanvik added a comment - Thanks, I'll mail you the URL.
        Hide
        Sandeep Dixit added a comment -

        Sure - what is the URL to the binary snapshot?

        Thanks,
        Sandeep
        216-926-6696

        On Mon, Nov 21, 2011 at 4:24 PM, Dag H. Wanvik (Commented) (JIRA)

        Show
        Sandeep Dixit added a comment - Sure - what is the URL to the binary snapshot? – Thanks, Sandeep 216-926-6696 On Mon, Nov 21, 2011 at 4:24 PM, Dag H. Wanvik (Commented) (JIRA)
        Hide
        Dag H. Wanvik added a comment -

        Committed derby-5501-3 as svn 1204712, resolving. This is a back-port candidate.

        Sandeep, would you have a chance to validate this change in your environment either by compiling Derby trunk with the patch or downloading a binary snapshot?

        Show
        Dag H. Wanvik added a comment - Committed derby-5501-3 as svn 1204712, resolving. This is a back-port candidate. Sandeep, would you have a chance to validate this change in your environment either by compiling Derby trunk with the patch or downloading a binary snapshot?
        Hide
        Dag H. Wanvik added a comment -

        Uploading derby-5501-3, small polishing of patch 2 only.

        Show
        Dag H. Wanvik added a comment - Uploading derby-5501-3, small polishing of patch 2 only.
        Hide
        Dag H. Wanvik added a comment -

        No. It's the old knee jerk reaction I'm having here: improvements should be documented in the release documentation, if not in the release notes. I guess we could change the issue back to "bug" and be done with it.
        Regressions passed with patch #2.

        Show
        Dag H. Wanvik added a comment - No. It's the old knee jerk reaction I'm having here: improvements should be documented in the release documentation, if not in the release notes. I guess we could change the issue back to "bug" and be done with it. Regressions passed with patch #2.
        Hide
        Knut Anders Hatlen added a comment -

        Why would we need a release note for this issue? I can't spot anything obvious that would cause problems for existing applications.

        Show
        Knut Anders Hatlen added a comment - Why would we need a release note for this issue? I can't spot anything obvious that would cause problems for existing applications.
        Hide
        Dag H. Wanvik added a comment -

        Uploading version 2 of the patch. I adjusted the logic in verifySelectStarSubquery and setResultToBooleanTrueNode, although I wasn't able to make any example fail without adjusting them.

        1) If the test in verifySelectStarSubquery which throws SQLState.LANG_EXPOSED_NAME_NOT_FOUND is not made, the error will be caught later:

        Caused by: ERROR 42X10: 'FOO' is not an exposed table name in the scope in which it appears.
        at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278)
        at org.apache.derby.impl.sql.compile.FromList.expandAll(FromList.java:514)
        at org.apache.derby.impl.sql.compile.ResultColumnList.expandAllsAndNameColumns(ResultColumnList.java:1737)
        at org.apache.derby.impl.sql.compile.ResultColumnList.bindExpressions(ResultColumnList.java:825)
        at org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(SelectNode.java:555)
        at org.apache.derby.impl.sql.compile.SelectNode.bindTargetExpressions(SelectNode.java:734)
        at org.apache.derby.impl.sql.compile.SubqueryNode.bindExpression(SubqueryNode.java:532)

        so the check may be redundant. But I did adjust it so it catches the following bad in column 2 in the check in verifySelectStarSubquery (foo is bogus):

        select i from t5501b t1 where not exists (select t2.,foo. from t5501a t2 where t1.i=t2.i)

        2) As for setResultToBooleanTrueNode, i adjusted it so the following would now (also) be rewritten:

        select true, .. from .. -> select true from ..

        although not doing so didn't seem to matter.

        3) I also adjusted a negative test case in lang/subquery.sql to becoming a positive one.

        Rerunning regressions.

        Show
        Dag H. Wanvik added a comment - Uploading version 2 of the patch. I adjusted the logic in verifySelectStarSubquery and setResultToBooleanTrueNode, although I wasn't able to make any example fail without adjusting them. 1) If the test in verifySelectStarSubquery which throws SQLState.LANG_EXPOSED_NAME_NOT_FOUND is not made, the error will be caught later: Caused by: ERROR 42X10: 'FOO' is not an exposed table name in the scope in which it appears. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:278) at org.apache.derby.impl.sql.compile.FromList.expandAll(FromList.java:514) at org.apache.derby.impl.sql.compile.ResultColumnList.expandAllsAndNameColumns(ResultColumnList.java:1737) at org.apache.derby.impl.sql.compile.ResultColumnList.bindExpressions(ResultColumnList.java:825) at org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(SelectNode.java:555) at org.apache.derby.impl.sql.compile.SelectNode.bindTargetExpressions(SelectNode.java:734) at org.apache.derby.impl.sql.compile.SubqueryNode.bindExpression(SubqueryNode.java:532) so the check may be redundant. But I did adjust it so it catches the following bad in column 2 in the check in verifySelectStarSubquery (foo is bogus): select i from t5501b t1 where not exists (select t2. ,foo. from t5501a t2 where t1.i=t2.i) 2) As for setResultToBooleanTrueNode, i adjusted it so the following would now (also) be rewritten: select true, .. from .. -> select true from .. although not doing so didn't seem to matter. 3) I also adjusted a negative test case in lang/subquery.sql to becoming a positive one. Rerunning regressions.
        Hide
        Dag H. Wanvik added a comment -

        Thanks, Knut. I missed that when scanning the standard on this; I only saw the <table subquery> and assumed it was a Derby specific restriction. In any case, it is as per the standard, and this issue is definitely not a bug then, but an improvement. I'll check the release note needed flag and file a documentation JIRA too.

        Show
        Dag H. Wanvik added a comment - Thanks, Knut. I missed that when scanning the standard on this; I only saw the <table subquery> and assumed it was a Derby specific restriction. In any case, it is as per the standard, and this issue is definitely not a bug then, but an improvement. I'll check the release note needed flag and file a documentation JIRA too.
        Hide
        Knut Anders Hatlen added a comment -

        Derby currently implements SQL Feature E061-08, EXISTS PREDICATE,
        which only allows an <asterisk> or a single <derived column>. There is
        also Feature T501, Enhanced EXISTS predicate, which allows multiple
        columns, but that feature is not supported by Derby yet.

        SQL:2003, part 2, 8.9 <exists predicate>:

        ,----

        Conformance Rules
        1) Without Feature T501, “Enhanced EXISTS predicate”, conforming SQL
        language shall not contain an <exists predicate> that simply contains
        a <table subquery> in which the <select list> of a <query
        specification> directly contained in the <table subquery> does not
        comprise either an <asterisk> or a single <derived column>.
        `----

        So it seems to me that the changes proposed here are allowed by the
        SQL standard.

        The patch may need to make some changes to the
        verifySelectStarSubquery() and setResultToBooleanTrueNode() overrides
        too, as those methods assume that there's only one result column right
        now (they only work on resultColumns.elementAt(0)).

        Show
        Knut Anders Hatlen added a comment - Derby currently implements SQL Feature E061-08, EXISTS PREDICATE, which only allows an <asterisk> or a single <derived column>. There is also Feature T501, Enhanced EXISTS predicate, which allows multiple columns, but that feature is not supported by Derby yet. SQL:2003, part 2, 8.9 <exists predicate>: ,---- Conformance Rules 1) Without Feature T501, “Enhanced EXISTS predicate”, conforming SQL language shall not contain an <exists predicate> that simply contains a <table subquery> in which the <select list> of a <query specification> directly contained in the <table subquery> does not comprise either an <asterisk> or a single <derived column>. `---- So it seems to me that the changes proposed here are allowed by the SQL standard. The patch may need to make some changes to the verifySelectStarSubquery() and setResultToBooleanTrueNode() overrides too, as those methods assume that there's only one result column right now (they only work on resultColumns.elementAt(0)).
        Hide
        Dag H. Wanvik added a comment -

        Uploading a patch which lifts this restriction for EXISTS/NOT EXISTS and adds a test fixture to NestedWhereSubqueryTest. I haven't thought through the ramifications about allowing this, but right now I can't see why lifting this restriction would cause any problems: internally the select list is rewritten to a "SELECT TRUE FROM.." in any case. Interestingly, the case "select *" is already allowed, cf. the tests added.

        Running regressions.

        Show
        Dag H. Wanvik added a comment - Uploading a patch which lifts this restriction for EXISTS/NOT EXISTS and adds a test fixture to NestedWhereSubqueryTest. I haven't thought through the ramifications about allowing this, but right now I can't see why lifting this restriction would cause any problems: internally the select list is rewritten to a "SELECT TRUE FROM.." in any case. Interestingly, the case "select *" is already allowed, cf. the tests added. Running regressions.
        Hide
        Dag H. Wanvik added a comment -

        Attaching a repro (derby-5501-repro) as a diff to NestedWhereSubqueryTest.

        Show
        Dag H. Wanvik added a comment - Attaching a repro (derby-5501-repro) as a diff to NestedWhereSubqueryTest.
        Hide
        Dag H. Wanvik added a comment -

        Thanks for filing this issue. Changing to improvement request.

        Show
        Dag H. Wanvik added a comment - Thanks for filing this issue. Changing to improvement request.

          People

          • Assignee:
            Dag H. Wanvik
            Reporter:
            Sandeep Dixit
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development