Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-07 Thread Zhenghua Gao
Thanks Timo! Look forward your design! *Best Regards,* *Zhenghua Gao* On Fri, Feb 7, 2020 at 5:26 PM Timo Walther wrote: > Hi Zhenghua, > > Jark is right. The reason why we haven't updated those interfaces yet is > because we are actually would like to introduce new interfaces. We > should

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-07 Thread Jark Wu
Cool! Looking forward to the design doc. Best, Jark On Fri, 7 Feb 2020 at 17:26, Timo Walther wrote: > Hi Zhenghua, > > Jark is right. The reason why we haven't updated those interfaces yet is > because we are actually would like to introduce new interfaces. We > should target new interfaces

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-07 Thread Timo Walther
Hi Zhenghua, Jark is right. The reason why we haven't updated those interfaces yet is because we are actually would like to introduce new interfaces. We should target new interfaces in this release. Even a short-term fix as you proposed with `getRecordDataType` does actually not help as

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Zhenghua Gao
Hi Jark, thanks for your comments. >>>IIUC, the framework will only recognize getRecordDataType and >>>ignore getConsumedDataType for UpsertStreamTableSink, is that right? Your are right. >>>getRecordDataType is little confused as UpsertStreamTableSink already has >>>three getXXXType(). the

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Zhenghua Gao
Should we distinguish *record data type* and *consumed data type*? Currently the design of UpsertStreamTableSink and RetractStreamTableSink DO distinguish them. In my proposal the framework will ignore *getConsumedDataType*, so it's ok to use *getConsumedDataType* to do the job if we don't

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Jark Wu
Thanks Zhenghua for starting this discussion. Currently, all the UpsertStreamTableSinks can't upgrade to the new type system which affects usability a lot. I hope we can fix that in 1.11. I'm find with *getRecordDataType* for a temporary solution. IIUC, the framework will only recognize

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Zhenghua Gao
Hi Jingsong, For now, only UpsertStreamTableSink and RetractStreamTableSink consumes JTuple2 So the 'getConsumedDataType' interface is not necessary in validate & codegen phase. See https://github.com/apache/flink/pull/10880/files#diff-137d090bd719f3a99ae8ba6603ed81ebR52 and

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Jingsong Li
Hi Zhenghua, The *getRecordDataType* looks good to me. But the main problem is how to represent the tuple type in DataType. I understand that it is necessary to use StructuredType, but at present, planner does not support StructuredType, so the other way is to support StructuredType. Best,

Re: [DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-03 Thread Kurt Young
Would overriding `getConsumedDataType` do the job? Best, Kurt On Mon, Feb 3, 2020 at 3:52 PM Zhenghua Gao wrote: > Hi all, > > FLINK-12254[1] [2] updated TableSink and related interfaces to new type > system which > allows connectors use the new type system based on DataTypes. > > But

[DISCUSS] Update UpsertStreamTableSink and RetractStreamTableSink to new type system

2020-02-02 Thread Zhenghua Gao
Hi all, FLINK-12254[1] [2] updated TableSink and related interfaces to new type system which allows connectors use the new type system based on DataTypes. But FLINK-12911 port UpsertStreamTableSink and RetractStreamTableSink to flink-api-java-bridge and returns TypeInformation of the requested