You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Jinfeng Ni <jn...@apache.org> on 2016/01/08 02:40:20 UTC

Assertion check in RelOptTableImpl.create()

Does anyone know why one of the static create() methods in
RelOptTableImpl has the following assertion check (to check table is
instance of TranslatableTable, or ScannableTable, or ModifiableTable)
[1], while the rest of create() methods do not do such check? [2]

Looks like RelOptTableImpl.toRel() actually expects table instance
other than the above three class[3].

Does it makes sense to remove the assertion check in [1]?

Best Regards,

Jinfeng


[1]. https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169

[2] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118

[3] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225

Re: Assertion check in RelOptTableImpl.create()

Posted by Jinfeng Ni <ji...@gmail.com>.
Sounds good. Thanks!


On Sun, Jan 10, 2016 at 5:58 PM, Julian Hyde <jh...@apache.org> wrote:
> I don’t think it would do any harm if DrillTable implements ScannableTable. The method could throw UnsupportedOperationException if you don’t intend to use it.
>
>> On Jan 8, 2016, at 9:58 PM, Jinfeng Ni <ji...@gmail.com> wrote:
>>
>> Thanks for the explanation, Julian.
>>
>> The places where I want to create an instance of RelOptTable is not in
>> SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
>> CalciteSchema or Path is not available, which makes it not possible to
>> call the other create() methods. In that sense, such table is "free
>> floating" table.
>>
>> The RelOptRule, which essentially is trying to pushing partition
>> filter into scan, has to create a new instance of RelOptTable, after
>> certain transformation. In that sense, it's quite similar to
>> DeltaTableScanRule in Calcite code base [1].  The difference is
>> DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
>> does not implement ScannableTable [2].
>>
>> Do you think it makes sense to make DrillTable implement ScannableTable?
>>
>> Thanks!
>>
>> [1] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
>> [2] https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34
>>
>> On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
>>> Tables created using “create(RelOptSchema, RelDataType, Table)” have their names field set to the empty list. That is, they don’t know their location within the schema hierarchy and in fact may not be in the schema hierarchy. Usually a table implements itself by generating code like
>>>
>>>  rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
>>>
>>> But such “free floating” tables cannot implement themselves in that way. Therefore this method is only for kinds of tables that know how to get to their own data: TranslatableTable, ModifiableTable, ScannableTable.
>>>
>>> Julian
>>>
>>>
>>>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
>>>>
>>>> Does anyone know why one of the static create() methods in
>>>> RelOptTableImpl has the following assertion check (to check table is
>>>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
>>>> [1], while the rest of create() methods do not do such check? [2]
>>>>
>>>> Looks like RelOptTableImpl.toRel() actually expects table instance
>>>> other than the above three class[3].
>>>>
>>>> Does it makes sense to remove the assertion check in [1]?
>>>>
>>>> Best Regards,
>>>>
>>>> Jinfeng
>>>>
>>>>
>>>> [1]. https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
>>>>
>>>> [2] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
>>>>
>>>> [3] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
>>>
>

Re: Assertion check in RelOptTableImpl.create()

Posted by Julian Hyde <jh...@apache.org>.
ScannableTable etc. are not “marker” interfaces — I was just trying to be flexible. That method is intended for tables that are able to implement themselves. I’m not prepared to change the intent of that method, so the assert needs to stay.

Why don’t you implement TranslatableTable? All that requires is a toRel method.


> On Jan 11, 2016, at 10:23 AM, Jinfeng Ni <ji...@gmail.com> wrote:
> 
> Whether Table implements
> ScannableTable/QuerableTable/TranslatableTable would have impact of
> RelOptTableImpl.toRel() method.
> toRel() method will rely on that to return LogicalScan, or
> EnumerableTableScan etc.
> 
> Although throw UnsupportedException would solve my current problem,
> I'm not clear why those "free floating" tables have to implement the
> interface, and the regular tables do not have to.  If there is no
> other reason, I kindly agree with Jacques that we better remove the
> assertion.
> 
> 
> 
> On Sun, Jan 10, 2016 at 7:47 PM, Jacques Nadeau <ja...@apache.org> wrote:
>> It seems better to remove the assertion. If those are marker interfaces
>> that have more meaning, I think we should avoid forcing people to implement
>> one of those interfaces and then not support the implied functionality.
>> Hadoop had a similiar problem when they added ByteBuffer read apis marker
>> interface and then added some implementations that had the marker interface
>> but fail to implement the appropriate functionality. This means people can
>> no longer rely on the marker interface to ensure that functionality is
>> available. For those that are interested, this is the kind of hoops the
>> Parquet library had to jump through because of this problem [1].
>> 
>> [1]
>> https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/CompatibilityUtil.java#L79
>> 
>> 
>> On Sun, Jan 10, 2016 at 5:58 PM, Julian Hyde <jh...@apache.org> wrote:
>> 
>>> I don’t think it would do any harm if DrillTable implements
>>> ScannableTable. The method could throw UnsupportedOperationException if you
>>> don’t intend to use it.
>>> 
>>>> On Jan 8, 2016, at 9:58 PM, Jinfeng Ni <ji...@gmail.com> wrote:
>>>> 
>>>> Thanks for the explanation, Julian.
>>>> 
>>>> The places where I want to create an instance of RelOptTable is not in
>>>> SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
>>>> CalciteSchema or Path is not available, which makes it not possible to
>>>> call the other create() methods. In that sense, such table is "free
>>>> floating" table.
>>>> 
>>>> The RelOptRule, which essentially is trying to pushing partition
>>>> filter into scan, has to create a new instance of RelOptTable, after
>>>> certain transformation. In that sense, it's quite similar to
>>>> DeltaTableScanRule in Calcite code base [1].  The difference is
>>>> DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
>>>> does not implement ScannableTable [2].
>>>> 
>>>> Do you think it makes sense to make DrillTable implement ScannableTable?
>>>> 
>>>> Thanks!
>>>> 
>>>> [1]
>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
>>>> [2]
>>> https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34
>>>> 
>>>> On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
>>>>> Tables created using “create(RelOptSchema, RelDataType, Table)” have
>>> their names field set to the empty list. That is, they don’t know their
>>> location within the schema hierarchy and in fact may not be in the schema
>>> hierarchy. Usually a table implements itself by generating code like
>>>>> 
>>>>> rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
>>>>> 
>>>>> But such “free floating” tables cannot implement themselves in that
>>> way. Therefore this method is only for kinds of tables that know how to get
>>> to their own data: TranslatableTable, ModifiableTable, ScannableTable.
>>>>> 
>>>>> Julian
>>>>> 
>>>>> 
>>>>>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
>>>>>> 
>>>>>> Does anyone know why one of the static create() methods in
>>>>>> RelOptTableImpl has the following assertion check (to check table is
>>>>>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
>>>>>> [1], while the rest of create() methods do not do such check? [2]
>>>>>> 
>>>>>> Looks like RelOptTableImpl.toRel() actually expects table instance
>>>>>> other than the above three class[3].
>>>>>> 
>>>>>> Does it makes sense to remove the assertion check in [1]?
>>>>>> 
>>>>>> Best Regards,
>>>>>> 
>>>>>> Jinfeng
>>>>>> 
>>>>>> 
>>>>>> [1].
>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
>>>>>> 
>>>>>> [2]
>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
>>>>>> 
>>>>>> [3]
>>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
>>>>> 
>>> 
>>> 


Re: Assertion check in RelOptTableImpl.create()

Posted by Jinfeng Ni <ji...@gmail.com>.
Whether Table implements
ScannableTable/QuerableTable/TranslatableTable would have impact of
RelOptTableImpl.toRel() method.
toRel() method will rely on that to return LogicalScan, or
EnumerableTableScan etc.

Although throw UnsupportedException would solve my current problem,
I'm not clear why those "free floating" tables have to implement the
interface, and the regular tables do not have to.  If there is no
other reason, I kindly agree with Jacques that we better remove the
assertion.



On Sun, Jan 10, 2016 at 7:47 PM, Jacques Nadeau <ja...@apache.org> wrote:
> It seems better to remove the assertion. If those are marker interfaces
> that have more meaning, I think we should avoid forcing people to implement
> one of those interfaces and then not support the implied functionality.
> Hadoop had a similiar problem when they added ByteBuffer read apis marker
> interface and then added some implementations that had the marker interface
> but fail to implement the appropriate functionality. This means people can
> no longer rely on the marker interface to ensure that functionality is
> available. For those that are interested, this is the kind of hoops the
> Parquet library had to jump through because of this problem [1].
>
> [1]
> https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/CompatibilityUtil.java#L79
>
>
> On Sun, Jan 10, 2016 at 5:58 PM, Julian Hyde <jh...@apache.org> wrote:
>
>> I don’t think it would do any harm if DrillTable implements
>> ScannableTable. The method could throw UnsupportedOperationException if you
>> don’t intend to use it.
>>
>> > On Jan 8, 2016, at 9:58 PM, Jinfeng Ni <ji...@gmail.com> wrote:
>> >
>> > Thanks for the explanation, Julian.
>> >
>> > The places where I want to create an instance of RelOptTable is not in
>> > SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
>> > CalciteSchema or Path is not available, which makes it not possible to
>> > call the other create() methods. In that sense, such table is "free
>> > floating" table.
>> >
>> > The RelOptRule, which essentially is trying to pushing partition
>> > filter into scan, has to create a new instance of RelOptTable, after
>> > certain transformation. In that sense, it's quite similar to
>> > DeltaTableScanRule in Calcite code base [1].  The difference is
>> > DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
>> > does not implement ScannableTable [2].
>> >
>> > Do you think it makes sense to make DrillTable implement ScannableTable?
>> >
>> > Thanks!
>> >
>> > [1]
>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
>> > [2]
>> https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34
>> >
>> > On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
>> >> Tables created using “create(RelOptSchema, RelDataType, Table)” have
>> their names field set to the empty list. That is, they don’t know their
>> location within the schema hierarchy and in fact may not be in the schema
>> hierarchy. Usually a table implements itself by generating code like
>> >>
>> >>  rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
>> >>
>> >> But such “free floating” tables cannot implement themselves in that
>> way. Therefore this method is only for kinds of tables that know how to get
>> to their own data: TranslatableTable, ModifiableTable, ScannableTable.
>> >>
>> >> Julian
>> >>
>> >>
>> >>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
>> >>>
>> >>> Does anyone know why one of the static create() methods in
>> >>> RelOptTableImpl has the following assertion check (to check table is
>> >>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
>> >>> [1], while the rest of create() methods do not do such check? [2]
>> >>>
>> >>> Looks like RelOptTableImpl.toRel() actually expects table instance
>> >>> other than the above three class[3].
>> >>>
>> >>> Does it makes sense to remove the assertion check in [1]?
>> >>>
>> >>> Best Regards,
>> >>>
>> >>> Jinfeng
>> >>>
>> >>>
>> >>> [1].
>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
>> >>>
>> >>> [2]
>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
>> >>>
>> >>> [3]
>> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
>> >>
>>
>>

Re: Assertion check in RelOptTableImpl.create()

Posted by Jacques Nadeau <ja...@apache.org>.
It seems better to remove the assertion. If those are marker interfaces
that have more meaning, I think we should avoid forcing people to implement
one of those interfaces and then not support the implied functionality.
Hadoop had a similiar problem when they added ByteBuffer read apis marker
interface and then added some implementations that had the marker interface
but fail to implement the appropriate functionality. This means people can
no longer rely on the marker interface to ensure that functionality is
available. For those that are interested, this is the kind of hoops the
Parquet library had to jump through because of this problem [1].

[1]
https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/CompatibilityUtil.java#L79


On Sun, Jan 10, 2016 at 5:58 PM, Julian Hyde <jh...@apache.org> wrote:

> I don’t think it would do any harm if DrillTable implements
> ScannableTable. The method could throw UnsupportedOperationException if you
> don’t intend to use it.
>
> > On Jan 8, 2016, at 9:58 PM, Jinfeng Ni <ji...@gmail.com> wrote:
> >
> > Thanks for the explanation, Julian.
> >
> > The places where I want to create an instance of RelOptTable is not in
> > SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
> > CalciteSchema or Path is not available, which makes it not possible to
> > call the other create() methods. In that sense, such table is "free
> > floating" table.
> >
> > The RelOptRule, which essentially is trying to pushing partition
> > filter into scan, has to create a new instance of RelOptTable, after
> > certain transformation. In that sense, it's quite similar to
> > DeltaTableScanRule in Calcite code base [1].  The difference is
> > DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
> > does not implement ScannableTable [2].
> >
> > Do you think it makes sense to make DrillTable implement ScannableTable?
> >
> > Thanks!
> >
> > [1]
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
> > [2]
> https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34
> >
> > On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
> >> Tables created using “create(RelOptSchema, RelDataType, Table)” have
> their names field set to the empty list. That is, they don’t know their
> location within the schema hierarchy and in fact may not be in the schema
> hierarchy. Usually a table implements itself by generating code like
> >>
> >>  rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
> >>
> >> But such “free floating” tables cannot implement themselves in that
> way. Therefore this method is only for kinds of tables that know how to get
> to their own data: TranslatableTable, ModifiableTable, ScannableTable.
> >>
> >> Julian
> >>
> >>
> >>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
> >>>
> >>> Does anyone know why one of the static create() methods in
> >>> RelOptTableImpl has the following assertion check (to check table is
> >>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
> >>> [1], while the rest of create() methods do not do such check? [2]
> >>>
> >>> Looks like RelOptTableImpl.toRel() actually expects table instance
> >>> other than the above three class[3].
> >>>
> >>> Does it makes sense to remove the assertion check in [1]?
> >>>
> >>> Best Regards,
> >>>
> >>> Jinfeng
> >>>
> >>>
> >>> [1].
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
> >>>
> >>> [2]
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
> >>>
> >>> [3]
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
> >>
>
>

Re: Assertion check in RelOptTableImpl.create()

Posted by Julian Hyde <jh...@apache.org>.
I don’t think it would do any harm if DrillTable implements ScannableTable. The method could throw UnsupportedOperationException if you don’t intend to use it.

> On Jan 8, 2016, at 9:58 PM, Jinfeng Ni <ji...@gmail.com> wrote:
> 
> Thanks for the explanation, Julian.
> 
> The places where I want to create an instance of RelOptTable is not in
> SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
> CalciteSchema or Path is not available, which makes it not possible to
> call the other create() methods. In that sense, such table is "free
> floating" table.
> 
> The RelOptRule, which essentially is trying to pushing partition
> filter into scan, has to create a new instance of RelOptTable, after
> certain transformation. In that sense, it's quite similar to
> DeltaTableScanRule in Calcite code base [1].  The difference is
> DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
> does not implement ScannableTable [2].
> 
> Do you think it makes sense to make DrillTable implement ScannableTable?
> 
> Thanks!
> 
> [1] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
> [2] https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34
> 
> On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
>> Tables created using “create(RelOptSchema, RelDataType, Table)” have their names field set to the empty list. That is, they don’t know their location within the schema hierarchy and in fact may not be in the schema hierarchy. Usually a table implements itself by generating code like
>> 
>>  rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
>> 
>> But such “free floating” tables cannot implement themselves in that way. Therefore this method is only for kinds of tables that know how to get to their own data: TranslatableTable, ModifiableTable, ScannableTable.
>> 
>> Julian
>> 
>> 
>>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
>>> 
>>> Does anyone know why one of the static create() methods in
>>> RelOptTableImpl has the following assertion check (to check table is
>>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
>>> [1], while the rest of create() methods do not do such check? [2]
>>> 
>>> Looks like RelOptTableImpl.toRel() actually expects table instance
>>> other than the above three class[3].
>>> 
>>> Does it makes sense to remove the assertion check in [1]?
>>> 
>>> Best Regards,
>>> 
>>> Jinfeng
>>> 
>>> 
>>> [1]. https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
>>> 
>>> [2] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
>>> 
>>> [3] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
>> 


Re: Assertion check in RelOptTableImpl.create()

Posted by Jinfeng Ni <ji...@gmail.com>.
Thanks for the explanation, Julian.

The places where I want to create an instance of RelOptTable is not in
SqlValidator or SqlToRelConverter. Rather, it's in a RelOptRule, where
CalciteSchema or Path is not available, which makes it not possible to
call the other create() methods. In that sense, such table is "free
floating" table.

The RelOptRule, which essentially is trying to pushing partition
filter into scan, has to create a new instance of RelOptTable, after
certain transformation. In that sense, it's quite similar to
DeltaTableScanRule in Calcite code base [1].  The difference is
DeltaTableScanRule has a ScannableTable, while in Drill, DrillTable
does not implement ScannableTable [2].

Do you think it makes sense to make DrillTable implement ScannableTable?

Thanks!

[1] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/stream/StreamRules.java#L192-L195
[2] https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillTable.java#L34

On Fri, Jan 8, 2016 at 5:24 PM, Julian Hyde <jh...@apache.org> wrote:
> Tables created using “create(RelOptSchema, RelDataType, Table)” have their names field set to the empty list. That is, they don’t know their location within the schema hierarchy and in fact may not be in the schema hierarchy. Usually a table implements itself by generating code like
>
>   rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)
>
> But such “free floating” tables cannot implement themselves in that way. Therefore this method is only for kinds of tables that know how to get to their own data: TranslatableTable, ModifiableTable, ScannableTable.
>
> Julian
>
>
>> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
>>
>> Does anyone know why one of the static create() methods in
>> RelOptTableImpl has the following assertion check (to check table is
>> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
>> [1], while the rest of create() methods do not do such check? [2]
>>
>> Looks like RelOptTableImpl.toRel() actually expects table instance
>> other than the above three class[3].
>>
>> Does it makes sense to remove the assertion check in [1]?
>>
>> Best Regards,
>>
>> Jinfeng
>>
>>
>> [1]. https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
>>
>> [2] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
>>
>> [3] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225
>

Re: Assertion check in RelOptTableImpl.create()

Posted by Julian Hyde <jh...@apache.org>.
Tables created using “create(RelOptSchema, RelDataType, Table)” have their names field set to the empty list. That is, they don’t know their location within the schema hierarchy and in fact may not be in the schema hierarchy. Usually a table implements itself by generating code like

  rootSchema.getSubSchema(“FOODMART”).getTable(“CUSTOMERS”)

But such “free floating” tables cannot implement themselves in that way. Therefore this method is only for kinds of tables that know how to get to their own data: TranslatableTable, ModifiableTable, ScannableTable.

Julian


> On Jan 7, 2016, at 5:40 PM, Jinfeng Ni <jn...@apache.org> wrote:
> 
> Does anyone know why one of the static create() methods in
> RelOptTableImpl has the following assertion check (to check table is
> instance of TranslatableTable, or ScannableTable, or ModifiableTable)
> [1], while the rest of create() methods do not do such check? [2]
> 
> Looks like RelOptTableImpl.toRel() actually expects table instance
> other than the above three class[3].
> 
> Does it makes sense to remove the assertion check in [1]?
> 
> Best Regards,
> 
> Jinfeng
> 
> 
> [1]. https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L167-L169
> 
> [2] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L118
> 
> [3] https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/RelOptTableImpl.java#L225