[jira] [Commented] (CALCITE-4964) Reduce recursion when push predicate into CASE

2021-12-27 Thread Ziwei Liu (Jira)


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

Ziwei Liu commented on CALCITE-4964:


Thanks a lot, I will try to test it.

> Reduce recursion when push predicate into CASE
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4966) In class CsvEnumerator, the inner class RowConverter should be public

2021-12-27 Thread Ioan Eugen Stan (Jira)


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

Ioan Eugen Stan commented on CALCITE-4966:
--

I looked at the class and I agree - making it public should come with some 
tests.

Given time and me continuing to use Calcite, I'll write the tests. 

Thanks for the feedback :). 

> In class CsvEnumerator, the inner class RowConverter should be public
> -
>
> Key: CALCITE-4966
> URL: https://issues.apache.org/jira/browse/CALCITE-4966
> Project: Calcite
>  Issue Type: Bug
>  Components: file-adapter
>Reporter: Ioan Eugen Stan
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> Hello,
> I believe class org.apache.calcite.adapter.file.CsvEnumerator.RowConverter 
> should be public instead of package private since it's exposed in the public 
> api via 
> public constructor:
> {code:java}
> public CsvEnumerator(Source source, AtomicBoolean cancelFlag, boolean stream,
> @Nullable String @Nullable [] filterValues, RowConverter rowConverter) 
> { {code}
> public static method:
> {code:java}
> public static RowConverter<@Nullable Object[]> arrayConverter(
> List fieldTypes, List fields, boolean stream) {
>   return new ArrayRowConverter(fieldTypes, fields, stream);
> } {code}
>  
> Patch is trivial, add "public" in front :)
> {code:java}
> public abstract static class RowConverter { {code}
> Would love to get this into 1.29.0 - so I can make a surprise for Calcite :D 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4966) In class CsvEnumerator, the inner class RowConverter should be public

2021-12-27 Thread Julian Hyde (Jira)


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

Julian Hyde commented on CALCITE-4966:
--

I agree, it's a bug. But if someone were to fix it by making a class public 
that doesn't have a test, they would be introducing a new bug. So the fix needs 
to include a test for the now-public class.

By the way, the class was 'leaked' for a good reason - we didn't want to have 
an identical class in the file adapter and the example CSV adapter.

> In class CsvEnumerator, the inner class RowConverter should be public
> -
>
> Key: CALCITE-4966
> URL: https://issues.apache.org/jira/browse/CALCITE-4966
> Project: Calcite
>  Issue Type: Bug
>  Components: file-adapter
>Reporter: Ioan Eugen Stan
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> Hello,
> I believe class org.apache.calcite.adapter.file.CsvEnumerator.RowConverter 
> should be public instead of package private since it's exposed in the public 
> api via 
> public constructor:
> {code:java}
> public CsvEnumerator(Source source, AtomicBoolean cancelFlag, boolean stream,
> @Nullable String @Nullable [] filterValues, RowConverter rowConverter) 
> { {code}
> public static method:
> {code:java}
> public static RowConverter<@Nullable Object[]> arrayConverter(
> List fieldTypes, List fields, boolean stream) {
>   return new ArrayRowConverter(fieldTypes, fields, stream);
> } {code}
>  
> Patch is trivial, add "public" in front :)
> {code:java}
> public abstract static class RowConverter { {code}
> Would love to get this into 1.29.0 - so I can make a surprise for Calcite :D 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4966) In class CsvEnumerator, the inner class RowConverter should be public

2021-12-27 Thread Ioan Eugen Stan (Jira)


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

Ioan Eugen Stan commented on CALCITE-4966:
--

Hi,

A unit test testing what exactly ? 

As it stands right now the API is leaking a private class. 

This IMO is a bug. 

