Hi Timo,

I think it's a good idea to use `SqlParser#parseExpression()` to parse
literals.
That means the string format of literal is SQL compatible.
After some discussion with Kurt, we think why not one more step forward,
i.e. convert the whole expression to SQL format.

For example, the above expression will be converted to:

`cat`.`db`.`func`(`cat`.`db`.`f0`, TIMESTAMP '2019-10-21 12:12:12')

There are some benefits from this:
0) the string representation is more readable, and can be manually typed
more easily.
1) the string format is SQL syntax, not customized, which means it can be
integrated by third party projects.
2) we can reuse Calcite's SqlParser to parse string and SqlNode#unparse to
generate string, this can avoid introducing duplicate code and a custom
parser.
3) no compatible problems.

Regarding to how Expression can be converted into a SQL string, I think we
can leverage some Calcite utils:

ResolvedExpression ---(ExpressionConverter)---> RexNode
----(RexToSqlNodeConverter)---> SqlNode --> SqlNode#unparse()

What do you think?

Best,
Jark

On Mon, 21 Oct 2019 at 22:08, Timo Walther <twal...@apache.org> wrote:

> Hi Jark,
>
> thanks for the proposal. This is a great effort to finalize the new API
> design.
>
> I'm wondering if we could simply use the SQL parser like
> `org.apache.calcite.sql.parser.SqlParser#parseExpression(..)` to parse
> an expression that contain only literals. This would avoid any
> discussion as the syntax is already defined by the SQL standard. And it
> would also be very unlikely to have a need for a version.
>
> For example:
>
> CALL('FUNC', FIELD('f0'), VALUE('TIMESTAMP(3)', TIMESTAMP '2019-10-21
> 12:12:12'))
>
> Or even further if the SQL parser allows that:
>
> CALL('FUNC', `cat`.`db`.`f0`, TIMESTAMP '2019-10-21 12:12:12')
>
> I would find it confusing if we use different representation for
> literals such as intervals and timestamps in the properties. This would
> also reduce code duplication as we reuse logic for parsing identifiers etc.
>
> What do you think?
>
> Regards,
> Timo
>
>
> On 18.10.19 12:28, Jark Wu wrote:
> > Hi everyone,
> >
> > I would like to start a discussion[1] about how to make Expression string
> > serializable and deserializable. Expression is the general interface for
> > all kinds of expressions in Flink Table API & SQL, it represents a
> logical
> > tree for producing a computation result. In FLIP-66[2] and FLIP-70[3], we
> > introduced watermark and computed column syntax in DDL. The watermark
> > strategy and computed column are both represented in Expression. In order
> > to persist watermark and computed column information in catalog, we need
> to
> > figure out how to persist and restore Expression.
> >
> > FLIP-80:
> >
> https://docs.google.com/document/d/1LxPEzbPuEVWNixb1L_USv0gFgjRMgoZuMsAecS_XvdE/edit?usp=sharing
> >
> > Thanks for any feedback!
> >
> > Best,
> > Jark
> >
> > [1]:
> >
> https://docs.google.com/document/d/1LxPEzbPuEVWNixb1L_USv0gFgjRMgoZuMsAecS_XvdE/edit?usp=sharing
> > [2]:
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-66%3A+Support+time+attribute+in+SQL+DDL
> > [3]:
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-70%3A+Flink+SQL+Computed+Column+Design
> >
>
>

Reply via email to