Derby
  1. Derby
  2. DERBY-672

Re-enable user defined aggregates

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.10.1.1
    • Component/s: SQL
    • Labels:
      None
    • Issue & fix info:
      Patch Available

      Description

      Nicolas Dufour in an email thread titled "functions and list" started on November 2, 2005 requests the ability to create user defined aggregates.

      This functionality used to be in Cloudscape. It was disabled presumably because it was considered non-standard. However, most of the machinery needed for this feature is still in the code. We should re-enable user defined aggregates after we agree on acceptable syntax.

      1. derby-672-23-aa-improveJavadocForAggregator.merge.diff
        1 kB
        Rick Hillegas
      2. derby-672-22-aa-makeModeAggregateStateSerializable.diff
        0.9 kB
        Rick Hillegas
      3. UserDefinedAggregates.html
        18 kB
        Rick Hillegas
      4. derby-672-21-aa-typeDependencies.diff
        5 kB
        Rick Hillegas
      5. d672-no-exceptions.diff
        3 kB
        Knut Anders Hatlen
      6. derby-672-20-aa-exactBounds.diff
        16 kB
        Rick Hillegas
      7. derby-672-19-aa-precisionChecks.diff
        5 kB
        Rick Hillegas
      8. derby-672-18-aa-udaInJar.diff
        6 kB
        Rick Hillegas
      9. derby-672-17-aa-moreTests.diff
        7 kB
        Rick Hillegas
      10. derby-672-16-aa-forbidInGroupBy.diff
        5 kB
        Rick Hillegas
      11. derby-672-15-aa-setCollation.diff
        2 kB
        Rick Hillegas
      12. derby-672-14-aa-collations.diff
        2 kB
        Rick Hillegas
      13. UserDefinedAggregates.html
        18 kB
        Rick Hillegas
      14. derby-672-13-aa-differentReturnType.diff
        7 kB
        Rick Hillegas
      15. derby-672-12-aa-implicitCasts.diff
        16 kB
        Rick Hillegas
      16. derby-672-11-ab-tests.diff
        42 kB
        Rick Hillegas
      17. derby-672-11-aa-tests.diff
        38 kB
        Rick Hillegas
      18. derby-672-10-af-typeBounds.diff
        26 kB
        Rick Hillegas
      19. derby-672-09-ab-udtAggregates.diff
        11 kB
        Rick Hillegas
      20. derby-672-08-aa-fixJSR169yetAgain.diff
        0.7 kB
        Rick Hillegas
      21. derby-672-07-aa-fixJSR169again.diff
        0.8 kB
        Rick Hillegas
      22. derby-672-06-aa-grantRevoke.diff
        23 kB
        Rick Hillegas
      23. derby-672-05-aa-java7testOrderProblem.diff
        16 kB
        Rick Hillegas
      24. derby-672-04-aa-fixJSR169test.diff
        1 kB
        Rick Hillegas
      25. derby-672-03-ab-distinct.diff
        22 kB
        Rick Hillegas
      26. derby-672-03-aa-distinct.diff
        16 kB
        Rick Hillegas
      27. derby-672-02-ac-nonDistinct.diff
        80 kB
        Rick Hillegas
      28. derby-672-01-aa-ddl.diff
        57 kB
        Rick Hillegas
      29. UserDefinedAggregates.html
        15 kB
        Rick Hillegas
      30. UserDefinedAggregates.html
        12 kB
        Rick Hillegas

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Rick Hillegas added a comment - - edited

          I do not see ANSI syntax for this feature. Here is the syntax used by some other databases.

          ----------------------------------------

          Cloudscape 3.5:

          CREATE AGGREGATE AggregateName FOR

          { JavaClassName | ClassAlias }

          The aliased class has to be an implementation of a Cloudscape interface (COM.cloudscape.aggregates.AggregateDefinition). That, in turn, means that the class has to implement a couple methods:

          accumulate() – accumulator for non-grouped results
          getResult() – the final result of the accumulation
          merge() – accumulator for grouped results

          ---------------------------------------

          Postgres 8.0.0:

          CREATE AGGREGATE name (
          BASETYPE = input_data_type,
          SFUNC = sfunc,
          STYPE = state_data_type
          [ , FINALFUNC = ffunc ]
          [ , INITCOND = initial_condition ]
          )

          Here

          SFUNC is the name of an accumulator function
          FFUNC is the name of a function which returns the accumulated result

          -----------------------------------------------------------

          IBM Informix:

          CREATE AGGREGATE average
          WITH (
          INIT = initFunction,
          ITER = iterationFunction,
          COMBINE = combinationFunction,
          FINAL = resultFunction
          )

          where

          initFunction is a function initializing the accumulator
          iterationFunction a counting function
          combinationFunction an accumulator function
          resultFunction a function which returns the accumulated result

          -----------------------------------------------------------

          Oracle:

          In Oracle, you define a UDT which implements an aggregator interface. Then you bind that UDT to an aggregate name. See http://download.oracle.com/docs/cd/B28359_01/appdev.111/b28425/aggr_functions.htm

          CREATE TYPE MyAggregatorRoutines(
          STATIC FUNCTION ODCIAggregateInitialize( ... ) ...,
          MEMBER FUNCTION ODCIAggregateIterate(...) ... ,
          MEMBER FUNCTION ODCIAggregateMerge(...) ...,
          MEMBER FUNCTION ODCIAggregateTerminate(...)
          );
          CREATE TYPE BODY MyAggregatorRoutines IS
          ...
          END;

          CREATE FUNCTION MyAggregate(x SomeDataType) RETURN SomeDataType
          AGGREGATE USING MyAggregateRoutines;

          Show
          Rick Hillegas added a comment - - edited I do not see ANSI syntax for this feature. Here is the syntax used by some other databases. ---------------------------------------- Cloudscape 3.5: CREATE AGGREGATE AggregateName FOR { JavaClassName | ClassAlias } The aliased class has to be an implementation of a Cloudscape interface (COM.cloudscape.aggregates.AggregateDefinition). That, in turn, means that the class has to implement a couple methods: accumulate() – accumulator for non-grouped results getResult() – the final result of the accumulation merge() – accumulator for grouped results --------------------------------------- Postgres 8.0.0: CREATE AGGREGATE name ( BASETYPE = input_data_type, SFUNC = sfunc, STYPE = state_data_type [ , FINALFUNC = ffunc ] [ , INITCOND = initial_condition ] ) Here SFUNC is the name of an accumulator function FFUNC is the name of a function which returns the accumulated result ----------------------------------------------------------- IBM Informix: CREATE AGGREGATE average WITH ( INIT = initFunction, ITER = iterationFunction, COMBINE = combinationFunction, FINAL = resultFunction ) where initFunction is a function initializing the accumulator iterationFunction a counting function combinationFunction an accumulator function resultFunction a function which returns the accumulated result ----------------------------------------------------------- Oracle: In Oracle, you define a UDT which implements an aggregator interface. Then you bind that UDT to an aggregate name. See http://download.oracle.com/docs/cd/B28359_01/appdev.111/b28425/aggr_functions.htm CREATE TYPE MyAggregatorRoutines( STATIC FUNCTION ODCIAggregateInitialize( ... ) ..., MEMBER FUNCTION ODCIAggregateIterate(...) ... , MEMBER FUNCTION ODCIAggregateMerge(...) ..., MEMBER FUNCTION ODCIAggregateTerminate(...) ); CREATE TYPE BODY MyAggregatorRoutines IS ... END; CREATE FUNCTION MyAggregate(x SomeDataType) RETURN SomeDataType AGGREGATE USING MyAggregateRoutines;
          Hide
          Rick Hillegas added a comment -

          Another use-case for this feature came up on the Derby user list: implementing a MySQL-style GROUP_CONCAT function. See http://old.nabble.com/help-with-sql-query-for-Derby-to31503029.html#a31503029

          Show
          Rick Hillegas added a comment - Another use-case for this feature came up on the Derby user list: implementing a MySQL-style GROUP_CONCAT function. See http://old.nabble.com/help-with-sql-query-for-Derby-to31503029.html#a31503029
          Hide
          Rick Hillegas added a comment -

          The SQL Server syntax for user-defined aggregates can be found here: http://msdn.microsoft.com/en-us/library/ms182741.aspx and http://msdn.microsoft.com/en-us/library/ms131051.aspx In summary, you bind an external class to an aggregate name. The class must satisfy a contract, which requires it to have methods to initialize, aggregate, merge, and terminate:

          CREATE AGGREGATE [<schemaName>.]<aggregateName> ( <functionParameter> [, <functionParameter> ] * )
          RETURNS <returnDataType>
          EXTERNAL NAME <className>

          where <functionParameter> is as defined for CREATE FUNCTION (a parameter name followed by a dataType)

          and <className> implements the following methods:

          public void init();
          public void accumulate( <methodParameter> [, <methodParameter> ] *); // method parameters correspond to function parameters above
          public void merge( <className> otherAggregator );
          public <returnType> terminate(); // returnType corresponds to returnDataType above

          Show
          Rick Hillegas added a comment - The SQL Server syntax for user-defined aggregates can be found here: http://msdn.microsoft.com/en-us/library/ms182741.aspx and http://msdn.microsoft.com/en-us/library/ms131051.aspx In summary, you bind an external class to an aggregate name. The class must satisfy a contract, which requires it to have methods to initialize, aggregate, merge, and terminate: CREATE AGGREGATE [<schemaName>.] <aggregateName> ( <functionParameter> [, <functionParameter> ] * ) RETURNS <returnDataType> EXTERNAL NAME <className> where <functionParameter> is as defined for CREATE FUNCTION (a parameter name followed by a dataType) and <className> implements the following methods: public void init(); public void accumulate( <methodParameter> [, <methodParameter> ] *); // method parameters correspond to function parameters above public void merge( <className> otherAggregator ); public <returnType> terminate(); // returnType corresponds to returnDataType above
          Hide
          Rick Hillegas added a comment -

          The Virtuoso syntax for declaring a user-defined aggregate can be found here: http://en.wikipedia.org/wiki/Virtuoso_Universal_Server

          CREATE AGGREGATE aggregate_name (parameter, parameter ...) [ RETURNS data_type ]
          FROM
          init_procedure,
          acc_procedure,
          final_function
          [, merge_procedure ]

          parameter : parameter_type name data_type

          parameter_type : IN | INOUT

          Show
          Rick Hillegas added a comment - The Virtuoso syntax for declaring a user-defined aggregate can be found here: http://en.wikipedia.org/wiki/Virtuoso_Universal_Server CREATE AGGREGATE aggregate_name (parameter, parameter ...) [ RETURNS data_type ] FROM init_procedure, acc_procedure, final_function [, merge_procedure ] parameter : parameter_type name data_type parameter_type : IN | INOUT
          Hide
          Rick Hillegas added a comment -

          The IBM Informix syntax for declaring a user-defined aggregate can be found here: www.informixoncampus.org/IDS_DOC/SQLSyntax1150.pdf

          CREATE AGGREGATE [<schemaName>.]<aggregateName>
          WITH
          (
          [ INIT = <initFunctionName> ]
          ITER = <accumulatorFunctionName>
          COMBINE = <mergeFunctionName>
          [ FINAL = <resultsFunctionName> ]
          [ HANDLESNULLS ]
          )

          Show
          Rick Hillegas added a comment - The IBM Informix syntax for declaring a user-defined aggregate can be found here: www.informixoncampus.org/IDS_DOC/SQLSyntax1150.pdf CREATE AGGREGATE [<schemaName>.] <aggregateName> WITH ( [ INIT = <initFunctionName> ] ITER = <accumulatorFunctionName> COMBINE = <mergeFunctionName> [ FINAL = <resultsFunctionName> ] [ HANDLESNULLS ] )
          Hide
          Rick Hillegas added a comment -

          Here is a proposal for creating/dropping user defined aggregates in Derby. If this seems reasonable, I will write a functional spec. I would appreciate feedback on:

          1) Whether the syntax is acceptable.

          2) Whether the restriction to Java 5 (and above) is acceptable.

          Thanks,
          -Rick

          --------------------------------

          Because there is no SQL Standard syntax for user defined aggregates, I see only two ways to offer this frequently requested feature:

          i) Introduce new system procedures to create and drop user defined aggregates.

          ii) Introduce Derby-specific syntax.

          Approach might look something like this:

          sys.create_aggregate
          (
          aggregateSchema varchar( 128 ),
          aggregateName varchar( 128 ),
          valueDataTypeSchema varchar( 128 ),
          valueDataTypeName varchar( 128 ),
          returnTypeSchema varchar( 128 ),
          returnTypeName varchar( 128 ),
          aggregateClassName varchar( 32672 )
          )

          sys.drop_aggregate
          (
          aggregateSchema varchar( 128 ),
          aggregateName varchar( 128 )
          )

          Approach (ii) might look something like the following. Note that the extra "derby" keyword flags these statements as Derby extensions and protects us from syntax conflicts in case ANSI/ISO decide to introduce standard syntax in the future:

          create derby aggregate [<schemaName>.]<aggregateName> for <argumentDataType>
          returns <returnDataType>
          external name <className>

          drop derby aggregate [<schemaName>.]<aggregateName>

          The two approaches would look like this to the user:

          call sys.create_aggregate ( 'APP', 'MODE', null, 'int', null, 'int', 'com.mycompany.myapp.aggs.Mode' );

          call sys.drop_aggregate ( 'APP', 'MODE' );

          vs.:

          create derby aggregate MODE for int
          returns int
          external name 'com.mycompany.myapp.aggs.Mode';

          drop derby aggregate MODE;

          Here's how I rate these two approaches:

          :

          + Compact

          • Cryptic
          • Suffers the same identifier casing problems which mar our other system procedures.

          (ii):

          + Readable

          • Verbose

          I think that approach (ii) is more elegant and attractive.

          --------------------------------

          Regardless of how we declare and drop user defined aggregates, they would be invoked just like builtin aggregates. For example:

          select age, mode( salary )
          from employee
          group by age;

          --------------------------------

          User defined aggregates look like parameterized types to me. That means that they would be available on platforms operating at level Java 5 or higher. They would not be available on CDC platforms. To run them on small devices, you would need Java Embedded SE.

          I think that a user defined aggregate is a class which implements the following interface. A little mapping code would be necessary to map between this interface and the Java implementations expected by Postgres, IBM, and Oracle. Regardless of the Java api we require, some (probably trivial) re-coding would be necessary to port an aggregate between Derby and Microsoft's .NET apis.

          Note that the interface extends Serializable. That is because Derby may have to serialize these objects when sorts spill to disk.

          package org.apache.derby.agg;

          import java.io.Serializable;
          import java.sql.SQLException;

          /**

          • <p>
          • Behavior of a user-defined Derby aggregator. Aggregates values
          • of type V and returns a result of type R. In addition to the methods
          • in the interface, implementing classes must have a 0-arg public
          • constructor.
          • </p>
            */
            public interface Aggregator<V,R> extends Serializable { /** Initialize the Aggregator */ public void init() throws SQLException; /** Accumulate the next scalar value */ public void accumulate( V value ) throws SQLException; /** * For merging another partial result into this Aggregator. * This lets the SQL interpreter divide the incoming rows into * subsets, aggregating each subset in isolation, and then merging * the partial results together. */ public void merge( Aggregator<V,R> otherAggregator ) throws SQLException; /** Return the result scalar value */ public R terminate() throws SQLException; }
          Show
          Rick Hillegas added a comment - Here is a proposal for creating/dropping user defined aggregates in Derby. If this seems reasonable, I will write a functional spec. I would appreciate feedback on: 1) Whether the syntax is acceptable. 2) Whether the restriction to Java 5 (and above) is acceptable. Thanks, -Rick -------------------------------- Because there is no SQL Standard syntax for user defined aggregates, I see only two ways to offer this frequently requested feature: i) Introduce new system procedures to create and drop user defined aggregates. ii) Introduce Derby-specific syntax. Approach might look something like this: sys.create_aggregate ( aggregateSchema varchar( 128 ), aggregateName varchar( 128 ), valueDataTypeSchema varchar( 128 ), valueDataTypeName varchar( 128 ), returnTypeSchema varchar( 128 ), returnTypeName varchar( 128 ), aggregateClassName varchar( 32672 ) ) sys.drop_aggregate ( aggregateSchema varchar( 128 ), aggregateName varchar( 128 ) ) Approach (ii) might look something like the following. Note that the extra "derby" keyword flags these statements as Derby extensions and protects us from syntax conflicts in case ANSI/ISO decide to introduce standard syntax in the future: create derby aggregate [<schemaName>.] <aggregateName> for <argumentDataType> returns <returnDataType> external name <className> drop derby aggregate [<schemaName>.] <aggregateName> The two approaches would look like this to the user: call sys.create_aggregate ( 'APP', 'MODE', null, 'int', null, 'int', 'com.mycompany.myapp.aggs.Mode' ); call sys.drop_aggregate ( 'APP', 'MODE' ); vs.: create derby aggregate MODE for int returns int external name 'com.mycompany.myapp.aggs.Mode'; drop derby aggregate MODE; Here's how I rate these two approaches: : + Compact Cryptic Suffers the same identifier casing problems which mar our other system procedures. (ii): + Readable Verbose I think that approach (ii) is more elegant and attractive. -------------------------------- Regardless of how we declare and drop user defined aggregates, they would be invoked just like builtin aggregates. For example: select age, mode( salary ) from employee group by age; -------------------------------- User defined aggregates look like parameterized types to me. That means that they would be available on platforms operating at level Java 5 or higher. They would not be available on CDC platforms. To run them on small devices, you would need Java Embedded SE. I think that a user defined aggregate is a class which implements the following interface. A little mapping code would be necessary to map between this interface and the Java implementations expected by Postgres, IBM, and Oracle. Regardless of the Java api we require, some (probably trivial) re-coding would be necessary to port an aggregate between Derby and Microsoft's .NET apis. Note that the interface extends Serializable. That is because Derby may have to serialize these objects when sorts spill to disk. package org.apache.derby.agg; import java.io.Serializable; import java.sql.SQLException; /** <p> Behavior of a user-defined Derby aggregator. Aggregates values of type V and returns a result of type R. In addition to the methods in the interface, implementing classes must have a 0-arg public constructor. </p> */ public interface Aggregator<V,R> extends Serializable { /** Initialize the Aggregator */ public void init() throws SQLException; /** Accumulate the next scalar value */ public void accumulate( V value ) throws SQLException; /** * For merging another partial result into this Aggregator. * This lets the SQL interpreter divide the incoming rows into * subsets, aggregating each subset in isolation, and then merging * the partial results together. */ public void merge( Aggregator<V,R> otherAggregator ) throws SQLException; /** Return the result scalar value */ public R terminate() throws SQLException; }
          Hide
          Knut Anders Hatlen added a comment -

          The proposal looks good to me. I would be fine with either of the two alternatives, but I agree that alternative (ii) sounds more elegant. It might also be more flexible, as adding optional clauses to the syntax later is probably easier than adding more parameters to existing procedures.

          I have no objections to restricting new features to modern platforms. By using parameterized types, the user doesn't have to cast the parameter in the accumulate() method, and it is checked at compile time that the value returned from terminate() has the correct type.

          It does however still seem to me that the user will have to cast the otherAggregator parameter that's passed to the merge() method, as the implementation in most cases will need to access fields/method of the instance that's passed in. Example (reimplementing the SUM aggregate function for integers):

          public class MyIntSum implements Aggregator<Integer,Integer> {
          private int sum;
          public void init()

          { sum=0; }
          public void accumulate(Integer value) { sum += value; }
          public void merge(Aggregator<Integer,Integer> otherAggregator) { MyIntSum that = (MyIntSum) otherAggregator; this.sum += that.sum; }
          public Integer terminate() { return sum; }
          }

          Not sure what's the cleanest way to handle this. Maybe we could add a third parameterized type that tells which kind of aggregators it can be merged with. That would mean changing the interface declaration like this:

          public interface Aggregator<V, R, T extends Aggregator<V,R,T>> extends Serializable { ... public void merge( T otherAggregator ) throws SQLException; ... }

          and MyIntSum would become:

          public class MyIntSum implements Aggregator<Integer,Integer,MyIntSum> {
          private int sum;
          public void init() { sum=0; }

          public void accumulate(Integer value)

          { sum += value; }

          public void merge(MyIntSum that)

          { this.sum += that.sum; }

          public Integer terminate()

          { return sum; }

          }

          It makes the declaration a little more messy, but the implementation becomes more concise and statically type-checked.

          Show
          Knut Anders Hatlen added a comment - The proposal looks good to me. I would be fine with either of the two alternatives, but I agree that alternative (ii) sounds more elegant. It might also be more flexible, as adding optional clauses to the syntax later is probably easier than adding more parameters to existing procedures. I have no objections to restricting new features to modern platforms. By using parameterized types, the user doesn't have to cast the parameter in the accumulate() method, and it is checked at compile time that the value returned from terminate() has the correct type. It does however still seem to me that the user will have to cast the otherAggregator parameter that's passed to the merge() method, as the implementation in most cases will need to access fields/method of the instance that's passed in. Example (reimplementing the SUM aggregate function for integers): public class MyIntSum implements Aggregator<Integer,Integer> { private int sum; public void init() { sum=0; } public void accumulate(Integer value) { sum += value; } public void merge(Aggregator<Integer,Integer> otherAggregator) { MyIntSum that = (MyIntSum) otherAggregator; this.sum += that.sum; } public Integer terminate() { return sum; } } Not sure what's the cleanest way to handle this. Maybe we could add a third parameterized type that tells which kind of aggregators it can be merged with. That would mean changing the interface declaration like this: public interface Aggregator<V, R, T extends Aggregator<V,R,T>> extends Serializable { ... public void merge( T otherAggregator ) throws SQLException; ... } and MyIntSum would become: public class MyIntSum implements Aggregator<Integer,Integer,MyIntSum> { private int sum; public void init() { sum=0; } public void accumulate(Integer value) { sum += value; } public void merge(MyIntSum that) { this.sum += that.sum; } public Integer terminate() { return sum; } } It makes the declaration a little more messy, but the implementation becomes more concise and statically type-checked.
          Hide
          Knut Anders Hatlen added a comment -

          The proposal doesn't say whether the class name argument itself could be a parameterized type.

          That is, could we have

          public class MySum<T extends Number> implements Aggragator<T,T>

          { ... }

          and then define aggregates for two different types using that single class

          create derby aggregate int_sum for int returns int external name 'MySum<Integer>';
          create derby aggregate bigint_sum for bigint returns bigint external name 'MySum<Long>';

          ?

          Or would we have to define two separate sub-classes with fully specified types, like

          public class MyIntSum extends MySum<Integer> {}
          public class MyBigintSum extends MySum<Long> {}

          create derby aggregate int_sum for int returns int external name 'MyIntSum';
          create derby aggregate bigint_sum for bigint returns bigint external name 'MyBigintSum';

          ?

          Show
          Knut Anders Hatlen added a comment - The proposal doesn't say whether the class name argument itself could be a parameterized type. That is, could we have public class MySum<T extends Number> implements Aggragator<T,T> { ... } and then define aggregates for two different types using that single class create derby aggregate int_sum for int returns int external name 'MySum<Integer>'; create derby aggregate bigint_sum for bigint returns bigint external name 'MySum<Long>'; ? Or would we have to define two separate sub-classes with fully specified types, like public class MyIntSum extends MySum<Integer> {} public class MyBigintSum extends MySum<Long> {} create derby aggregate int_sum for int returns int external name 'MyIntSum'; create derby aggregate bigint_sum for bigint returns bigint external name 'MyBigintSum'; ?
          Hide
          Rick Hillegas added a comment -

          Thanks for continuing the discussion, Knut. I like your suggestion for how to eliminate the need to cast inside the merge() method. I agree that it makes the Aggregator interface iself a little more wordy, but I don't have a better idea.

          I hope that users will be able to re-use an Aggregator for multiple datatypes as shown in your first example:

          create derby aggregate int_sum for int returns int external name 'MySum<Integer>';
          create derby aggregate bigint_sum for bigint returns bigint external name 'MySum<Long>';

          I suppose it depends on whether Derby can currently swallow a genericized class name and, if not, how hard it would be to teach Derby this trick. We may need to get further into implementation before we can answer your question. Thanks.

          Show
          Rick Hillegas added a comment - Thanks for continuing the discussion, Knut. I like your suggestion for how to eliminate the need to cast inside the merge() method. I agree that it makes the Aggregator interface iself a little more wordy, but I don't have a better idea. I hope that users will be able to re-use an Aggregator for multiple datatypes as shown in your first example: create derby aggregate int_sum for int returns int external name 'MySum<Integer>'; create derby aggregate bigint_sum for bigint returns bigint external name 'MySum<Long>'; I suppose it depends on whether Derby can currently swallow a genericized class name and, if not, how hard it would be to teach Derby this trick. We may need to get further into implementation before we can answer your question. Thanks.
          Hide
          Rick Hillegas added a comment -

          Attaching UserDefinedAggregates.html, the first rev of a functional spec for this feature.

          Show
          Rick Hillegas added a comment - Attaching UserDefinedAggregates.html, the first rev of a functional spec for this feature.
          Hide
          Kathey Marsden added a comment -

          I am sorry I haven't been following recent discussions closely. I think stored procedures are better than non-standard SQL even if less elegant.

          Show
          Kathey Marsden added a comment - I am sorry I haven't been following recent discussions closely. I think stored procedures are better than non-standard SQL even if less elegant.
          Hide
          Mamta A. Satoor added a comment -

          I agree with Kathey, I would prefer us adding a new stored procedure rather than adding non-SQL compliant SQL. We have used procedures for other non-standard features like update/drop statistics and it will be good to follow the same path for any non-compliant work.

          Show
          Mamta A. Satoor added a comment - I agree with Kathey, I would prefer us adding a new stored procedure rather than adding non-SQL compliant SQL. We have used procedures for other non-standard features like update/drop statistics and it will be good to follow the same path for any non-compliant work.
          Hide
          Kathey Marsden added a comment -

          I am curious. Is there any ongoing or proposed SQL standard work regarding user defined aggregates?

          Show
          Kathey Marsden added a comment - I am curious. Is there any ongoing or proposed SQL standard work regarding user defined aggregates?
          Hide
          Rick Hillegas added a comment -

          Thanks for continuing the discussion, Kathey and Mamta. In writing the first draft of the functional spec, I realized that we would also need GRANT/REVOKE syntax for user defined aggregates (UDAs). That further tipped me toward preferring using SQL for declaring/dropping these objects. I thought it would look odd for some UDA-related DDL operations to happen via SQL and others via stored procedures. In addition, I decided that using procedures would be a break with current Derby practice: today we use SQL to create and drop all other kinds of schema objects. No other kind of schema object is created/dropped via procedures.

          I want to address Mamta's comment about SQL compliance. The word "compliance" does not appear in the Standard. Probably what is intended here is "SQL conformance". Every DBMS vendor supports vendor-specific extensions in their SQL dialects. These extensions don't make their implementations non-conforming. Non-conformance is caused by failing to implement required features or by mis-implementing features which are defined by the Standard. SQL conformance is defined by part 1 chapter 8, part 2 chapter 25, and (for a Java database like Derby) part 13 chapter 16. In particular, part 1 section 8.4 says:

          "An SQL-implementation may provide implementation-defined features that are additional to those specified by ISO/IEC 9075, and may add to the list of reserved words."

          Vendor-specific extensions must be carefully considered, however. We want to avoid the following problems:

          1) Creating the impression that the extension is Standard and therefore portable.

          2) Increasing the porting burden for applications which need to run on multiple DBMSes.

          3) Creating syntax which may become non-conforming if the Standard defines conflicting syntax later on.

          The proposed syntax does not suffer from these problems, for the following reasons:

          1') The use of the DERBY keyword clearly flags this syntax as a Derby extension.

          2') Many DBMSes support UDAs but the Standard has not defined an api in this area. Using UDAs necessarily causes a porting issue. Simply implementing this JIRA will hopefully reduce the porting burden because applications will not have to rewrite their DML in order to workaround Derby's lack of support for UDAs. I do not see how the porting burden is significantly affected by whether Derby uses stored procedures vs. SQL.

          3') The use of the DERBY keyword insulates us from future changes to the SQL spec, in case the Standard provides official syntax later on. There is no way that our syntax would conflict with the Standard and become non-conforming.

          Also note that if the Standard did supply official syntax later on, we would want to consider implementing it, hooking it up to the machinery which this JIRA will enable. That is true regardless of whether Derby uses stored procedures or SQL.

          However, I am not aware of any interest by the SQL committee in trying to harmonize the divergent extensions in this area.

          Hope this clarifies my reasoning.

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Thanks for continuing the discussion, Kathey and Mamta. In writing the first draft of the functional spec, I realized that we would also need GRANT/REVOKE syntax for user defined aggregates (UDAs). That further tipped me toward preferring using SQL for declaring/dropping these objects. I thought it would look odd for some UDA-related DDL operations to happen via SQL and others via stored procedures. In addition, I decided that using procedures would be a break with current Derby practice: today we use SQL to create and drop all other kinds of schema objects. No other kind of schema object is created/dropped via procedures. I want to address Mamta's comment about SQL compliance. The word "compliance" does not appear in the Standard. Probably what is intended here is "SQL conformance". Every DBMS vendor supports vendor-specific extensions in their SQL dialects. These extensions don't make their implementations non-conforming. Non-conformance is caused by failing to implement required features or by mis-implementing features which are defined by the Standard. SQL conformance is defined by part 1 chapter 8, part 2 chapter 25, and (for a Java database like Derby) part 13 chapter 16. In particular, part 1 section 8.4 says: "An SQL-implementation may provide implementation-defined features that are additional to those specified by ISO/IEC 9075, and may add to the list of reserved words." Vendor-specific extensions must be carefully considered, however. We want to avoid the following problems: 1) Creating the impression that the extension is Standard and therefore portable. 2) Increasing the porting burden for applications which need to run on multiple DBMSes. 3) Creating syntax which may become non-conforming if the Standard defines conflicting syntax later on. The proposed syntax does not suffer from these problems, for the following reasons: 1') The use of the DERBY keyword clearly flags this syntax as a Derby extension. 2') Many DBMSes support UDAs but the Standard has not defined an api in this area. Using UDAs necessarily causes a porting issue. Simply implementing this JIRA will hopefully reduce the porting burden because applications will not have to rewrite their DML in order to workaround Derby's lack of support for UDAs. I do not see how the porting burden is significantly affected by whether Derby uses stored procedures vs. SQL. 3') The use of the DERBY keyword insulates us from future changes to the SQL spec, in case the Standard provides official syntax later on. There is no way that our syntax would conflict with the Standard and become non-conforming. Also note that if the Standard did supply official syntax later on, we would want to consider implementing it, hooking it up to the machinery which this JIRA will enable. That is true regardless of whether Derby uses stored procedures or SQL. However, I am not aware of any interest by the SQL committee in trying to harmonize the divergent extensions in this area. Hope this clarifies my reasoning. Thanks, -Rick
          Hide
          Rick Hillegas added a comment -

          Attaching 2nd rev of a functional spec for this issue. The chief change here is to clarify that user-defined aggregates live in the same namespace as 1-arg user-defined functions and to avoid widening the namespace issue identified by DERBY-5901.

          Show
          Rick Hillegas added a comment - Attaching 2nd rev of a functional spec for this issue. The chief change here is to clarify that user-defined aggregates live in the same namespace as 1-arg user-defined functions and to avoid widening the namespace issue identified by DERBY-5901 .
          Hide
          Rick Hillegas added a comment -

          Linking to DERBY-5901. Solutions to that issue may take advantage of work done on user-defined aggregates.

          Show
          Rick Hillegas added a comment - Linking to DERBY-5901 . Solutions to that issue may take advantage of work done on user-defined aggregates.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-01-aa-ddl.diff. This patch adds CREATE and DROP ddl for user-defined aggregates, including dblook support and upgrade tests. I am running full regression tests now.

          This patch adds support for the following two DDL statements:

          CREATE DERBY AGGREGATE [ schemaName. ] SQL92Identifier
          FOR ValueDataType
          [ RETURNS ReturnDataType ]
          EXTERNAL NAME ClassNameString

          and

          DROP DERBY AGGREGATE [ schemaName. ] SQL92Identifier RESTRICT

          I have gone to some trouble to prevent name collisions between user-defined aggregates and 1-arg functions. That work may be useful when we address DERBY-5901.

          Touches the following files:

          --------------

          M java/engine/org/apache/derby/iapi/sql/depend/DependencyManager.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/AliasDescriptor.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/PermDescriptor.java
          M java/engine/org/apache/derby/iapi/services/io/RegisteredFormatIds.java
          M java/engine/org/apache/derby/iapi/services/io/StoredFormatIds.java
          M java/engine/org/apache/derby/catalog/AliasInfo.java
          A java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java
          M java/engine/org/apache/derby/impl/sql/catalog/SYSALIASESRowFactory.java

          A user-defined aggregate is implemented as a kind of Java alias, like SQL routines and UDTs.

          --------------

          M java/engine/org/apache/derby/impl/sql/compile/DropAliasNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java
          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
          M java/engine/org/apache/derby/impl/sql/execute/CreateAliasConstantAction.java

          Actual DDL support. Introduces 2 new non-reserved keywords: DERBY and AGGREGATE.

          --------------

          M java/tools/org/apache/derby/loc/toolsmessages.properties
          M java/engine/org/apache/derby/loc/messages.xml
          M java/shared/org/apache/derby/shared/common/reference/SQLState.java

          New error messages introduced by the new DDL.

          --------------

          M java/tools/org/apache/derby/tools/dblook.java
          M java/tools/org/apache/derby/impl/tools/dblook/DB_Alias.java

          Tool support for re-creating aggregate DDL when dumping a database.

          --------------

          M java/testing/org/apache/derbyTesting/junit/JDBC.java

          Support for dropping aggregates when scouring out schemas inbetween regression test cases.

          --------------

          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/Changes10_10.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/tools/dblook_makeDB_2.sql
          M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test_territory.out
          M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test.out

          New regression tests for basic aggregate DDL, dblook, and upgrade.

          Show
          Rick Hillegas added a comment - Attaching derby-672-01-aa-ddl.diff. This patch adds CREATE and DROP ddl for user-defined aggregates, including dblook support and upgrade tests. I am running full regression tests now. This patch adds support for the following two DDL statements: CREATE DERBY AGGREGATE [ schemaName. ] SQL92Identifier FOR ValueDataType [ RETURNS ReturnDataType ] EXTERNAL NAME ClassNameString and DROP DERBY AGGREGATE [ schemaName. ] SQL92Identifier RESTRICT I have gone to some trouble to prevent name collisions between user-defined aggregates and 1-arg functions. That work may be useful when we address DERBY-5901 . Touches the following files: -------------- M java/engine/org/apache/derby/iapi/sql/depend/DependencyManager.java M java/engine/org/apache/derby/iapi/sql/dictionary/AliasDescriptor.java M java/engine/org/apache/derby/iapi/sql/dictionary/PermDescriptor.java M java/engine/org/apache/derby/iapi/services/io/RegisteredFormatIds.java M java/engine/org/apache/derby/iapi/services/io/StoredFormatIds.java M java/engine/org/apache/derby/catalog/AliasInfo.java A java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java M java/engine/org/apache/derby/impl/sql/catalog/SYSALIASESRowFactory.java A user-defined aggregate is implemented as a kind of Java alias, like SQL routines and UDTs. -------------- M java/engine/org/apache/derby/impl/sql/compile/DropAliasNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj M java/engine/org/apache/derby/impl/sql/execute/CreateAliasConstantAction.java Actual DDL support. Introduces 2 new non-reserved keywords: DERBY and AGGREGATE. -------------- M java/tools/org/apache/derby/loc/toolsmessages.properties M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java New error messages introduced by the new DDL. -------------- M java/tools/org/apache/derby/tools/dblook.java M java/tools/org/apache/derby/impl/tools/dblook/DB_Alias.java Tool support for re-creating aggregate DDL when dumping a database. -------------- M java/testing/org/apache/derbyTesting/junit/JDBC.java Support for dropping aggregates when scouring out schemas inbetween regression test cases. -------------- A java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java M java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/Changes10_10.java M java/testing/org/apache/derbyTesting/functionTests/tests/tools/dblook_makeDB_2.sql M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test_territory.out M java/testing/org/apache/derbyTesting/functionTests/master/dblook_test.out New regression tests for basic aggregate DDL, dblook, and upgrade.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-01-aa-ddl.diff.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-01-aa-ddl.diff.
          Hide
          Rick Hillegas added a comment -

          Committed derby-672-01-aa-ddl.diff at subversion revision 1374399.

          Show
          Rick Hillegas added a comment - Committed derby-672-01-aa-ddl.diff at subversion revision 1374399.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-02-ac-nonDistinct.diff. This patch adds bind() and execute() support for non-distinct user-defined aggregates. I am running regression tests now.

          With this patch you can invoke user-defined aggregates in simple queries. These include both scalar and grouped results. So, for instance, the following script now runs correctly:

          create derby aggregate mode for int external name 'org.apache.derbyTesting.functionTests.tests.lang.ModeAggregate';
          create table mode_inputs( a int, b int );
          insert into mode_inputs( a, b ) values ( 1, 1 ), ( 1, 2 ), ( 1, 2 ), ( 1, 2 ), ( 2, 3 ), ( 2, 3 ), ( 2, 4 );
          select mode( b ) from mode_inputs;
          select a, mode( b ) from mode_inputs group by a;

          Support is NOT included for DISTINCT invocations of user-defined aggregates nor for user-defined aggregates in HAVING clauses.

          On 2012-07-18 Knut wondered whether we would support parameterized aggregates, e.g., with elegant syntax like the following:

          create derby aggregate int_mode for int external name 'GenericMode<Integer>';

          Unfortunately, I have not been able to make this work. That is because type erasure makes it impossible to instantiate the aggregate via Class.forName(). The following sample code demonstrates the problem:

          public class z
          {
          public static void main( String[] args ) throws Exception

          { // raises a ClassNotFoundException Class.forName( "java.util.ArrayList<String>" ); }

          }

          Your thoughts about how to work around this limitation are welcome. In the meantime, the following less elegant syntax will work:

          public class GenericMode<V extends Comparable<V>> implements Aggregator<V,V,GenericMode<V>>
          {
          public static final class IntMode extends GenericMode<Integer> {}

          ...
          }

          create derby aggregate intMode for int external name 'GenericMode$IntMode';

          Touches the following files:

          -----------------

          A java/engine/org/apache/derby/agg
          A java/engine/org/apache/derby/agg/Aggregator.java
          A java/engine/org/apache/derby/agg/build.xml
          M tools/javadoc/publishedapi.ant
          M java/engine/build.xml

          Adds this interface to the public api. This is the interface which users implement in order to create a user-defined aggregate.

          -----------------

          M java/engine/org/apache/derby/impl/services/reflect/DatabaseClasses.java
          A java/engine/org/apache/derby/impl/services/reflect/Java5ClassFactory.java
          M java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java
          M java/engine/org/apache/derby/impl/services/build.xml
          M java/engine/org/apache/derby/modules.properties
          M java/engine/org/apache/derby/iapi/services/build.xml
          A java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java
          M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java

          Adds a new implementation of the ClassFactory module, for use on JVMs which support generics. Adds the following new method to ClassInspector:

          /**

          • Given an implementation of a parameterized class/interface, return
          • the actual concrete types of the parameters. Types are returned in the
          • order that they are declared by the parameterized class/interface.
          • So for instance, if the parameterized class is Map<K,V> and the
          • implementation is HashMap<Integer,String>, then the return value is
          • [ Integer.class, String.class ]. This method raises an exception if the
          • JVM does not support generics. May return null if type resolution fails.
            */
            public Class[] getGenericParameterTypes( Class parameterizedType, Class implementation )
            throws StandardException;

          On CDC/FP this method raises an exception. On Java 5 and higher, this resolves the types of the generic variables. Fortunately, this type resolution is possible via the reflection classes. Unfortunately, getting at this information is far from straightforward.

          -----------------

          M java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java

          Added some accessors to this metadata descriptor.

          -----------------

          M java/engine/org/apache/derby/impl/sql/compile/MethodCallNode.java
          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/JavaToSQLValueNode.java
          A java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java
          M java/engine/org/apache/derby/impl/sql/compile/JavaValueNode.java
          M java/engine/org/apache/derby/impl/sql/compile/AggregateDefinition.java
          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java

          Bind-time logic for handling non-distinct user-defined aggregates in the SELECT list. These are the highlights:

          1) JavaToSQLValueNode.bindExpression() and StaticMethodCallNode.bindExpression() cooperate to check whether the invocation of a single-arg function is actually the invocation of a user-defined aggregate.

          2) If so, StaticMethodCallNode.bindExpression() returns an AggregateNode wrapping a UserAggregateDefinition.

          3) UserAggregateDefinition calls ClassInspector.getGenericParameterTypes() in order to resolve the user-defined aggregate's types. It is expected that the actual types will correspond to the declared types under the same rules which map SQL routines args to Java method args.

          -----------------

          M java/engine/org/apache/derby/iapi/sql/execute/ExecAggregator.java
          M java/engine/org/apache/derby/iapi/services/io/RegisteredFormatIds.java
          M java/engine/org/apache/derby/iapi/reference/ClassName.java
          M java/engine/org/apache/derby/impl/sql/execute/GenericAggregator.java
          M java/engine/org/apache/derby/impl/sql/execute/CountAggregator.java
          M java/engine/org/apache/derby/impl/sql/execute/MaxMinAggregator.java
          A java/engine/org/apache/derby/impl/sql/execute/UserDefinedAggregator.java
          M java/engine/org/apache/derby/impl/sql/execute/OrderableAggregator.java
          M tools/jar/extraDBMSclasses.properties

          Execution logic for invoking user-defined aggregates. Some additional parameters were added to the ExecAggregator.setup() method. The guts of the execution logic are in UserDefinedAggregator, a new implementation of ExecAggregator.

          -----------------

          M java/engine/org/apache/derby/loc/messages.xml
          M java/shared/org/apache/derby/shared/common/reference/SQLState.java
          M java/shared/org/apache/derby/shared/common/reference/MessageId.java

          New error messages.

          -----------------

          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java

          Tests for this functionality. Includes tests for scalar and grouped user-defined aggregates. Includes tests for user-defined aggregates which extend generic implementations. These tests do not run on small devices.

          -----------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java

          Added a test case for CDC/FP, verifying that we raise a reasonable error if someone tries to execute a user-defined aggregate on a small device.

          Show
          Rick Hillegas added a comment - Attaching derby-672-02-ac-nonDistinct.diff. This patch adds bind() and execute() support for non-distinct user-defined aggregates. I am running regression tests now. With this patch you can invoke user-defined aggregates in simple queries. These include both scalar and grouped results. So, for instance, the following script now runs correctly: create derby aggregate mode for int external name 'org.apache.derbyTesting.functionTests.tests.lang.ModeAggregate'; create table mode_inputs( a int, b int ); insert into mode_inputs( a, b ) values ( 1, 1 ), ( 1, 2 ), ( 1, 2 ), ( 1, 2 ), ( 2, 3 ), ( 2, 3 ), ( 2, 4 ); select mode( b ) from mode_inputs; select a, mode( b ) from mode_inputs group by a; Support is NOT included for DISTINCT invocations of user-defined aggregates nor for user-defined aggregates in HAVING clauses. On 2012-07-18 Knut wondered whether we would support parameterized aggregates, e.g., with elegant syntax like the following: create derby aggregate int_mode for int external name 'GenericMode<Integer>'; Unfortunately, I have not been able to make this work. That is because type erasure makes it impossible to instantiate the aggregate via Class.forName(). The following sample code demonstrates the problem: public class z { public static void main( String[] args ) throws Exception { // raises a ClassNotFoundException Class.forName( "java.util.ArrayList<String>" ); } } Your thoughts about how to work around this limitation are welcome. In the meantime, the following less elegant syntax will work: public class GenericMode<V extends Comparable<V>> implements Aggregator<V,V,GenericMode<V>> { public static final class IntMode extends GenericMode<Integer> {} ... } create derby aggregate intMode for int external name 'GenericMode$IntMode'; Touches the following files: ----------------- A java/engine/org/apache/derby/agg A java/engine/org/apache/derby/agg/Aggregator.java A java/engine/org/apache/derby/agg/build.xml M tools/javadoc/publishedapi.ant M java/engine/build.xml Adds this interface to the public api. This is the interface which users implement in order to create a user-defined aggregate. ----------------- M java/engine/org/apache/derby/impl/services/reflect/DatabaseClasses.java A java/engine/org/apache/derby/impl/services/reflect/Java5ClassFactory.java M java/engine/org/apache/derby/impl/services/reflect/ReflectClassesJava2.java M java/engine/org/apache/derby/impl/services/build.xml M java/engine/org/apache/derby/modules.properties M java/engine/org/apache/derby/iapi/services/build.xml A java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java Adds a new implementation of the ClassFactory module, for use on JVMs which support generics. Adds the following new method to ClassInspector: /** Given an implementation of a parameterized class/interface, return the actual concrete types of the parameters. Types are returned in the order that they are declared by the parameterized class/interface. So for instance, if the parameterized class is Map<K,V> and the implementation is HashMap<Integer,String>, then the return value is [ Integer.class, String.class ]. This method raises an exception if the JVM does not support generics. May return null if type resolution fails. */ public Class[] getGenericParameterTypes( Class parameterizedType, Class implementation ) throws StandardException; On CDC/FP this method raises an exception. On Java 5 and higher, this resolves the types of the generic variables. Fortunately, this type resolution is possible via the reflection classes. Unfortunately, getting at this information is far from straightforward. ----------------- M java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java Added some accessors to this metadata descriptor. ----------------- M java/engine/org/apache/derby/impl/sql/compile/MethodCallNode.java M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java M java/engine/org/apache/derby/impl/sql/compile/JavaToSQLValueNode.java A java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java M java/engine/org/apache/derby/impl/sql/compile/JavaValueNode.java M java/engine/org/apache/derby/impl/sql/compile/AggregateDefinition.java M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java Bind-time logic for handling non-distinct user-defined aggregates in the SELECT list. These are the highlights: 1) JavaToSQLValueNode.bindExpression() and StaticMethodCallNode.bindExpression() cooperate to check whether the invocation of a single-arg function is actually the invocation of a user-defined aggregate. 2) If so, StaticMethodCallNode.bindExpression() returns an AggregateNode wrapping a UserAggregateDefinition. 3) UserAggregateDefinition calls ClassInspector.getGenericParameterTypes() in order to resolve the user-defined aggregate's types. It is expected that the actual types will correspond to the declared types under the same rules which map SQL routines args to Java method args. ----------------- M java/engine/org/apache/derby/iapi/sql/execute/ExecAggregator.java M java/engine/org/apache/derby/iapi/services/io/RegisteredFormatIds.java M java/engine/org/apache/derby/iapi/reference/ClassName.java M java/engine/org/apache/derby/impl/sql/execute/GenericAggregator.java M java/engine/org/apache/derby/impl/sql/execute/CountAggregator.java M java/engine/org/apache/derby/impl/sql/execute/MaxMinAggregator.java A java/engine/org/apache/derby/impl/sql/execute/UserDefinedAggregator.java M java/engine/org/apache/derby/impl/sql/execute/OrderableAggregator.java M tools/jar/extraDBMSclasses.properties Execution logic for invoking user-defined aggregates. Some additional parameters were added to the ExecAggregator.setup() method. The guts of the execution logic are in UserDefinedAggregator, a new implementation of ExecAggregator. ----------------- M java/engine/org/apache/derby/loc/messages.xml M java/shared/org/apache/derby/shared/common/reference/SQLState.java M java/shared/org/apache/derby/shared/common/reference/MessageId.java New error messages. ----------------- A java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java A java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java Tests for this functionality. Includes tests for scalar and grouped user-defined aggregates. Includes tests for user-defined aggregates which extend generic implementations. These tests do not run on small devices. ----------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java Added a test case for CDC/FP, verifying that we raise a reasonable error if someone tries to execute a user-defined aggregate on a small device.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-02-ac-nonDistinct.diff. Committed at subversion revision 1378639.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-02-ac-nonDistinct.diff. Committed at subversion revision 1378639.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-03-aa-distinct.diff. This patch adds support for invoking DISTINCT user-defined aggregates in the SELECT list. I am running regression tests now.

          Touches the following files:

          ---------------

          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj

          New parser production for DISTINCT user-defined aggregate invocations. When looking for function invocations, we also look for DISTINCT user-defined aggregate invocations. If we find one, we return an AggregateNode rather than a StaticMethodCallNode.

          ---------------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java

          Bind-time support for AggregateNodes which were created by the new parser production.

          ---------------

          M java/engine/org/apache/derby/impl/sql/execute/UserDefinedAggregator.java

          Fixes a casting error in the runtime logic for user-defined aggregates. The cast occurs in the merge() processing, which is tickled by DISTINCT aggregates.

          ---------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          New test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-672-03-aa-distinct.diff. This patch adds support for invoking DISTINCT user-defined aggregates in the SELECT list. I am running regression tests now. Touches the following files: --------------- M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj New parser production for DISTINCT user-defined aggregate invocations. When looking for function invocations, we also look for DISTINCT user-defined aggregate invocations. If we find one, we return an AggregateNode rather than a StaticMethodCallNode. --------------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java Bind-time support for AggregateNodes which were created by the new parser production. --------------- M java/engine/org/apache/derby/impl/sql/execute/UserDefinedAggregator.java Fixes a casting error in the runtime logic for user-defined aggregates. The cast occurs in the merge() processing, which is tickled by DISTINCT aggregates. --------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java New test cases.
          Hide
          Knut Anders Hatlen added a comment -

          The last commit seems to have caused a test failure on Java ME: http://dbtg.foundry.sun.com/derby/test/Daily/javaME/testing/Limited/testSummary-1378997.html

          1) testUDAWithoutGenerics(org.apache.derbyTesting.functionTests.tests.lang.UngroupedAggregatesNegativeTest)junit.framework.ComparisonFailure: Unexpected SQL state. expected:<X[BCM5]> but was:<X[J001]>
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:853)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:882)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:896)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementErrorMinion(BaseJDBCTestCase.java:1106)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java:1052)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java:1135)
          at org.apache.derbyTesting.functionTests.tests.lang.UngroupedAggregatesNegativeTest.testUDAWithoutGenerics(UngroupedAggregatesNegativeTest.java:121)
          (...)
          Caused by: java.lang.ClassNotFoundException: org.apache.derbyTesting.functionTests.tests.lang.ModeAggregate : org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate (Unsupported major.minor version 49.0)
          (...)

          Show
          Knut Anders Hatlen added a comment - The last commit seems to have caused a test failure on Java ME: http://dbtg.foundry.sun.com/derby/test/Daily/javaME/testing/Limited/testSummary-1378997.html 1) testUDAWithoutGenerics(org.apache.derbyTesting.functionTests.tests.lang.UngroupedAggregatesNegativeTest)junit.framework.ComparisonFailure: Unexpected SQL state. expected:<X [BCM5] > but was:<X [J001] > at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:853) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:882) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState(BaseJDBCTestCase.java:896) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementErrorMinion(BaseJDBCTestCase.java:1106) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java:1052) at org.apache.derbyTesting.junit.BaseJDBCTestCase.assertStatementError(BaseJDBCTestCase.java:1135) at org.apache.derbyTesting.functionTests.tests.lang.UngroupedAggregatesNegativeTest.testUDAWithoutGenerics(UngroupedAggregatesNegativeTest.java:121) (...) Caused by: java.lang.ClassNotFoundException: org.apache.derbyTesting.functionTests.tests.lang.ModeAggregate : org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate (Unsupported major.minor version 49.0) (...)
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-03-aa-distinct.diff. Attaching a second rev of the patch, which includes more test cases: derby-672-03-ab-distinct.diff. Committed at revision 1379505.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-03-aa-distinct.diff. Attaching a second rev of the patch, which includes more test cases: derby-672-03-ab-distinct.diff. Committed at revision 1379505.
          Hide
          Rick Hillegas added a comment -

          Thanks for spotting that test failure, Knut. Attaching derby-672-04-aa-fixJSR169test.diff, which hopefully fixes the test. The patch changes the test to treat both XBCM5 and XJ001 as valid errors. Committed at subversion revision 1379519.

          I don't understand what's different between my Java ME environment and that of the test. With and without the patch, the test runs cleanly for me on both OJEC and phoneME.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java

          Show
          Rick Hillegas added a comment - Thanks for spotting that test failure, Knut. Attaching derby-672-04-aa-fixJSR169test.diff, which hopefully fixes the test. The patch changes the test to treat both XBCM5 and XJ001 as valid errors. Committed at subversion revision 1379519. I don't understand what's different between my Java ME environment and that of the test. With and without the patch, the test runs cleanly for me on both OJEC and phoneME. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java
          Hide
          Rick Hillegas added a comment - - edited

          I see the following errors on the test run on Java 7 on Sol32. See http://dbtg.foundry.sun.com/derby/test/Daily/jvm1.7/testing/Limited/testSummary-1378997.html

          1) test_01_basicSyntax(org.apache.derbyTesting.functionTests.tests.lang.UserDefinedAggregatesTest)java.sql.SQLException: DERBY AGGREGATE 'MODE' already exists.
          2) test_01_basicSyntax(org.apache.derbyTesting.functionTests.tests.lang.UserDefinedAggregatesTest)java.sql.SQLTransactionRollbackException: DERBY AGGREGATE 'MODE' already exists.

          Probably caused by running the test cases in a different order on that platform.

          Show
          Rick Hillegas added a comment - - edited I see the following errors on the test run on Java 7 on Sol32. See http://dbtg.foundry.sun.com/derby/test/Daily/jvm1.7/testing/Limited/testSummary-1378997.html 1) test_01_basicSyntax(org.apache.derbyTesting.functionTests.tests.lang.UserDefinedAggregatesTest)java.sql.SQLException: DERBY AGGREGATE 'MODE' already exists. 2) test_01_basicSyntax(org.apache.derbyTesting.functionTests.tests.lang.UserDefinedAggregatesTest)java.sql.SQLTransactionRollbackException: DERBY AGGREGATE 'MODE' already exists. Probably caused by running the test cases in a different order on that platform.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-05-aa-java7testOrderProblem.diff. Hopefully this fixes the test order problem on Java 7 on Sol32. Committed at subversion revision 1379527.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-05-aa-java7testOrderProblem.diff. Hopefully this fixes the test order problem on Java 7 on Sol32. Committed at subversion revision 1379527. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-06-aa-grantRevoke.diff. This patch adds GRANT/REVOKE USAGE support for user-defined aggregates. Regression tests passed cleanly for me. Committed at subversion revision 1380202.

          Touches the following files:

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
          M java/engine/org/apache/derby/iapi/sql/dictionary/StatementGenericPermission.java
          M java/engine/org/apache/derby/iapi/sql/dictionary/PermDescriptor.java
          M java/engine/org/apache/derby/impl/sql/compile/PrivilegeNode.java
          M java/engine/org/apache/derby/impl/sql/depend/BasicDependencyManager.java

          Add user-defined aggregates to the dependency/privilege managers.

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java

          Wire-in dependency and privilege checking for invocations of user-defined aggregates.

          -------------

          M java/tools/org/apache/derby/impl/tools/dblook/DB_GrantRevoke.java
          M java/tools/org/apache/derby/loc/toolsmessages.properties

          Support for reconstructing permissions on aggregates when the database schema is dumped.

          -------------

          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/UDAPermsTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java

          Add a new test for USAGE privilege on user-defined aggregates

          -------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Add a test case for dependencies of view/triggers on user-defined aggregates.

          Show
          Rick Hillegas added a comment - Attaching derby-672-06-aa-grantRevoke.diff. This patch adds GRANT/REVOKE USAGE support for user-defined aggregates. Regression tests passed cleanly for me. Committed at subversion revision 1380202. Touches the following files: ------------- M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj M java/engine/org/apache/derby/iapi/sql/dictionary/StatementGenericPermission.java M java/engine/org/apache/derby/iapi/sql/dictionary/PermDescriptor.java M java/engine/org/apache/derby/impl/sql/compile/PrivilegeNode.java M java/engine/org/apache/derby/impl/sql/depend/BasicDependencyManager.java Add user-defined aggregates to the dependency/privilege managers. ------------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java Wire-in dependency and privilege checking for invocations of user-defined aggregates. ------------- M java/tools/org/apache/derby/impl/tools/dblook/DB_GrantRevoke.java M java/tools/org/apache/derby/loc/toolsmessages.properties Support for reconstructing permissions on aggregates when the database schema is dumped. ------------- A java/testing/org/apache/derbyTesting/functionTests/tests/lang/UDAPermsTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java Add a new test for USAGE privilege on user-defined aggregates ------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Add a test case for dependencies of view/triggers on user-defined aggregates.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-07-aa-fixJSR169again.diff. This is a second attempt to fix UngroupedAggregatesNegativeTest. The previous fix just seemed to push the problem around. Committed at subversion revision 1380207.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-07-aa-fixJSR169again.diff. This is a second attempt to fix UngroupedAggregatesNegativeTest. The previous fix just seemed to push the problem around. Committed at subversion revision 1380207. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UngroupedAggregatesNegativeTest.java
          Hide
          Mamta A. Satoor added a comment -

          Hi Rick,

          The user-defined aggregate name will not conflict with builtin function names, right? May be it is not even an issue for user-defined aggregates but I wanted to bring it up anyways. There is a pre-existing jira for conflict between user-defined function names with builtin functions DERBY-5901 (You can declare user-defined functions which shadow builtin functions by the same name. )

          Show
          Mamta A. Satoor added a comment - Hi Rick, The user-defined aggregate name will not conflict with builtin function names, right? May be it is not even an issue for user-defined aggregates but I wanted to bring it up anyways. There is a pre-existing jira for conflict between user-defined function names with builtin functions DERBY-5901 (You can declare user-defined functions which shadow builtin functions by the same name. )
          Hide
          Rick Hillegas added a comment -

          Hi Mamta,

          Thanks for thinking about this issue. Right, I have put in logic to prevent user-defined aggregate names from colliding with the names of functions, both builtin functions and user-defined functions. Thinking about this collision is what caused me to stumble across the existing problem with user-defined functions and to log DERBY-5901. Technically, all that has to be prevented is the collision of user-defined aggregate names with the names of 1-arg functions. If someone complains about the over-broad limitation, we can dial it back later on. Thanks.

          Show
          Rick Hillegas added a comment - Hi Mamta, Thanks for thinking about this issue. Right, I have put in logic to prevent user-defined aggregate names from colliding with the names of functions, both builtin functions and user-defined functions. Thinking about this collision is what caused me to stumble across the existing problem with user-defined functions and to log DERBY-5901 . Technically, all that has to be prevented is the collision of user-defined aggregate names with the names of 1-arg functions. If someone complains about the over-broad limitation, we can dial it back later on. Thanks.
          Hide
          Rick Hillegas added a comment -

          Attaching 3rd attempt to fix test failure on jsr 169: derby-672-08-aa-fixJSR169yetAgain.diff. Committed at subversion revision 1380784.

          Show
          Rick Hillegas added a comment - Attaching 3rd attempt to fix test failure on jsr 169: derby-672-08-aa-fixJSR169yetAgain.diff. Committed at subversion revision 1380784.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-09-ab-udtAggregates.diff. This patch makes it possible to create aggregates on user defined types. I am running regression tests now.

          Touches the following files:

          ----------

          M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java
          M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java

          The input and return types of the aggregate needed to be bound in order for CREATE DERBY AGGREGATE to succeed on user defined types.

          ----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/FullName.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml

          Additional test case for creating aggregates on a user-defined type.

          Show
          Rick Hillegas added a comment - Attaching derby-672-09-ab-udtAggregates.diff. This patch makes it possible to create aggregates on user defined types. I am running regression tests now. Touches the following files: ---------- M java/engine/org/apache/derby/impl/sql/compile/QueryTreeNode.java M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java The input and return types of the aggregate needed to be bound in order for CREATE DERBY AGGREGATE to succeed on user defined types. ---------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java A java/testing/org/apache/derbyTesting/functionTests/tests/lang/FullName.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml Additional test case for creating aggregates on a user-defined type.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for eliminating the duplicated code. The 09-ab patch looks good to me. +1.

          Show
          Knut Anders Hatlen added a comment - Thanks for eliminating the duplicated code. The 09-ab patch looks good to me. +1.
          Hide
          Rick Hillegas added a comment -

          Thanks for the quick review, Knut. Tests passed cleanly for me. Committed derby-672-09-ab-udtAggregates.diff at subversion revision 1391034.

          Show
          Rick Hillegas added a comment - Thanks for the quick review, Knut. Tests passed cleanly for me. Committed derby-672-09-ab-udtAggregates.diff at subversion revision 1391034.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-10-af-typeBounds.diff. This patch implements something akin to the elegant pattern which Knut suggested in his comment on 2012-07-18. I am running regression tests now.

          With this patch, you can write a single generic Java class:

          public class GenericMode<V extends Comparable<V>> implements Aggregator<V,V,GenericMode<V>>

          {...}

          and bind many type-specific aggregates to it:

          create derby aggregate intMode_09 for int external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode';
          create derby aggregate varcharMode_09 for varchar( 5 ) external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode';
          create derby aggregate fullNameMode_09 for FullName_09 external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode';

          To achieve this, I replaced the type inspection code used on the input and return types of the aggregator. Previously, we were checking for raw types. With this patch, we are checking that the declared input and return types fit within the generic aggregator's type bounds.

          Touches the following files:

          -------------

          M java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java
          M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java

          Replaced the tricky discovery of raw types with a simpler scheme for
          discovering type bounds.

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java
          M java/engine/org/apache/derby/loc/messages.xml

          Used type bounds rather than raw types to match declared SQL types to
          actual Java types.

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java

          When a type mismatch occurs, report it using the SQL name of the
          aggregate rather than its Java name.

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java

          Bind user-defined types better.

          -------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Added test cases for generic aggregator classes.

          Show
          Rick Hillegas added a comment - Attaching derby-672-10-af-typeBounds.diff. This patch implements something akin to the elegant pattern which Knut suggested in his comment on 2012-07-18. I am running regression tests now. With this patch, you can write a single generic Java class: public class GenericMode<V extends Comparable<V>> implements Aggregator<V,V,GenericMode<V>> {...} and bind many type-specific aggregates to it: create derby aggregate intMode_09 for int external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode'; create derby aggregate varcharMode_09 for varchar( 5 ) external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode'; create derby aggregate fullNameMode_09 for FullName_09 external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode'; To achieve this, I replaced the type inspection code used on the input and return types of the aggregator. Previously, we were checking for raw types. With this patch, we are checking that the declared input and return types fit within the generic aggregator's type bounds. Touches the following files: ------------- M java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java Replaced the tricky discovery of raw types with a simpler scheme for discovering type bounds. ------------- M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java M java/engine/org/apache/derby/loc/messages.xml Used type bounds rather than raw types to match declared SQL types to actual Java types. ------------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java When a type mismatch occurs, report it using the SQL name of the aggregate rather than its Java name. ------------- M java/engine/org/apache/derby/impl/sql/compile/ValueNode.java Bind user-defined types better. ------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Added test cases for generic aggregator classes.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly on derby-672-10-af-typeBounds.diff.

          Show
          Rick Hillegas added a comment - Tests passed cleanly on derby-672-10-af-typeBounds.diff.
          Hide
          Rick Hillegas added a comment -

          Committed derby-672-10-af-typeBounds.diff at subversion revision 1392307.

          Show
          Rick Hillegas added a comment - Committed derby-672-10-af-typeBounds.diff at subversion revision 1392307.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-11-aa-tests.diff. This patch adds a battery of data type tests for user-defined aggregates. I will run regression tests.

          Touches the following files:

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java
          M java/engine/org/apache/derby/loc/messages.xml

          Prevent XML from being used as an input or return type of a user-defined aggregate. We can't support aggregates on XML types until we add support for java.sql.SQLXML, the corresponding Java data type.

          -------------

          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java

          Handle binary SQL types by translating their Java type name as "[B" rather than "byte[]".

          -------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/LobMode.java

          Additional tests.

          Show
          Rick Hillegas added a comment - Attaching derby-672-11-aa-tests.diff. This patch adds a battery of data type tests for user-defined aggregates. I will run regression tests. Touches the following files: ------------- M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java M java/engine/org/apache/derby/loc/messages.xml Prevent XML from being used as an input or return type of a user-defined aggregate. We can't support aggregates on XML types until we add support for java.sql.SQLXML, the corresponding Java data type. ------------- M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java Handle binary SQL types by translating their Java type name as "[B" rather than "byte[]". ------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java A java/testing/org/apache/derbyTesting/functionTests/tests/lang/LobMode.java Additional tests.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-11-aa-tests.diff.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-11-aa-tests.diff.
          Hide
          Knut Anders Hatlen added a comment -

          In UserAggregateDefinition.getJavaClass():

          + if ( "byte[]".equals( javaClassName ) )

          { javaClassName = "[B"; }

          Perhaps just return byte[].class directly, so that we get rid of one of the magic strings? And maybe add a short comment saying why byte[] needs special handling.

          It might also be helpful to add a class-level javadoc comment to LobMode describing what kind of operation it implements.

          The catch clause in LobMode.toString(Object) loses the stack trace of the original exception. Using "throw new IllegalArgumentException(e)" would produce a better trace, should it ever fail.

          Show
          Knut Anders Hatlen added a comment - In UserAggregateDefinition.getJavaClass(): + if ( "byte[]".equals( javaClassName ) ) { javaClassName = "[B"; } Perhaps just return byte[].class directly, so that we get rid of one of the magic strings? And maybe add a short comment saying why byte[] needs special handling. It might also be helpful to add a class-level javadoc comment to LobMode describing what kind of operation it implements. The catch clause in LobMode.toString(Object) loses the stack trace of the original exception. Using "throw new IllegalArgumentException(e)" would produce a better trace, should it ever fail.
          Hide
          Knut Anders Hatlen added a comment -

          Two more nits:

          LobMode.Accumulator.compareTo() doesn't satisfy the contract of Comparable.compareTo() in its handling of null values. It's supposed to throw NPE if the object is null, but the implemented method returns 1. Just removing the null check at the beginning of the method should be enough to make it satisfy the contract.

          LobMode.terminate() would be more concise if it used the Collections.max() library method. Then we could also remove the SuppressWarnings annotation from the method, as we no longer needed an intermediate, weakly typed array representation of the accumulators. The body of the method could be as simple as return _accumulators.isEmpty() ? null : Collections.max(_accumulators.values()).getValue().

          (Both of these comments also apply to the already existing GenericMode and ModeAggregate classes, by the way.)

          Show
          Knut Anders Hatlen added a comment - Two more nits: LobMode.Accumulator.compareTo() doesn't satisfy the contract of Comparable.compareTo() in its handling of null values. It's supposed to throw NPE if the object is null, but the implemented method returns 1. Just removing the null check at the beginning of the method should be enough to make it satisfy the contract. LobMode.terminate() would be more concise if it used the Collections.max() library method. Then we could also remove the SuppressWarnings annotation from the method, as we no longer needed an intermediate, weakly typed array representation of the accumulators. The body of the method could be as simple as return _accumulators.isEmpty() ? null : Collections.max(_accumulators.values()).getValue(). (Both of these comments also apply to the already existing GenericMode and ModeAggregate classes, by the way.)
          Hide
          Rick Hillegas added a comment -

          Thanks for the quick review, Knut. Attaching derby-672-11-ab-tests.diff. This version makes all of the improvements you suggested. Committed at subversion revision 1396589.

          I also added another test condition. UserDefinedAggregatesTest.test_11_datatypes() will now fail if we add another builtin datatype to Derby. The comment accompanying that check explains that when we add a new builtin datatype, we need to add a corresponding test case to test_11_datatypes().

          Touches the following additional file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java

          Show
          Rick Hillegas added a comment - Thanks for the quick review, Knut. Attaching derby-672-11-ab-tests.diff. This version makes all of the improvements you suggested. Committed at subversion revision 1396589. I also added another test condition. UserDefinedAggregatesTest.test_11_datatypes() will now fail if we add another builtin datatype to Derby. The comment accompanying that check explains that when we add a new builtin datatype, we need to add a corresponding test case to test_11_datatypes(). Touches the following additional file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-12-aa-implicitCasts.diff. This patch attempts to make datatype coercion work for user-defined aggregates the way that it works for function arguments. I am running regression tests now.

          This patch makes two noteworthy changes:

          1) The input arg to the user-defined aggregate is wrapped in a CAST node if we would do the same thing when invoking a similar user-defined function on that argument.

          2) Input datatype checking for user-defined aggregates is relaxed. Instead of demanding exact type match, we check to see whether the actual datatype can be stored in the expected type. This is what we do for user-defined functions. This allows implicit casting to occur at runtime.

          Touches the following files:

          --------------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java
          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java
          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java

          Changes needed for (1) and (2).

          --------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Additional tests.

          Show
          Rick Hillegas added a comment - Attaching derby-672-12-aa-implicitCasts.diff. This patch attempts to make datatype coercion work for user-defined aggregates the way that it works for function arguments. I am running regression tests now. This patch makes two noteworthy changes: 1) The input arg to the user-defined aggregate is wrapped in a CAST node if we would do the same thing when invoking a similar user-defined function on that argument. 2) Input datatype checking for user-defined aggregates is relaxed. Instead of demanding exact type match, we check to see whether the actual datatype can be stored in the expected type. This is what we do for user-defined functions. This allows implicit casting to occur at runtime. Touches the following files: -------------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java Changes needed for (1) and (2). -------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Additional tests.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me. Committed at subversion revision 1398352.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me. Committed at subversion revision 1398352.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-13-aa-differentReturnType.diff. This patch makes it possible for a user-defined aggregate to have different input and return types. I am running regression tests now.

          Touches the following files:

          --------------

          M java/engine/org/apache/derby/impl/sql/compile/GroupByNode.java

          The problem was that the type of the aggregate result column was being derived from the type of the aggregate's input expression, for all aggregates except for COUNT. This file is changed to use the actual bound type of the aggregate result.

          --------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/LongMagnitude.java

          Additional tests.

          Show
          Rick Hillegas added a comment - Attaching derby-672-13-aa-differentReturnType.diff. This patch makes it possible for a user-defined aggregate to have different input and return types. I am running regression tests now. Touches the following files: -------------- M java/engine/org/apache/derby/impl/sql/compile/GroupByNode.java The problem was that the type of the aggregate result column was being derived from the type of the aggregate's input expression, for all aggregates except for COUNT. This file is changed to use the actual bound type of the aggregate result. -------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/lang/build.xml A java/testing/org/apache/derbyTesting/functionTests/tests/lang/LongMagnitude.java Additional tests.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-13-aa-differentReturnType.diff. Committed at subversion revision 1398489.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-13-aa-differentReturnType.diff. Committed at subversion revision 1398489.
          Hide
          Rick Hillegas added a comment -

          Attaching 3rd rev of the functional spec for this feature. This rev incorporates the following changes:

          o Clarifies that a UDA's input and return types are always nullable.
          o Removes SQLException from the signature of Aggregator.
          o Clarifies that XML is not a valid datatype for UDA inputs and return values.
          o Clarifies that the type of a value passed to a UDA must be storable in the input type of the UDA.
          o Replaces the "Open Questions" section with an appendix showing an example which is suitable for the user guides.

          Show
          Rick Hillegas added a comment - Attaching 3rd rev of the functional spec for this feature. This rev incorporates the following changes: o Clarifies that a UDA's input and return types are always nullable. o Removes SQLException from the signature of Aggregator. o Clarifies that XML is not a valid datatype for UDA inputs and return values. o Clarifies that the type of a value passed to a UDA must be storable in the input type of the UDA. o Replaces the "Open Questions" section with an appendix showing an example which is suitable for the user guides.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-14-aa-collations.diff. This patch adds a fixture to make the user-defined aggregate tests run both with and without territory based collations. Committed at subversion revision 1398934.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-14-aa-collations.diff. This patch adds a fixture to make the user-defined aggregate tests run both with and without territory based collations. Committed at subversion revision 1398934. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-15-aa-setCollation.diff. This patch sets the collation for the string input and return types of user-defined aggregates in the same way that it is set for the arguments and return values of functions. Committed at subversion revision 1398952.

          Touches the following files:

          M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java
          M java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-15-aa-setCollation.diff. This patch sets the collation for the string input and return types of user-defined aggregates in the same way that it is set for the arguments and return values of functions. Committed at subversion revision 1398952. Touches the following files: M java/engine/org/apache/derby/impl/sql/compile/CreateAliasNode.java M java/engine/org/apache/derby/catalog/types/AggregateAliasInfo.java
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-16-aa-forbidInGroupBy.diff. This patch forbids user-defined aggregates in GROUP BY clauses, just as builtin aggregates are illegal there. I will run regression tests.

          Touches the following files:

          ------------

          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
          M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java

          Logic to forbid user-defined aggregates in GROUP by clauses.

          ------------

          M java/engine/org/apache/derby/impl/sql/compile/CollectNodesVisitor.java

          Corrected a javadoc comment here.

          ------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Test case.

          Show
          Rick Hillegas added a comment - Attaching derby-672-16-aa-forbidInGroupBy.diff. This patch forbids user-defined aggregates in GROUP BY clauses, just as builtin aggregates are illegal there. I will run regression tests. Touches the following files: ------------ M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj M java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java Logic to forbid user-defined aggregates in GROUP by clauses. ------------ M java/engine/org/apache/derby/impl/sql/compile/CollectNodesVisitor.java Corrected a javadoc comment here. ------------ M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Test case.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly on derby-672-16-aa-forbidInGroupBy.diff. Committed at subversion revision 1399364.

          Show
          Rick Hillegas added a comment - Tests passed cleanly on derby-672-16-aa-forbidInGroupBy.diff. Committed at subversion revision 1399364.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-17-aa-moreTests.diff. This patch adds negative tests for more illegal uses of user-defined aggregates. Along the way, some code had to be corrected. Tests ran cleanly for me. Committed at subversion revision 1400083.

          Touches the following files:

          -----------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java

          Changes some error messages to use the schema-qualified name of the user-defined aggregate rather than the name of the class it is bound to.

          -----------

          M java/engine/org/apache/derby/impl/sql/compile/ConditionalNode.java

          I was seeing NPEs when using user-defined aggregates in CASE expressions for the following reason: although the aggregate-bearing clauses were being bound in isolation, the bound forms weren't being substituted back into the larger expression tree. I corrected this by putting the bound forms back into the tree.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Additional test cases.

          Show
          Rick Hillegas added a comment - Attaching derby-672-17-aa-moreTests.diff. This patch adds negative tests for more illegal uses of user-defined aggregates. Along the way, some code had to be corrected. Tests ran cleanly for me. Committed at subversion revision 1400083. Touches the following files: ----------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java Changes some error messages to use the schema-qualified name of the user-defined aggregate rather than the name of the class it is bound to. ----------- M java/engine/org/apache/derby/impl/sql/compile/ConditionalNode.java I was seeing NPEs when using user-defined aggregates in CASE expressions for the following reason: although the aggregate-bearing clauses were being bound in isolation, the bound forms weren't being substituted back into the larger expression tree. I corrected this by putting the bound forms back into the tree. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Additional test cases.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-18-aa-udaInJar.diff. This patch adds support for running user-defined aggregates from jar files stored in the database. I am running regression tests now.

          Touches the following files:

          -----------

          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java

          Fault-in Aggregator before faulting-in the user-supplied class. I don't think this is necessary but it looks cleaner.

          -----------

          M java/engine/org/apache/derby/impl/services/reflect/JarLoader.java

          Add the Aggregator package to the list of Derby packages which the database class loader is willing to load.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/DatabaseClassLoadingTest.java
          A java/testing/org/apache/derbyTesting/functionTests/tests/lang/median_uda.jar

          Test for this functionality. The jar file holds a Median aggregator. A new test case in DatabaseClassLoadingTest installs the jar, wires it into the database class path, and then invokes the aggregate inside.

          Show
          Rick Hillegas added a comment - Attaching derby-672-18-aa-udaInJar.diff. This patch adds support for running user-defined aggregates from jar files stored in the database. I am running regression tests now. Touches the following files: ----------- M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java Fault-in Aggregator before faulting-in the user-supplied class. I don't think this is necessary but it looks cleaner. ----------- M java/engine/org/apache/derby/impl/services/reflect/JarLoader.java Add the Aggregator package to the list of Derby packages which the database class loader is willing to load. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/DatabaseClassLoadingTest.java A java/testing/org/apache/derbyTesting/functionTests/tests/lang/median_uda.jar Test for this functionality. The jar file holds a Median aggregator. A new test case in DatabaseClassLoadingTest installs the jar, wires it into the database class path, and then invokes the aggregate inside.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Rick,

          I saw you made this change in revision 1400083:

          /** Get the SQL name of the aggregate */
          public String getSQLName()

          • throws StandardException
            {
            if ( isUserDefinedAggregate() )
            {
          • return ((UserAggregateDefinition) uad).getAliasDescriptor().getQualifiedName();
            + try { + return ((UserAggregateDefinition) uad).getAliasDescriptor().getQualifiedName(); + }

            catch (StandardException se)
            +

            { + return aggregateName; + }

            }
            else

            { return aggregateName; }

            }

          Could you add a comment explaining what could cause exceptions here and why it's OK to ignore those exceptions?
          Thanks.

          Show
          Knut Anders Hatlen added a comment - Hi Rick, I saw you made this change in revision 1400083: /** Get the SQL name of the aggregate */ public String getSQLName() throws StandardException { if ( isUserDefinedAggregate() ) { return ((UserAggregateDefinition) uad).getAliasDescriptor().getQualifiedName(); + try { + return ((UserAggregateDefinition) uad).getAliasDescriptor().getQualifiedName(); + } catch (StandardException se) + { + return aggregateName; + } } else { return aggregateName; } } Could you add a comment explaining what could cause exceptions here and why it's OK to ignore those exceptions? Thanks.
          Hide
          Rick Hillegas added a comment -

          Hi Knut,

          Thanks for taking a look at the patch. I will add a comment to a future patch, probably derby-672-19-????. An exception can occur if the data dictionary fails to find the schema descriptor for the schema which holds the user-defined aggregate. Something would have to be seriously broken for that to occur. It would indicate a corrupt DB. The fallback action is to return a different identifier for the user-defined aggregate in order to give the user some information rather than choking on the database corruption. I don't think we would ever fall into this code. That is because I would expect the corrupted DB would have tripped other schema resolution errors long before we got to this code. Thanks.

          Show
          Rick Hillegas added a comment - Hi Knut, Thanks for taking a look at the patch. I will add a comment to a future patch, probably derby-672-19-????. An exception can occur if the data dictionary fails to find the schema descriptor for the schema which holds the user-defined aggregate. Something would have to be seriously broken for that to occur. It would indicate a corrupt DB. The fallback action is to return a different identifier for the user-defined aggregate in order to give the user some information rather than choking on the database corruption. I don't think we would ever fall into this code. That is because I would expect the corrupted DB would have tripped other schema resolution errors long before we got to this code. Thanks.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-672-18-aa-udaInJar.diff. Committed at subversion revision 1400161.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-672-18-aa-udaInJar.diff. Committed at subversion revision 1400161.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-19-aa-precisionChecks.diff. This patch adds tests for precision mismatches in datatype coercion for user-defined aggregates. Committed at subversion revision 1400208.

          Touches the following files:

          -----------

          M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java

          Adds a comment to the exception-swallowing in getSQLName() as requested by Knut.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Precision tests.

          Show
          Rick Hillegas added a comment - Attaching derby-672-19-aa-precisionChecks.diff. This patch adds tests for precision mismatches in datatype coercion for user-defined aggregates. Committed at subversion revision 1400208. Touches the following files: ----------- M java/engine/org/apache/derby/impl/sql/compile/AggregateNode.java Adds a comment to the exception-swallowing in getSQLName() as requested by Knut. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Precision tests.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-20-aa-exactBounds.diff. This patch tightens up the bounds checking on the input and return types of user-defined aggregates. Committed at subversion revision 1400984.

          When I simplified bounds checking for input and return types, I lost the ability to catch some user mistakes at compile time. So for instance, given the following UDA:

          create derby aggregate bigintMode_16 for bigint
          external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode$IntMode'

          ...the following statement would fail at run time when the user code tried to cast a Long to an Integer:

          select bigintMode_16( b ) from bigintMode_16_mode_inputs

          The solution was to revive the tricky code which I deprecated in patch derby-672-10-af-typeBounds.diff and to use that tricky code alongside the simpler bounds checking introduced by that patch. The tricky code catches the above mismatch at bind() time rather than failing at run time. I am not pleased by the complexity of the tricky code but I think it is better to catch these kinds of errors when the statement is compiled.

          Touches the following files:

          -----------

          M java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java
          M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java

          Re-introduce the tricky code.

          -----------

          M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java

          Wire the tricky code back into bind-time type resolution.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java

          While experimenting, I changed the name of the GenericMode type variable from V to B. This helps to clearly disambiguate it from the V type variable of the Aggregator class itself. It's a cosmetic change which I think may be useful in the future if we need to keep improving bounds checking for UDAs.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Additional tests.

          Show
          Rick Hillegas added a comment - Attaching derby-672-20-aa-exactBounds.diff. This patch tightens up the bounds checking on the input and return types of user-defined aggregates. Committed at subversion revision 1400984. When I simplified bounds checking for input and return types, I lost the ability to catch some user mistakes at compile time. So for instance, given the following UDA: create derby aggregate bigintMode_16 for bigint external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode$IntMode' ...the following statement would fail at run time when the user code tried to cast a Long to an Integer: select bigintMode_16( b ) from bigintMode_16_mode_inputs The solution was to revive the tricky code which I deprecated in patch derby-672-10-af-typeBounds.diff and to use that tricky code alongside the simpler bounds checking introduced by that patch. The tricky code catches the above mismatch at bind() time rather than failing at run time. I am not pleased by the complexity of the tricky code but I think it is better to catch these kinds of errors when the statement is compiled. Touches the following files: ----------- M java/engine/org/apache/derby/iapi/services/loader/Java5ClassInspector.java M java/engine/org/apache/derby/iapi/services/loader/ClassInspector.java Re-introduce the tricky code. ----------- M java/engine/org/apache/derby/impl/sql/compile/UserAggregateDefinition.java Wire the tricky code back into bind-time type resolution. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/GenericMode.java While experimenting, I changed the name of the GenericMode type variable from V to B. This helps to clearly disambiguate it from the V type variable of the Aggregator class itself. It's a cosmetic change which I think may be useful in the future if we need to keep improving bounds checking for UDAs. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Additional tests.
          Hide
          Knut Anders Hatlen added a comment -

          I took another look at the exception handling in AggregateNode.getSQLName() and found that there was no code called from it that could ever cause a StandardException. The need to catch StandardException came from AliasDescriptor.getSchemaName(), which is declared as throws StandardException even though it does not call any methods that could raise StandardExceptions.

          The patch d672-no-exceptions.diff removes the unnecessary throws clause from AliasDescriptor.getSchemaName() and the methods that call it, as well as the exception handler in getSQLName(). The patch also removes a redundant null check in isUserDefinedAggregate() (redundant because instanceof implies not null). All the regression tests ran cleanly. Committed revision 1401250.

          Show
          Knut Anders Hatlen added a comment - I took another look at the exception handling in AggregateNode.getSQLName() and found that there was no code called from it that could ever cause a StandardException. The need to catch StandardException came from AliasDescriptor.getSchemaName(), which is declared as throws StandardException even though it does not call any methods that could raise StandardExceptions. The patch d672-no-exceptions.diff removes the unnecessary throws clause from AliasDescriptor.getSchemaName() and the methods that call it, as well as the exception handler in getSQLName(). The patch also removes a redundant null check in isUserDefinedAggregate() (redundant because instanceof implies not null). All the regression tests ran cleanly. Committed revision 1401250.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-21-aa-typeDependencies.diff. This patch adds support for dependencies of user-defined aggregates on user-defined types. Committed at subversion revision 1401303.

          Touches the following files:

          -----------

          M java/engine/org/apache/derby/impl/sql/execute/DDLConstantAction.java

          Added dependencies of aggregates on types similar to the dependencies of routines on types.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UDAPermsTest.java

          Tests for GRANT/REVOKE USAGE on types which are used by aggregates.

          -----------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java

          Tests for attempting to drop a type which an aggregate uses.

          Show
          Rick Hillegas added a comment - Attaching derby-672-21-aa-typeDependencies.diff. This patch adds support for dependencies of user-defined aggregates on user-defined types. Committed at subversion revision 1401303. Touches the following files: ----------- M java/engine/org/apache/derby/impl/sql/execute/DDLConstantAction.java Added dependencies of aggregates on types similar to the dependencies of routines on types. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UDAPermsTest.java Tests for GRANT/REVOKE USAGE on types which are used by aggregates. ----------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/UserDefinedAggregatesTest.java Tests for attempting to drop a type which an aggregate uses.
          Hide
          Rick Hillegas added a comment -

          Attaching a 4th rev of the functional spec for this feature. This rev makes a couple minor changes:

          o Further clarify that a UDA's input and return types may not be XML.
          o Add package declaration to javadoc for Aggregator.

          At this point, I have reached the limit of my ability to test this feature. I cannot see past my own blinders. Hopefully, other people will be able to find some time to buddy-test this feature.

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Attaching a 4th rev of the functional spec for this feature. This rev makes a couple minor changes: o Further clarify that a UDA's input and return types may not be XML. o Add package declaration to javadoc for Aggregator. At this point, I have reached the limit of my ability to test this feature. I cannot see past my own blinders. Hopefully, other people will be able to find some time to buddy-test this feature. Thanks, -Rick
          Hide
          Kim Haase added a comment -

          Is it also true, as we said of user-defined types, that "If a qualified aggregate name is specified, the
          schema name cannot begin with SYS"? Thanks!

          Show
          Kim Haase added a comment - Is it also true, as we said of user-defined types, that "If a qualified aggregate name is specified, the schema name cannot begin with SYS"? Thanks!
          Hide
          Rick Hillegas added a comment -

          Hi Kim,

          Yes. You can't create a schema which begins with SYS so you can't create any objects in such a schema:

          ij version 10.10
          ij> connect 'jdbc:derby:memory:db;create=true';
          ij> create schema sysmine;
          ERROR 42939: An object cannot be created with the schema name 'SYSMINE'.
          ij> create derby aggregate sys.foo for int
          external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode';
          ERROR 42X62: 'CREATE DERBY AGGREGATE' is not allowed in the 'SYS' schema.
          ij> create derby aggregate sysmine.foo for int
          external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode';
          ERROR 42X62: 'CREATE DERBY AGGREGATE' is not allowed in the 'SYSMINE' schema.

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Hi Kim, Yes. You can't create a schema which begins with SYS so you can't create any objects in such a schema: ij version 10.10 ij> connect 'jdbc:derby:memory:db;create=true'; ij> create schema sysmine; ERROR 42939: An object cannot be created with the schema name 'SYSMINE'. ij> create derby aggregate sys.foo for int external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode'; ERROR 42X62: 'CREATE DERBY AGGREGATE' is not allowed in the 'SYS' schema. ij> create derby aggregate sysmine.foo for int external name 'org.apache.derbyTesting.functionTests.tests.lang.GenericMode'; ERROR 42X62: 'CREATE DERBY AGGREGATE' is not allowed in the 'SYSMINE' schema. Thanks, -Rick
          Hide
          Kim Haase added a comment -

          I notice the Documentation section of the spec mentions changes to the dblook documentation. Does the dblook tool produce SQL to recreate user-defined types, as well as UDAs? We didn't include that in the UDT documentation, so I thought I'd better check.

          Thanks!

          Show
          Kim Haase added a comment - I notice the Documentation section of the spec mentions changes to the dblook documentation. Does the dblook tool produce SQL to recreate user-defined types, as well as UDAs? We didn't include that in the UDT documentation, so I thought I'd better check. Thanks!
          Hide
          Rick Hillegas added a comment -

          Hi Kim,

          Yep, dblook outputs type-creating SQL too. Thanks.

          Show
          Rick Hillegas added a comment - Hi Kim, Yep, dblook outputs type-creating SQL too. Thanks.
          Hide
          Kim Haase added a comment -

          Thanks – I'll update the tools topic to include UDTs too.

          Show
          Kim Haase added a comment - Thanks – I'll update the tools topic to include UDTs too.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-22-aa-makeModeAggregateStateSerializable.diff. This patch makes a helper class Serializable. The helper class is used by the test class ModeAggregate. The UserDefinedAggregatesTest runs cleanly for me with this patch. Committed at subversion revision 1459784.

          I discovered that the helper class needs to be Serializable while writing experiments to try to force Derby to perform the merge() phase of aggregation. In my experiment, a grouped aggregate with millions of groups ended up serializing partial results to disk and this tripped across the unserializability of the helper class. It would be worthwhile to file an issue to add some advice to the Reference Manual: the Aggregator interface extends Serializable, which means that all of the state in your UDA needs to be serializable.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-22-aa-makeModeAggregateStateSerializable.diff. This patch makes a helper class Serializable. The helper class is used by the test class ModeAggregate. The UserDefinedAggregatesTest runs cleanly for me with this patch. Committed at subversion revision 1459784. I discovered that the helper class needs to be Serializable while writing experiments to try to force Derby to perform the merge() phase of aggregation. In my experiment, a grouped aggregate with millions of groups ended up serializing partial results to disk and this tripped across the unserializability of the helper class. It would be worthwhile to file an issue to add some advice to the Reference Manual: the Aggregator interface extends Serializable, which means that all of the state in your UDA needs to be serializable. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/lang/ModeAggregate.java
          Hide
          Rick Hillegas added a comment -

          Ported 1459784 to 10.10 branch at subversion revision 1459787.

          Show
          Rick Hillegas added a comment - Ported 1459784 to 10.10 branch at subversion revision 1459787.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-672-23-aa-improveJavadocForAggregator.merge.diff. This patch expands the javadoc for the Aggregator.merge() method. Committed at subversion revision 1459837.

          Buddy-testing raised the following issue: How can users test their merge() implementations? It's not clear what kinds of queries cause Derby to call the merge() method. The answer is that merge() is called when Derby has to spill intermediate results to disk (and retrieve them later) for grouped aggregates with a large number of groups. A query like the following can cause Derby to do this. The merge() method is called when Derby encounters rows which belong to groups which spilled to disk previously:

          select a, mode( b ) from mode_inputs group by a order by a

          Touches the following file:

          M java/engine/org/apache/derby/agg/Aggregator.java

          Show
          Rick Hillegas added a comment - Attaching derby-672-23-aa-improveJavadocForAggregator.merge.diff. This patch expands the javadoc for the Aggregator.merge() method. Committed at subversion revision 1459837. Buddy-testing raised the following issue: How can users test their merge() implementations? It's not clear what kinds of queries cause Derby to call the merge() method. The answer is that merge() is called when Derby has to spill intermediate results to disk (and retrieve them later) for grouped aggregates with a large number of groups. A query like the following can cause Derby to do this. The merge() method is called when Derby encounters rows which belong to groups which spilled to disk previously: select a, mode( b ) from mode_inputs group by a order by a Touches the following file: M java/engine/org/apache/derby/agg/Aggregator.java
          Hide
          Rick Hillegas added a comment -

          Ported 1459837 from trunk to 10.10 branch at subversion revision 1459837.

          Show
          Rick Hillegas added a comment - Ported 1459837 from trunk to 10.10 branch at subversion revision 1459837.
          Hide
          Rick Hillegas added a comment -

          Thanks for buddy-testing user-defined aggregates, Dag. Here are responses to questions you posed on this wiki page: http://wiki.apache.org/db-derby/TenTenOneBuddyTesting

          ----------------------

          >* Currently, our docs say:
          >
          > "An unqualified UDA name may not be the name of an aggregate defined in part 2 of the SQL Standard, section 10.9:
          >
          > ANY..
          >
          > But even when name is qualified, I see:
          >
          > ij> create derby aggregate app.ANY for int returns int external name 'foo.Agg';
          > ERROR 42X01: Syntax error: Encountered "ANY" at line 1, column 28.
          >
          > I need to quote it:
          >
          > ij> create derby aggregate app."ANY" for int returns int external name 'foo.Agg';
          > 0 rows inserted/updated/deleted
          >
          > Code or doc correct?

          The wording in the Reference Manual could be improved. Logged DERBY-6116 to track this.

          ----------------------

          >*
          >
          > "In general, UDAs live in the same namespace as one-argument user-defined functions (see CREATE FUNCTION statement)."
          >
          > But ij doesn't show aggregates:
          >
          > ij> create function app."ANY" (i int) returns int parameter style java language java external name 'foo.bar';
          >
          > ERROR X0Y87: There is already an aggregate or function with one argument whose name is 'APP'.'ANY'.
          >
          > ij> show functions in app;
          > FUNCTION_SCHEM|FUNCTION_NAME |REMARKS
          > -------------------------------------------------------------------------------
          > 0 rows selected

          I think what's missing here is a "show aggregates" command. Logged DERBY-6119 to track this.

          ----------------------

          >* checked simple int aggregator: found agg class could not be static nested class in another
          >
          > Expected or pilot error? If expected, why? document? Currently we say:
          >
          > "The !ClassNameString is a single-quoted string. It is the full name of a Java class which implements the org.apache.derby.agg.Aggre>gator interface."

          I believe that the problem here is that a period rather than a dollar sign was used in the name of the nested class in the "external name" clause. This gotcha trips up a lot of people and indicates that we need to provide some more advice in the Reference Manual. Logged DERBY-6120 to track this issue.

          ----------------------

          >* The merge method wasn't used in my simple example. When is it used?
          >
          > If only needed in compex queries, we might want to warn the user on how to construct test cases to debug it...

          This method is called when Derby has to spill intermediate results to disk (and retrieve them later) when processing a grouped aggregate with a large number of groups. I have added this explanation to the javadoc for Aggregator.merge() via patch derby-672-23-aa-improveJavadocForAggregator.merge.diff.

          While verifying this behavior, I tripped across a problem in the test class ModeAggregator: its state was not serializable. I corrected that problem via patch derby-672-22-aa-makeModeAggregateStateSerializable.diff. This prompted me to log an issue for beefing up the UDA documentation with the advice that all of the state of a UDA must be serializable: DERBY-6127.

          ----------------------

          >* In the GRANT-statement refman page, we are inconsisten when it comes to explaining
          >
          > identifiers (seen when looking at GRANT USAGE of aggregates): "table-Name" occurences are linked to a section explainig them (rre>ftablename.html), but UDA names are defined in-lined as
          >
          > GRANT USAGE ON DERBY AGGREGATE [ schemaName. ] SQL92Identifier TO grantees
          >
          > In a third variant,
          >
          > GRANT EXECUTE ON

          { FUNCTION | PROCEDURE }

          routine-designator TO grantees
          >
          > routine-designator is defined locally as
          >
          > routine-designator

          { function-name | procedure-name }


          >
          > without links to what "function-name" or "procedure-name" might look like. It would be good to harmonize the latter two forms to> a central definition as for "table-Name".
          >
          > Finally, I noted that the definition page for SchemaName doesn't link to the SQL92Identifier page...

          Logged DERBY-6121 to track this issue.

          Show
          Rick Hillegas added a comment - Thanks for buddy-testing user-defined aggregates, Dag. Here are responses to questions you posed on this wiki page: http://wiki.apache.org/db-derby/TenTenOneBuddyTesting ---------------------- >* Currently, our docs say: > > "An unqualified UDA name may not be the name of an aggregate defined in part 2 of the SQL Standard, section 10.9: > > ANY.. > > But even when name is qualified, I see: > > ij> create derby aggregate app.ANY for int returns int external name 'foo.Agg'; > ERROR 42X01: Syntax error: Encountered "ANY" at line 1, column 28. > > I need to quote it: > > ij> create derby aggregate app."ANY" for int returns int external name 'foo.Agg'; > 0 rows inserted/updated/deleted > > Code or doc correct? The wording in the Reference Manual could be improved. Logged DERBY-6116 to track this. ---------------------- >* > > "In general, UDAs live in the same namespace as one-argument user-defined functions (see CREATE FUNCTION statement)." > > But ij doesn't show aggregates: > > ij> create function app."ANY" (i int) returns int parameter style java language java external name 'foo.bar'; > > ERROR X0Y87: There is already an aggregate or function with one argument whose name is 'APP'.'ANY'. > > ij> show functions in app; > FUNCTION_SCHEM|FUNCTION_NAME |REMARKS > ------------------------------------------------------------------------------- > 0 rows selected I think what's missing here is a "show aggregates" command. Logged DERBY-6119 to track this. ---------------------- >* checked simple int aggregator: found agg class could not be static nested class in another > > Expected or pilot error? If expected, why? document? Currently we say: > > "The !ClassNameString is a single-quoted string. It is the full name of a Java class which implements the org.apache.derby.agg.Aggre>gator interface." I believe that the problem here is that a period rather than a dollar sign was used in the name of the nested class in the "external name" clause. This gotcha trips up a lot of people and indicates that we need to provide some more advice in the Reference Manual. Logged DERBY-6120 to track this issue. ---------------------- >* The merge method wasn't used in my simple example. When is it used? > > If only needed in compex queries, we might want to warn the user on how to construct test cases to debug it... This method is called when Derby has to spill intermediate results to disk (and retrieve them later) when processing a grouped aggregate with a large number of groups. I have added this explanation to the javadoc for Aggregator.merge() via patch derby-672-23-aa-improveJavadocForAggregator.merge.diff. While verifying this behavior, I tripped across a problem in the test class ModeAggregator: its state was not serializable. I corrected that problem via patch derby-672-22-aa-makeModeAggregateStateSerializable.diff. This prompted me to log an issue for beefing up the UDA documentation with the advice that all of the state of a UDA must be serializable: DERBY-6127 . ---------------------- >* In the GRANT-statement refman page, we are inconsisten when it comes to explaining > > identifiers (seen when looking at GRANT USAGE of aggregates): "table-Name" occurences are linked to a section explainig them (rre>ftablename.html), but UDA names are defined in-lined as > > GRANT USAGE ON DERBY AGGREGATE [ schemaName. ] SQL92Identifier TO grantees > > In a third variant, > > GRANT EXECUTE ON { FUNCTION | PROCEDURE } routine-designator TO grantees > > routine-designator is defined locally as > > routine-designator { function-name | procedure-name } > > without links to what "function-name" or "procedure-name" might look like. It would be good to harmonize the latter two forms to> a central definition as for "table-Name". > > Finally, I noted that the definition page for SchemaName doesn't link to the SQL92Identifier page... Logged DERBY-6121 to track this issue.
          Hide
          Knut Anders Hatlen added a comment -

          Is there more work remaining on this issue, or can it be resolved now? The feature was part of the Derby 10.10.1.1 release.

          Show
          Knut Anders Hatlen added a comment - Is there more work remaining on this issue, or can it be resolved now? The feature was part of the Derby 10.10.1.1 release.
          Hide
          Rick Hillegas added a comment -

          Thanks for the nudge, Knut. Resolving.

          Show
          Rick Hillegas added a comment - Thanks for the nudge, Knut. Resolving.

            People

            • Assignee:
              Rick Hillegas
              Reporter:
              Rick Hillegas
            • Votes:
              6 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development