> In class CsvEnumerator, the inner class RowConverter should be public
> -
>
> Key: CALCITE-4966
> URL: https://issues.apache.org/jira/browse/CALCITE-4966
> Project: Calcite
>  Issue Type: Bug
>  Components: file-adapter
>Reporter: Ioan Eugen Stan
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> Hello,
> I believe class org.apache.calcite.adapter.file.CsvEnumerator.RowConverter 
> should be public instead of package private since it's exposed in the public 
> api via 
> public constructor:
> {code:java}
> public CsvEnumerator(Source source, AtomicBoolean cancelFlag, boolean stream,
> @Nullable String @Nullable [] filterValues, RowConverter rowConverter) 
> { {code}
> public static method:
> {code:java}
> public static RowConverter<@Nullable Object[]> arrayConverter(
> List fieldTypes, List fields, boolean stream) {
>   return new ArrayRowConverter(fieldTypes, fields, stream);
> } {code}
>  
> Patch is trivial, add "public" in front :)
> {code:java}
> public abstract static class RowConverter { {code}
> Would love to get this into 1.29.0 - so I can make a surprise for Calcite :D 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Updated] (CALCITE-4966) In class CsvEnumerator, the inner class RowConverter should be public

2021-12-27 Thread Ioan Eugen Stan (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4966?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ioan Eugen Stan updated CALCITE-4966:
-
Summary: In class CsvEnumerator, the inner class RowConverter should be 
public  (was: org.apache.calcite.adapter.file.CsvEnumerator.RowConverter should 
be public)

> In class CsvEnumerator, the inner class RowConverter should be public
> -
>
> Key: CALCITE-4966
> URL: https://issues.apache.org/jira/browse/CALCITE-4966
> Project: Calcite
>  Issue Type: Bug
>  Components: file-adapter
>Reporter: Ioan Eugen Stan
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 50m
>  Remaining Estimate: 0h
>
> Hello,
> I believe class org.apache.calcite.adapter.file.CsvEnumerator.RowConverter 
> should be public instead of package private since it's exposed in the public 
> api via 
> public constructor:
> {code:java}
> public CsvEnumerator(Source source, AtomicBoolean cancelFlag, boolean stream,
> @Nullable String @Nullable [] filterValues, RowConverter rowConverter) 
> { {code}
> public static method:
> {code:java}
> public static RowConverter<@Nullable Object[]> arrayConverter(
> List fieldTypes, List fields, boolean stream) {
>   return new ArrayRowConverter(fieldTypes, fields, stream);
> } {code}
>  
> Patch is trivial, add "public" in front :)
> {code:java}
> public abstract static class RowConverter { {code}
> Would love to get this into 1.29.0 - so I can make a surprise for Calcite :D 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4964) Reduce recursion when push predicate into CASE

2021-12-27 Thread Julian Hyde (Jira)


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

Julian Hyde commented on CALCITE-4964:
--

Or maybe the real problem is that this code is called even if there is no 
{{CASE}}? If so, should {{ReduceExpressionsRule}} check that there is a 
{{CASE}} somewhere in the expression before calling this code?

> Reduce recursion when push predicate into CASE
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4964) Reduce recursion when push predicate into CASE

2021-12-27 Thread Julian Hyde (Jira)


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

Julian Hyde commented on CALCITE-4964:
--

It seems to me that the benefit is proportional to the average depth of CASE 
branches. If the average depth is 3, then the benefit is 3x. Not worth it if 
this method is only called 10 times while preparing a typical query.

I can see that the method is called for all expressions, not just those with a 
{{CASE}}. That works in your favor, but perhaps those expressions are never 
changed (i.e. {{old}} always equals {{call}}).

Can you add a counter - say a static {{AtomicInteger}} - to that method, and 
see how many times the method is called, and how many times that loop iterates 
during the Calcite suite suite. See how many iterations your revised method 
saves.

And/or devise a test that is much, much slower without your change.

> Reduce recursion when push predicate into CASE
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4942) Deprecate boilerplate for Rel Metadata

2021-12-27 Thread Jacques Nadeau (Jira)


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

Jacques Nadeau commented on CALCITE-4942:
-

