[DISCUSS] Simplify SQL lookup join (temporal join with latest) syntax

[Gyula Fóra]

Hi All!

Playing around with the SQL syntax for temporal join with latest table I
feel there is some room for optimizing the current syntax to provide a
better user experience.

The current system for specifying the lookup side is:

lookuptable FOR SYSTEM_TIME AS OF probe.proctime_column

It feels a bit clumsy to have to define a proctime() column in the probe
table as I think it brings no real syntactic value and just introduces an
overhead.

I think we should allow the following syntax instead:

lookuptable FOR SYSTEM_TIME AS OF proctime()

To me this means the same thing and Flink can easily map it to the same
lookup join operator. Playing around with the planner logic, this is
surprisingly simple to implement (basicly just a 2 line change).

It would be good to hear some SQL expert opinions of any potential downside
to this. If this makes sense I am happy to contribute this change.

Cheers,
Gyula

[Jingsong Li]

+1 for simplifying.

We already have a thread of this topic:
http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Make-Temporal-Join-syntax-easier-to-use-td47296.html
FYI.

Best,
Jingsong

On Tue, Apr 20, 2021 at 4:55 PM Gyula Fóra <[hidden email]> wrote:

> Hi All!
>
> Playing around with the SQL syntax for temporal join with latest table I
> feel there is some room for optimizing the current syntax to provide a
> better user experience.
>
> The current system for specifying the lookup side is:
>
> lookuptable FOR SYSTEM_TIME AS OF probe.proctime_column
>
> It feels a bit clumsy to have to define a proctime() column in the probe
> table as I think it brings no real syntactic value and just introduces an
> overhead.
>
> I think we should allow the following syntax instead:
>
> lookuptable FOR SYSTEM_TIME AS OF proctime()
>
> To me this means the same thing and Flink can easily map it to the same
> lookup join operator. Playing around with the planner logic, this is
> surprisingly simple to implement (basicly just a 2 line change).
>
> It would be good to hear some SQL expert opinions of any potential downside
> to this. If this makes sense I am happy to contribute this change.
>
> Cheers,
> Gyula
>

--
Best, Jingsong Lee

[Gyula Fóra]

Thanks for the pointer Jingsong,

I don't see how proctime() is ambiguous though as it always refers to the
current wall clock time. I think thats much better than adding a magic
pseudocolumn.

Cheers
Gyula

On Tue, Apr 20, 2021 at 11:06 AM Jingsong Li <[hidden email]> wrote:

> +1 for simplifying.
>
> We already have a thread of this topic:
>
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Make-Temporal-Join-syntax-easier-to-use-td47296.html
> FYI.
>
> Best,
> Jingsong
>
> On Tue, Apr 20, 2021 at 4:55 PM Gyula Fóra <[hidden email]> wrote:
>
> > Hi All!
> >
> > Playing around with the SQL syntax for temporal join with latest table I
> > feel there is some room for optimizing the current syntax to provide a
> > better user experience.
> >
> > The current system for specifying the lookup side is:
> >
> > lookuptable FOR SYSTEM_TIME AS OF probe.proctime_column
> >
> > It feels a bit clumsy to have to define a proctime() column in the probe
> > table as I think it brings no real syntactic value and just introduces an
> > overhead.
> >
> > I think we should allow the following syntax instead:
> >
> > lookuptable FOR SYSTEM_TIME AS OF proctime()
> >
> > To me this means the same thing and Flink can easily map it to the same
> > lookup join operator. Playing around with the planner logic, this is
> > surprisingly simple to implement (basicly just a 2 line change).
> >
> > It would be good to hear some SQL expert opinions of any potential
> downside
> > to this. If this makes sense I am happy to contribute this change.
> >
> > Cheers,
> > Gyula
> >
>
>
> --
> Best, Jingsong Lee
>