[jira] [Comment Edited] (CALCITE-452) JavaRowFormat.comparer relies on proper equals/compareTo in custom row classes

["Julian Hyde (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14196448#comment-14196448 ] 

Julian Hyde edited comment on CALCITE-452 at 11/4/14 5:55 PM:
--------------------------------------------------------------

Makes sense. I think it's reasonable to expect people to implement equals and hashCode. We don't require/expect them to implement compareTo or Comparable because SQL doesn't have a "default ordering" of the rows of a relation.

So, that would mean adding equals and hashCode in Emp and similar classes.

Can you commit this? But please wait until the big CALCITE-296 code re-org is checked into master.


was (Author: julianhyde):
Makes sense. I think it's reasonable to expect people to implement equals and hashCode. We don't require/expect them to implement compareTo or Comparable because SQL doesn't have a "default ordering" of the rows of a relation.

Can you commit this? But please wait until the big CALCITE-296 code re-org is checked into master.

> JavaRowFormat.comparer relies on proper equals/compareTo in custom row classes
> ------------------------------------------------------------------------------
>
>                 Key: CALCITE-452
>                 URL: https://issues.apache.org/jira/browse/CALCITE-452
>             Project: Calcite
>          Issue Type: Bug
>    Affects Versions: 0.9.1
>            Reporter: Vladimir Sitnikov
>            Assignee: Vladimir Sitnikov
>            Priority: Minor
>
> When reviewing CALCITE-397 I noted that the fix was "convert to the target row type, then perform distinct".
> I wondered: why don't we perform distinct first, then convert row types? That might be much more efficient if the distinct reduces the number of rows dramatically, so we do not spend time on conversion.
> When I did tried this change, I noticed Calcite uses "default equals/compareTo".
> Not sure what is the best approach here: raise errors in such cases, create synthetic equals/hashCode/compareTo somehow, or reconvert to the type with proper comparison methods.
> Here's the change to use "distinct, then convert to the target physType":
> {code:java}
>             Expressions.return_(
>                 null,
> +                inputPhysType.convertTo(
>                 Expressions.call(
> +                    childExp,
> -                    inputPhysType.convertTo(childExp, physType),
>                     BuiltinMethod.DISTINCT.method,
>                     Expressions.<Expression>list()
>                             .appendIfNotNull(inputPhysType.comparer()))
> +                          , physType)
>                     ));
> {code}
> The generated java code is as following (note how {{.distinct()}} is used over {{JdbcTest$Employee}} that has no {{equals/compareTo}} methods):
> {code:java}
> /*   3 */ public net.hydromatic.linq4j.Enumerable bind(final net.hydromatic.optiq.DataContext root0) {
> /*   4 */   root = root0;
> /*   5 */   return net.hydromatic.linq4j.Linq4j.asEnumerable(((net.hydromatic.optiq.test.JdbcTest.HrSchema) ((net.hydromatic.optiq.impl.java.ReflectiveSchema) root.getRootSchema().getSubSchema("hr").unwrap(net.hydromatic.optiq.impl.java.ReflectiveSchema.class)).getTarget()).emps).distinct().select(new net.hydromatic.linq4j.function.Function1() {
> /*   6 */       public Object[] apply(net.hydromatic.optiq.test.JdbcTest.Employee o) {
> /*   7 */         return new Object[] {
> /*   8 */             o.empid,
> /*   9 */             o.deptno,
> /*  10 */             o.name,
> /*  11 */             o.salary,
> /*  12 */             o.commission};
> /*  13 */       }
> /*  14 */       public Object apply(Object o) {
> /*  15 */         return apply(
> /*  16 */           (net.hydromatic.optiq.test.JdbcTest.Employee) o);
> /*  17 */       }
> /*  18 */     }
> /*  19 */     );
> /*  20 */ }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)