[~jamesstarr], I actually did several iterations on how to construct handlers 
to be cleaner/more modern/simpler. The best I came up with can be found in 
[this gist|https://gist.github.com/jacques-n/19e06a39704d789a06cdd06cffacc8c1].

There are two different things that I think are ideal to accomplish:
- handlers are generic and declare the relnode interface they operate on. This 
creates an explicit functional definition of what metadata methods should look 
like (something that only exists implicitly today).
- handlers have a generic method for metadata and use a generic definition. 
This allows a simplification of code that wants to invoke them since there are 
only 6 total functional signatures that exist across all metadata operations. 
(It's six to avoid boxing, otherwise it would be three.)

If we're doing breaking changes, I think it would be helpful to introduce these 
patterns to not only clean up the old stuff but introducing a more standardized 
way to express these things.

> Deprecate boilerplate for Rel Metadata
> --
>
> Key: CALCITE-4942
> URL: https://issues.apache.org/jira/browse/CALCITE-4942
> Project: Calcite
>  Issue Type: Improvement
>Reporter: James Starr
>Assignee: James Starr
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> To define a Rel metadata type you need to create 2 class, 2 methods and a 
> static variable.  However, current implementation of RelMetadata back by 
> Janino only require the Handler class a single method signature.  The current 
> metadata handler interface also has a generic that provides no type safety 
> through out code. 
> Currently to define a rel metadata type:
> {code:java}
> public class MyMetadata extends Metadata {
>   MetadataDef DEF = ...
>  
>  VALUE myMethod1();
>   class Handler extends RelHandler {
> VALUE myMethod2(RelNode, MetadataQuery)
>   }
> }
> {code}
> What is actually needed to define a rel metadata type:
> {code:java}
> class MyMetadataHandler extends RelHandler {
>   VALUE myMethod(RelNode, MetadataQuery)
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4964) Reduce recursion when push predicate into CASE

2021-12-27 Thread Ziwei Liu (Jira)


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

Ziwei Liu commented on CALCITE-4964:


The original code is 
{code:java}
@Override public RexNode visitCall(RexCall call) {
  for (;;) {
call = (RexCall) super.visitCall(call);
final RexCall old = call;
call = pushPredicateIntoCase(call);
if (call == old) {
  return call;
}
  }
} {code}
If call != old, then new call will continue the depth-first traversal, and this 
traversal may not need to do.

eg:

select case 

           when value >300 and id > 100 then 1

           when value > 200 and id > 50 then 2

           else 3 as type

from src where type  = 3;

Then Filter will be EQ(CASE(...)).

After push EQ into CASE, this condition does not need to push EQ into inner 
predicate, so just need traverse the new call's operand.

If the when's predicate is more complicated than this example, the traversal 
will save more time.

 

> Reduce recursion when push predicate into CASE
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Updated] (CALCITE-4964) Reduce recursion when push predicate into CASE

2021-12-27 Thread Ziwei Liu (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4964?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ziwei Liu updated CALCITE-4964:
---
Summary: Reduce recursion when push predicate into CASE  (was: reduce 
recursion when push predicate into case)

> Reduce recursion when push predicate into CASE
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Updated] (CALCITE-4967) Support SQL hints for temporal table join

2021-12-27 Thread Jing Zhang (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4967?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jing Zhang updated CALCITE-4967:

Description: 
h1. Background
Like hint on join, sometimes we need hint on temporal table join. For example, 
specify USE_HASH hint on temporal table join like the following sql,

{code:java}
select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
for system_time as of orders.rowtime on orders.productid = 
products_temporal.productid
{code}

Now, above sql would translate into correlate instead of Join, like the 
following Tree Node.

{code:java}
LogicalDelta
  LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
LogicalFilter(condition=[=($1, $3)])
  LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
requiredColumns=[{0}])
LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
LogicalSnapshot(period=[$cor0.ROWTIME])
  LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
{code}

However, correlate is not a hintable node now and the hint could not be 
propagated to the correlate nodes. 
So I wonder could we extend correlate to be a hintable node?



  was:
h1. Background
Like hint on join, sometimes we need hint on temporal table join. For example, 
specify USE_HASH hint on temporal table join like the following sql,

{code:java}
select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
for system_time as of orders.rowtime on orders.productid = 
products_temporal.productid
{code}

Now, above sql would translate into correlate instead of Join, like the 
following Tree Node.

{code:java}
LogicalDelta
  LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
LogicalFilter(condition=[=($1, $3)])
  LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
requiredColumns=[{0}])
LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
LogicalSnapshot(period=[$cor0.ROWTIME])
  LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
{code}

However, correlate is not a hintable node now. So I wonder could we extend 
correlate to be a hintable node?




