You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by JiaTao Tao <ta...@apache.org> on 2020/10/09 07:27:53 UTC
TableScan#explainTerms may be not enough for digest
Hi fellows
Here's the problem:
We have our own RelOptTable implementation, and we replace
the RelOptTableImpl during a rule in HepPlanner. Something unexpected
occurred: We only replaced the `table` in TableScan, and the digest is not
changed after the rule, so in `HepPlanner#addRelToGraph`, directly return
the ori TableScan.
HepPlanner#addRelToGraph
```
Pair<String, List<RelDataType>> key = key(rel);
HepRelVertex equivVertex = mapDigestToVertex.get(key);
if (equivVertex != null) {
// Use existing vertex.
return equivVertex;
}
```
Regards!
Aron Tao
Re: TableScan#explainTerms may be not enough for digest
Posted by Chunwei Lei <ch...@gmail.com>.
I agree with Danny.
In our system, we also have to override the RelOptTable#getQualifiedName to
get more information
except for the table name.
Best,
Chunwei
On Sat, Oct 10, 2020 at 10:37 AM JiaTao Tao <ta...@gmail.com> wrote:
> Hi Haishen
> See this:
> HepPlanner#addRelToGraph
>
> Pair<String, List<RelDataType>> key = key(rel);
> HepRelVertex equivVertex = mapDigestToVertex.get(key);
> if (equivVertex != null) {
> // Use existing vertex.
> return equivVertex;
> }
>
>
> We only update the TableScan#table in TableScan, so the digest does not
> change, so it returns the original TableScan, as I said "the transformation
> is not applied".
>
>
> Regards!
>
> Aron Tao
>
>
> Haisheng Yuan <hy...@apache.org> 于2020年10月10日周六 上午3:58写道：
>
> > > The problem bothering me is that the transformation is not effective in
> > HepPlanner due to this.
> >
> > Aron, what do you mean by "the transformation is not effective"?
> >
> > Haisheng
> >
> > On 2020/10/09 09:49:27, JiaTao Tao <ta...@apache.org> wrote:
> > > Hi Danny,
> > > Does this info mainly for planner use? If this we can add an interface
> > > mainly for the planner.
> > >
> > > The problem bothering me is that the transformation is not effective in
> > > HepPlanner due to this.
> > >
> > >
> > > Regards!
> > >
> > > Aron Tao
> > >
> > >
> > > JiaTao Tao <ta...@apache.org> 于2020年10月9日周五 下午5:46写道：
> > >
> > > >
> > > > Hi Danny
> > > > So maybe it is better to add an interface for rel opt table to
> provide
> > > > extra digest and consider this in TableScan#explainTerms, but it
> > changes
> > > > the base output of TableScan, seems a quite big change.
> > > >
> > > >
> > > > Regards!
> > > >
> > > > Aron Tao
> > > >
> > > >
> > > > Danny Chan <yu...@gmail.com> 于2020年10月9日周五 下午5:29写道：
> > > >
> > > >> Thanks for driving this discussion, JiaTao, the Flink way:
> > > >>
> > > >> Extend the digest through getQualifiedName is hacky somehow.
> > > >>
> > > >> We actually need some interfaces to reflect/represent the pushed
> info,
> > > >> the pushed fields or filter expression.
> > > >>
> > > >> Best,
> > > >> Danny Chan
> > > >> 在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
> > > >> >
> > > >> > getQualifiedName
> > > >>
> > > >
> > >
> >
>
Re: TableScan#explainTerms may be not enough for digest
Posted by JiaTao Tao <ta...@gmail.com>.
Hi Haishen
See this:
HepPlanner#addRelToGraph
Pair<String, List<RelDataType>> key = key(rel);
HepRelVertex equivVertex = mapDigestToVertex.get(key);
if (equivVertex != null) {
// Use existing vertex.
return equivVertex;
}
We only update the TableScan#table in TableScan, so the digest does not
change, so it returns the original TableScan, as I said "the transformation
is not applied".
Regards!
Aron Tao
Haisheng Yuan <hy...@apache.org> 于2020年10月10日周六 上午3:58写道：
> > The problem bothering me is that the transformation is not effective in
> HepPlanner due to this.
>
> Aron, what do you mean by "the transformation is not effective"?
>
> Haisheng
>
> On 2020/10/09 09:49:27, JiaTao Tao <ta...@apache.org> wrote:
> > Hi Danny,
> > Does this info mainly for planner use? If this we can add an interface
> > mainly for the planner.
> >
> > The problem bothering me is that the transformation is not effective in
> > HepPlanner due to this.
> >
> >
> > Regards!
> >
> > Aron Tao
> >
> >
> > JiaTao Tao <ta...@apache.org> 于2020年10月9日周五 下午5:46写道：
> >
> > >
> > > Hi Danny
> > > So maybe it is better to add an interface for rel opt table to provide
> > > extra digest and consider this in TableScan#explainTerms, but it
> changes
> > > the base output of TableScan, seems a quite big change.
> > >
> > >
> > > Regards!
> > >
> > > Aron Tao
> > >
> > >
> > > Danny Chan <yu...@gmail.com> 于2020年10月9日周五 下午5:29写道：
> > >
> > >> Thanks for driving this discussion, JiaTao, the Flink way:
> > >>
> > >> Extend the digest through getQualifiedName is hacky somehow.
> > >>
> > >> We actually need some interfaces to reflect/represent the pushed info,
> > >> the pushed fields or filter expression.
> > >>
> > >> Best,
> > >> Danny Chan
> > >> 在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
> > >> >
> > >> > getQualifiedName
> > >>
> > >
> >
>
Re: TableScan#explainTerms may be not enough for digest
Posted by Haisheng Yuan <hy...@apache.org>.
> The problem bothering me is that the transformation is not effective in
HepPlanner due to this.
Aron, what do you mean by "the transformation is not effective"?
Haisheng
On 2020/10/09 09:49:27, JiaTao Tao <ta...@apache.org> wrote:
> Hi Danny,
> Does this info mainly for planner use? If this we can add an interface
> mainly for the planner.
>
> The problem bothering me is that the transformation is not effective in
> HepPlanner due to this.
>
>
> Regards!
>
> Aron Tao
>
>
> JiaTao Tao <ta...@apache.org> 于2020年10月9日周五 下午5:46写道：
>
> >
> > Hi Danny
> > So maybe it is better to add an interface for rel opt table to provide
> > extra digest and consider this in TableScan#explainTerms, but it changes
> > the base output of TableScan, seems a quite big change.
> >
> >
> > Regards!
> >
> > Aron Tao
> >
> >
> > Danny Chan <yu...@gmail.com> 于2020年10月9日周五 下午5:29写道：
> >
> >> Thanks for driving this discussion, JiaTao, the Flink way:
> >>
> >> Extend the digest through getQualifiedName is hacky somehow.
> >>
> >> We actually need some interfaces to reflect/represent the pushed info,
> >> the pushed fields or filter expression.
> >>
> >> Best,
> >> Danny Chan
> >> 在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
> >> >
> >> > getQualifiedName
> >>
> >
>
Re: TableScan#explainTerms may be not enough for digest
Posted by JiaTao Tao <ta...@apache.org>.
Hi Danny,
Does this info mainly for planner use? If this we can add an interface
mainly for the planner.
The problem bothering me is that the transformation is not effective in
HepPlanner due to this.
Regards!
Aron Tao
JiaTao Tao <ta...@apache.org> 于2020年10月9日周五 下午5:46写道：
>
> Hi Danny
> So maybe it is better to add an interface for rel opt table to provide
> extra digest and consider this in TableScan#explainTerms, but it changes
> the base output of TableScan, seems a quite big change.
>
>
> Regards!
>
> Aron Tao
>
>
> Danny Chan <yu...@gmail.com> 于2020年10月9日周五 下午5:29写道：
>
>> Thanks for driving this discussion, JiaTao, the Flink way:
>>
>> Extend the digest through getQualifiedName is hacky somehow.
>>
>> We actually need some interfaces to reflect/represent the pushed info,
>> the pushed fields or filter expression.
>>
>> Best,
>> Danny Chan
>> 在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
>> >
>> > getQualifiedName
>>
>
Re: TableScan#explainTerms may be not enough for digest
Posted by JiaTao Tao <ta...@apache.org>.
Hi Danny
So maybe it is better to add an interface for rel opt table to provide
extra digest and consider this in TableScan#explainTerms, but it changes
the base output of TableScan, seems a quite big change.
Regards!
Aron Tao
Danny Chan <yu...@gmail.com> 于2020年10月9日周五 下午5:29写道：
> Thanks for driving this discussion, JiaTao, the Flink way:
>
> Extend the digest through getQualifiedName is hacky somehow.
>
> We actually need some interfaces to reflect/represent the pushed info, the
> pushed fields or filter expression.
>
> Best,
> Danny Chan
> 在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
> >
> > getQualifiedName
>
Re: TableScan#explainTerms may be not enough for digest
Posted by Danny Chan <yu...@gmail.com>.
Thanks for driving this discussion, JiaTao, the Flink way:
Extend the digest through getQualifiedName is hacky somehow.
We actually need some interfaces to reflect/represent the pushed info, the pushed fields or filter expression.
Best,
Danny Chan
在 2020年10月9日 +0800 PM3:35，dev@calcite.apache.org，写道：
>
> getQualifiedName
Re: TableScan#explainTerms may be not enough for digest
Posted by JiaTao Tao <ta...@gmail.com>.
I think Flink meets the same problem, but the solution is not so elegant:
In org.apache.flink.table.planner.plan.rules.logical.
*PushPartitionIntoTableSourceScanRule*, they create a new LogicalTableScan
but only with new RelOptTable, :
TableSourceTable newTableSourceTable =
tableSourceTable.copy(dynamicTableSource, newStatistic, new
String[]{extraDigest});LogicalTableScan newScan =
LogicalTableScan.create(scan.getCluster(), newTableSourceTable,
scan.getHints());
In their RelOptTable(TableSourceTable), they inject extra info
getQualifiedName, so the TableScan's digest is diff to the first:
override def getQualifiedName: util.List[String] = {
val names = super.getQualifiedName
extraDigests.foreach(builder.add)
builder.build()
}
But this way, there's dirty info when we call getQualifiedName.
Regards!
Aron Tao
JiaTao Tao <ta...@apache.org> 于2020年10月9日周五 下午3:27写道：
> Hi fellows
> Here's the problem:
> We have our own RelOptTable implementation, and we replace
> the RelOptTableImpl during a rule in HepPlanner. Something unexpected
> occurred: We only replaced the `table` in TableScan, and the digest is not
> changed after the rule, so in `HepPlanner#addRelToGraph`, directly return
> the ori TableScan.
>
> HepPlanner#addRelToGraph
> ```
> Pair<String, List<RelDataType>> key = key(rel);
> HepRelVertex equivVertex = mapDigestToVertex.get(key);
> if (equivVertex != null) {
> // Use existing vertex.
> return equivVertex;
> }
> ```
>
>
> Regards!
>
> Aron Tao
>