> Support SQL hints for temporal table join
> -
>
> Key: CALCITE-4967
> URL: https://issues.apache.org/jira/browse/CALCITE-4967
> Project: Calcite
>  Issue Type: New Feature
>  Components: core
>Reporter: Jing Zhang
>Priority: Major
>
> h1. Background
> Like hint on join, sometimes we need hint on temporal table join. For 
> example, specify USE_HASH hint on temporal table join like the following sql,
> {code:java}
> select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
> for system_time as of orders.rowtime on orders.productid = 
> products_temporal.productid
> {code}
> Now, above sql would translate into correlate instead of Join, like the 
> following Tree Node.
> {code:java}
> LogicalDelta
>   LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
> NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
> LogicalFilter(condition=[=($1, $3)])
>   LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
> requiredColumns=[{0}])
> LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
> LogicalSnapshot(period=[$cor0.ROWTIME])
>   LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
> {code}
> However, correlate is not a hintable node now and the hint could not be 
> propagated to the correlate nodes. 
> So I wonder could we extend correlate to be a hintable node?



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4967) Support SQL hints for temporal table join

2021-12-27 Thread Danny Chen (Jira)


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

Danny Chen commented on CALCITE-4967:
-

+1 for the idea. We can made the correlate node hintable though.

> Support SQL hints for temporal table join
> -
>
> Key: CALCITE-4967
> URL: https://issues.apache.org/jira/browse/CALCITE-4967
> Project: Calcite
>  Issue Type: New Feature
>  Components: core
>Reporter: Jing Zhang
>Priority: Major
>
> h1. Background
> Like hint on join, sometimes we need hint on temporal table join. For 
> example, specify USE_HASH hint on temporal table join like the following sql,
> {code:java}
> select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
> for system_time as of orders.rowtime on orders.productid = 
> products_temporal.productid
> {code}
> Now, above sql would translate into correlate instead of Join, like the 
> following Tree Node.
> {code:java}
> LogicalDelta
>   LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
> NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
> LogicalFilter(condition=[=($1, $3)])
>   LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
> requiredColumns=[{0}])
> LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
> LogicalSnapshot(period=[$cor0.ROWTIME])
>   LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
> {code}
> However, correlate is not a hintable node now. So I wonder could we extend 
> correlate to be a hintable node?



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4967) Support SQL hints for temporal table join

2021-12-27 Thread Jing Zhang (Jira)


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

Jing Zhang commented on CALCITE-4967:
-

[~danny0405] Hi, danny, what do you think on this requirement?

> Support SQL hints for temporal table join
> -
>
> Key: CALCITE-4967
> URL: https://issues.apache.org/jira/browse/CALCITE-4967
> Project: Calcite
>  Issue Type: New Feature
>  Components: core
>Reporter: Jing Zhang
>Priority: Major
>
> h1. Background
> Like hint on join, sometimes we need hint on temporal table join. For 
> example, specify USE_HASH hint on temporal table join like the following sql,
> {code:java}
> select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
> for system_time as of orders.rowtime on orders.productid = 
> products_temporal.productid
> {code}
> Now, above sql would translate into correlate instead of Join, like the 
> following Tree Node.
> {code:java}
> LogicalDelta
>   LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
> NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
> LogicalFilter(condition=[=($1, $3)])
>   LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
> requiredColumns=[{0}])
> LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
> LogicalSnapshot(period=[$cor0.ROWTIME])
>   LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
> {code}
> However, correlate is not a hintable node now. So I wonder could we extend 
> correlate to be a hintable node?



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Created] (CALCITE-4967) Support SQL hints for temporal table join

2021-12-27 Thread Jing Zhang (Jira)
Jing Zhang created CALCITE-4967:
---

 Summary: Support SQL hints for temporal table join
 Key: CALCITE-4967
 URL: https://issues.apache.org/jira/browse/CALCITE-4967
 Project: Calcite
  Issue Type: New Feature
  Components: core
Reporter: Jing Zhang


h1. Background
Like hint on join, sometimes we need hint on temporal table join. For example, 
specify USE_HASH hint on temporal table join like the following sql,

{code:java}
select stream /*+ USE_HASH (orders) */ * from orders join products_temporal
for system_time as of orders.rowtime on orders.productid = 
products_temporal.productid
{code}

Now, above sql would translate into correlate instead of Join, like the 
following Tree Node.

{code:java}
LogicalDelta
  LogicalProject(ROWTIME=[$0], PRODUCTID=[$1], ORDERID=[$2], PRODUCTID0=[$3], 
NAME=[$4], SUPPLIERID=[$5], SYS_START=[$6], SYS_END=[$7])
LogicalFilter(condition=[=($1, $3)])
  LogicalCorrelate(correlation=[$cor0], joinType=[inner], 
requiredColumns=[{0}])
LogicalTableScan(table=[[CATALOG, SALES, ORDERS]])
LogicalSnapshot(period=[$cor0.ROWTIME])
  LogicalTableScan(table=[[CATALOG, SALES, PRODUCTS_TEMPORAL]])
{code}

However, correlate is not a hintable node now. So I wonder could we extend 
correlate to be a hintable node?





--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Commented] (CALCITE-4964) reduce recursion when push predicate into case

2021-12-27 Thread Julian Hyde (Jira)


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

Julian Hyde commented on CALCITE-4964:
--

Summary should start with a capital letter ("Reduce") and SQL keywords should 
be capitalized ("CASE").

Your version of that method is 3x as long as the original. Does the performance 
improvement justify the complexity?

> reduce recursion when push predicate into case
> --
>
> Key: CALCITE-4964
> URL: https://issues.apache.org/jira/browse/CALCITE-4964
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Ziwei Liu
>Assignee: Ziwei Liu
>Priority: Major
>
> I think CaseShuttle do not need a loop of for(;; ).When a predicate is pushed 
> into case, we only need to check the new RexCall whether need to push 
> predicate into the child operand, not need to do a new recursion.
> So I think CaseShuttle can be reduced like:
> {code:java}
> @Override public RexNode visitCall(RexCall call) {
>    call = (RexCall) super.visitCall(call);
>    if (call instanceof OdpsLambdaRexCall) {
>       return call;
>    }
>    final RexCall old = call;
>    call = ReduceExpressionsRule.pushPredicateIntoCase(call);
>    if (call == old) {
>      return call;
>    } else {
>       boolean containCase = false;
>       List newOps = new ArrayList<>(call.getOperands().size());
>       // call will be like case when c1 then f(x1 ...)
>       // check whether need push f into x1
>       for (int i = 0; i < call.getOperands().size(); i ++) {
>          RexNode op = call.getOperands().get(i);
>          RexNode newOp = op;
>          if (i % 2 == 1 || i == call.getOperands().size() - 1) {
>            if (op instanceof RexCall) {
>              newOp = ReduceExpressionsRule.pushPredicateIntoCase((RexCall) 
> op);
>            }
>          }
>          if (op != newOp) {
>            containCase = true;
>          }
>          newOps.add(newOp);
>        }
>        if (!containCase) {
>          return call;
>        } else {
>          return call.clone(call.getType(), newOps);
>        }
>    }    
> } {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Updated] (CALCITE-4864) Supports Polymorphic Table function

2021-12-27 Thread Jing Zhang (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4864?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jing Zhang updated CALCITE-4864:

Issue Type: New Feature  (was: Bug)

> Supports Polymorphic Table function
> ---
>
> Key: CALCITE-4864
> URL: https://issues.apache.org/jira/browse/CALCITE-4864
> Project: Calcite
>  Issue Type: New Feature
>  Components: core
>Reporter: Jing Zhang
>Priority: Major
>
> Being the part of ANSI SQL 2016 Polymorphic Table Functions are the evolution 
> of the table functions. They can be invoked in the FROM clause.  They have 
> the following features compare with normal table function:
>  # may have generic table parameters (i.e., no row type of table parameters 
> declared when the PTF is created) 
>  # returns a table whose row type is not declared when the function is 
> created, it may depend on the function arguments.
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Updated] (CALCITE-4957) Add new RelDistribution.Type to solve data skew caused by normal Hash Distribution type

2021-12-27 Thread Jing Zhang (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4957?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jing Zhang updated CALCITE-4957:

Issue Type: New Feature  (was: Improvement)

> Add new RelDistribution.Type to solve data skew caused by normal Hash 
> Distribution type
> ---
>
> Key: CALCITE-4957
> URL: https://issues.apache.org/jira/browse/CALCITE-4957
> Project: Calcite
>  Issue Type: New Feature
>  Components: core
>Reporter: Jing Zhang
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> I hope to extend `RelDistribution` to support more distribution types in 
> order to solve data skew in the normal hash distribution.
> When we use hash distribution to bring all records with the same hash key to 
> the same place, the job performance would be poor if there exists hot keys. 
> There is a solution to solve this problem, we could send a hot key to one of 
> serval downstream tasks, chosen at random.
> In HashJoin, we could use random hash partition in one side, for the other 
> input to the join, records relating to the hot key need to be replicated to 
> all downstream tasks handling that key.
> In HashAggregate, we could split the aggregate into partial-final if all the 
> aggregation functions support splitting.
> The 10th chapter in the book "Designing Data Intensive Applications" also 
> refers this solution to solve data skew.
> Anyway, we should first extend `RelDistribution` to support more distribution 
> types, for example, hash random type. 
> For example, there is a [lookup 
> join|https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/joins/#lookup-join]
>  in Flink. is typically used to enrich a table with data that is queried from 
> an external system. 
> For the following query
> {code:sql}
> select p.x, p.y, b.z, b.pt_year, b.pt_mon, b.pt_day 
> from default_catalog.default_database.probe as p
> join partition_table_3 for system_time as of p.proc_time as b
> on p.x=b.x and p.y=b.y
> {code}
> When use normal hash distribution.
> The logical plan is as following,
> {code:java}
>+- LookupJoin(joinType=[InnerJoin], lookup=[x=x, y=y], select=[x, y, x0, 
> y0, z, pt_year, pt_mon, pt_day])
>   +- Exchange(distribution=[hash[x, y]])
>  +- TableSourceScan(table=[[default_catalog, default_database, probe, 
> source: [CollectionTableSource(x, y)]]], fields=[x, y])
> {code}
> If enable data_skew solution in hint, the logical plan is as following,
> {code:java}
>+- LookupJoin(joinType=[InnerJoin], lookup=[x=x, y=y], select=[x, y, x0, 
> y0, z, pt_year, pt_mon, pt_day])
>   +- Exchange(distribution=[hash_random(key=[x, y], bucket_num=8)])
>  +- TableSourceScan(table=[[default_catalog, default_database, probe, 
> source: [CollectionTableSource(x, y)]]], fields=[x, y])
> {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Closed] (CALCITE-4547) Support Java 16 and 17

2021-12-27 Thread Julian Hyde (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Hyde closed CALCITE-4547.


Resolved in release 1.29.0 (2021-12-26)

> Support Java 16 and 17
> --
>
> Key: CALCITE-4547
> URL: https://issues.apache.org/jira/browse/CALCITE-4547
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.29.0
>
>  Time Spent: 1h
>  Remaining Estimate: 0h
>
> Support building and running on [Java 16|https://jdk.java.net/16/] (OpenJDK 
> 16) and [Java 17|https://jdk.java.net/17/](OpenJDK 17).
> This will require an [upgrade to Gradle 
> 7|https://melix.github.io/blog/2021/03/gradle-java16.html], because Gradle 6 
> will not support Java 16.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Resolved] (CALCITE-4547) Support Java 16 and 17

2021-12-27 Thread Julian Hyde (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-4547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Hyde resolved CALCITE-4547.
--
Resolution: Fixed

Fixed in 
[f79624a0|https://github.com/apache/calcite/commit/f79624a07feabee16376b519283cf375b76b8b9f];
 thanks for the PR, [~VAE]!

Although the above commit came after release 1.29, this bug is marked as fixed 
in release 1.29. The necessary code change to enable Java 16 and 17 was fixed 
in 
[6d51d277|https://github.com/apache/calcite/commit/6d51d277b158ff7073f29ada4a96a7a74c0b46fc],
 which occurred before release 1.29.

> Support Java 16 and 17
> --
>
> Key: CALCITE-4547
> URL: https://issues.apache.org/jira/browse/CALCITE-4547
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.29.0
>
>  Time Spent: 1h
>  Remaining Estimate: 0h
>
> Support building and running on [Java 16|https://jdk.java.net/16/] (OpenJDK 
> 16) and [Java 17|https://jdk.java.net/17/](OpenJDK 17).
> This will require an [upgrade to Gradle 
> 7|https://melix.github.io/blog/2021/03/gradle-java16.html], because Gradle 6 
> will not support Java 16.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)