Re: [DISCUSS] Sql-client lack support for new features

[Kurt Young <yk...@gmail.com>]

Thanks Jingsong for bringing this up discussion and sorry for the late
reply.

I'm in general +1 for #1, and want to expand the scope of #2.

First of all, I think the approach Jingsong proposed in #2 can help with
covering more
e2e use cases of SQL, which also draws a clean line between how to design
IT cases for
table API and SQL.

This is a good intent to have, but what I want to point out is it's still
not sufficient yet. I think
the ultimate problem we are facing is *designing a testing principle for
table & SQL*. I will
share some of my observations and issues I've seen:

1. The lack of SQL client's functionality is definitely the first victim of
this issue. During our
development, I don't know how many developers will take SQL client into
consideration when
they introduce new features, especially which need to be exposed through
SQL client.

2. We have a very fuzzy boundary of what kind of tests should be written in
table API or in SQL.
So we end up with two testing packages called "sql" and "table" for the
same testing purpose, with
*random tests created in each package*.

3. We don't have a guideline for developers, especially for new
contributors, about how many tests
they should write and where to put. For example, when we improve the data
type support, say we
start to support higher precision of Timestamp, we don't have a clue how
many tests we should provide.
Should we add some tests for the code generation part? should we add some
tests for the integration test?
Or is it already enough to test it with simple expression tests? We don't
have a clean answer for it.
As a result, we will be busy fixing bugs around new features we introduced,
even if the bug itself looks
very simple which *will surprise our users by the bad quality of Flink SQL
engine*.

So I think what we really need here (which is also very urgent IMO) is
*designing
some testing principles*
*for table API & SQL*. Writing SQL's IT case through sql client might be
one of them. If we can keep
improving the testing principles we have, I believe we will have a much
more reliable SQL engine in the
future, which can attract more users to the Flink community.

Best,
Kurt


On Thu, Jun 18, 2020 at 5:14 PM Jark Wu <im...@gmail.com> wrote:

> +1 for #1
> I think this is what we are currently doing, that forward SQL statements to
> TableEnv#executeSql, e.g. FLINK-17113, FLINK-18059.
> But IMO the SQL CLI specific statements (EXIT, QUIT) should still stay only
> in SQL CLI.
>
> Another idea is that, the reviewer/committer should check tests are both
> added for SQL CLI and TableEnv if it is a new statement.
>
> Best,
> Jark
>
> On Thu, 18 Jun 2020 at 15:40, Rui Li <li...@gmail.com> wrote:
>
> > Thanks Jingsong for bringing up the discussion. Perhaps we can do both #1
> > and #2? I like the idea that SQL Client should just forward SQL
> statements
> > to TableEnvironment. IIUC, with TableEnvironment::executeSql in place,
> most
> > statements can be forwarded. Only a very limited set of statements need
> to
> > be handled by SQL Client, like SET and EXIT.
> >
> > On Thu, Jun 18, 2020 at 3:19 PM Jingsong Li <ji...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I'd like to start a discussion for the new features lacking support of
> > > Sql-client.
> > >
> > > I've seen the new DDL syntax that SQL client lacks support for many
> > times.
> > > For every new DDL syntax, we need to add support in sql-client. Add
> > > a corresponding SqlCommand in sql-client, otherwise this DDL is still
> > > not working in sql-client.
> > >
> > > But it looks like developers always forgot to add support in
> sql-client.
> > > Lots of DDL features just be added in parser and planner, but lack
> > > sql-client support, so users will wait for the next release. Just like:
> > > https://issues.apache.org/jira/browse/FLINK-7151
> > > https://issues.apache.org/jira/browse/FLINK-17198
> > > https://issues.apache.org/jira/browse/FLINK-15468
> > > https://issues.apache.org/jira/browse/FLINK-15175
> > >
> > > How to solve this?
> > > I think we have two options:
> > >
> > > 1. Unify the parser in sql-client and TableEnvironment truly. Really
> make
> > > sql-client have all abilities from TableEnvironment. sql-client just
> > > forward sql to TableEnvironment. Can it be?
> > > 2. A new testing framework mechanism: We can make sql-related tests
> more
> > > "up front", we can move e2e tests (I think we are doing now) and it
> cases
> > > to sql-client oriented. This may require a new testing framework
> > mechanism,
> > > for example, we can do something like hive sql testing [1] to
> > > sql-client oriented. In this way, the testing can cover more horizontal
> > and
> > > vertical and it is easy to migrate tests from other systems too. And I
> > > think, Flink's DDLs are enough stronger to support pure SQLs testing.
> > >
> > > What do you think?
> > >
> > > [1]https://github.com/apache/hive/tree/master/ql/src/test/queries
> > >
> > > Best,
> > > Jingsong
> > >
> >
> >
> > --
> > Best regards!
> > Rui Li
> >
>

Re: [DISCUSS] Sql-client lack support for new features

["DONG, Weike" <ky...@connect.hku.hk>]

Thanks Jinsong for the suggestions.

+1 for #1, as users are constantly puzzled by the inconsistencies between
the Table / SQL module and SQL Client.

 Best,
Weike

On Mon, Jul 6, 2020 at 11:55 AM Jingsong Li <ji...@gmail.com> wrote:

> +1 for expanding the scope of #2, a SQL test framework(Should not be too
> heavy but clear) and guidelines for developers.
>
> Best,
> Jingsong
>
> On Thu, Jul 2, 2020 at 12:30 PM Kurt Young <yk...@gmail.com> wrote:
>
> > Thanks Jingsong for bringing this up discussion and sorry for the late
> > reply.
> >
> > I'm in general +1 for #1, and want to expand the scope of #2.
> >
> > First of all, I think the approach Jingsong proposed in #2 can help with
> > covering more
> > e2e use cases of SQL, which also draws a clean line between how to design
> > IT cases for
> > table API and SQL.
> >
> > This is a good intent to have, but what I want to point out is it's still
> > not sufficient yet. I think
> > the ultimate problem we are facing is *designing a testing principle for
> > table & SQL*. I will
> > share some of my observations and issues I've seen:
> >
> > 1. The lack of SQL client's functionality is definitely the first victim
> of
> > this issue. During our
> > development, I don't know how many developers will take SQL client into
> > consideration when
> > they introduce new features, especially which need to be exposed through
> > SQL client.
> >
> > 2. We have a very fuzzy boundary of what kind of tests should be written
> in
> > table API or in SQL.
> > So we end up with two testing packages called "sql" and "table" for the
> > same testing purpose, with
> > *random tests created in each package*.
> >
> > 3. We don't have a guideline for developers, especially for new
> > contributors, about how many tests
> > they should write and where to put. For example, when we improve the data
> > type support, say we
> > start to support higher precision of Timestamp, we don't have a clue how
> > many tests we should provide.
> > Should we add some tests for the code generation part? should we add some
> > tests for the integration test?
> > Or is it already enough to test it with simple expression tests? We don't
> > have a clean answer for it.
> > As a result, we will be busy fixing bugs around new features we
> introduced,
> > even if the bug itself looks
> > very simple which *will surprise our users by the bad quality of Flink
> SQL
> > engine*.
> >
> > So I think what we really need here (which is also very urgent IMO) is
> > *designing
> > some testing principles*
> > *for table API & SQL*. Writing SQL's IT case through sql client might be
> > one of them. If we can keep
> > improving the testing principles we have, I believe we will have a much
> > more reliable SQL engine in the
> > future, which can attract more users to the Flink community.
> >
> > Best,
> > Kurt
> >
> >
> > On Thu, Jun 18, 2020 at 5:14 PM Jark Wu <im...@gmail.com> wrote:
> >
> > > +1 for #1
> > > I think this is what we are currently doing, that forward SQL
> statements
> > to
> > > TableEnv#executeSql, e.g. FLINK-17113, FLINK-18059.
> > > But IMO the SQL CLI specific statements (EXIT, QUIT) should still stay
> > only
> > > in SQL CLI.
> > >
> > > Another idea is that, the reviewer/committer should check tests are
> both
> > > added for SQL CLI and TableEnv if it is a new statement.
> > >
> > > Best,
> > > Jark
> > >
> > > On Thu, 18 Jun 2020 at 15:40, Rui Li <li...@gmail.com> wrote:
> > >
> > > > Thanks Jingsong for bringing up the discussion. Perhaps we can do
> both
> > #1
> > > > and #2? I like the idea that SQL Client should just forward SQL
> > > statements
> > > > to TableEnvironment. IIUC, with TableEnvironment::executeSql in
> place,
> > > most
> > > > statements can be forwarded. Only a very limited set of statements
> need
> > > to
> > > > be handled by SQL Client, like SET and EXIT.
> > > >
> > > > On Thu, Jun 18, 2020 at 3:19 PM Jingsong Li <ji...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I'd like to start a discussion for the new features lacking support
> > of
> > > > > Sql-client.
> > > > >
> > > > > I've seen the new DDL syntax that SQL client lacks support for many
> > > > times.
> > > > > For every new DDL syntax, we need to add support in sql-client. Add
> > > > > a corresponding SqlCommand in sql-client, otherwise this DDL is
> still
> > > > > not working in sql-client.
> > > > >
> > > > > But it looks like developers always forgot to add support in
> > > sql-client.
> > > > > Lots of DDL features just be added in parser and planner, but lack
> > > > > sql-client support, so users will wait for the next release. Just
> > like:
> > > > > https://issues.apache.org/jira/browse/FLINK-7151
> > > > > https://issues.apache.org/jira/browse/FLINK-17198
> > > > > https://issues.apache.org/jira/browse/FLINK-15468
> > > > > https://issues.apache.org/jira/browse/FLINK-15175
> > > > >
> > > > > How to solve this?
> > > > > I think we have two options:
> > > > >
> > > > > 1. Unify the parser in sql-client and TableEnvironment truly.
> Really
> > > make
> > > > > sql-client have all abilities from TableEnvironment. sql-client
> just
> > > > > forward sql to TableEnvironment. Can it be?
> > > > > 2. A new testing framework mechanism: We can make sql-related tests
> > > more
> > > > > "up front", we can move e2e tests (I think we are doing now) and it
> > > cases
> > > > > to sql-client oriented. This may require a new testing framework
> > > > mechanism,
> > > > > for example, we can do something like hive sql testing [1] to
> > > > > sql-client oriented. In this way, the testing can cover more
> > horizontal
> > > > and
> > > > > vertical and it is easy to migrate tests from other systems too.
> And
> > I
> > > > > think, Flink's DDLs are enough stronger to support pure SQLs
> testing.
> > > > >
> > > > > What do you think?
> > > > >
> > > > > [1]https://github.com/apache/hive/tree/master/ql/src/test/queries
> > > > >
> > > > > Best,
> > > > > Jingsong
> > > > >
> > > >
> > > >
> > > > --
> > > > Best regards!
> > > > Rui Li
> > > >
> > >
> >
>
>
> --
> Best, Jingsong Lee
>

Re: [DISCUSS] Sql-client lack support for new features

[Jingsong Li <ji...@gmail.com>]

+1 for expanding the scope of #2, a SQL test framework(Should not be too
heavy but clear) and guidelines for developers.

Best,
Jingsong

On Thu, Jul 2, 2020 at 12:30 PM Kurt Young <yk...@gmail.com> wrote:

> Thanks Jingsong for bringing this up discussion and sorry for the late
> reply.
>
> I'm in general +1 for #1, and want to expand the scope of #2.
>
> First of all, I think the approach Jingsong proposed in #2 can help with
> covering more
> e2e use cases of SQL, which also draws a clean line between how to design
> IT cases for
> table API and SQL.
>
> This is a good intent to have, but what I want to point out is it's still
> not sufficient yet. I think
> the ultimate problem we are facing is *designing a testing principle for
> table & SQL*. I will
> share some of my observations and issues I've seen:
>
> 1. The lack of SQL client's functionality is definitely the first victim of
> this issue. During our
> development, I don't know how many developers will take SQL client into
> consideration when
> they introduce new features, especially which need to be exposed through
> SQL client.
>
> 2. We have a very fuzzy boundary of what kind of tests should be written in
> table API or in SQL.
> So we end up with two testing packages called "sql" and "table" for the
> same testing purpose, with
> *random tests created in each package*.
>
> 3. We don't have a guideline for developers, especially for new
> contributors, about how many tests
> they should write and where to put. For example, when we improve the data
> type support, say we
> start to support higher precision of Timestamp, we don't have a clue how
> many tests we should provide.
> Should we add some tests for the code generation part? should we add some
> tests for the integration test?
> Or is it already enough to test it with simple expression tests? We don't
> have a clean answer for it.
> As a result, we will be busy fixing bugs around new features we introduced,
> even if the bug itself looks
> very simple which *will surprise our users by the bad quality of Flink SQL
> engine*.
>
> So I think what we really need here (which is also very urgent IMO) is
> *designing
> some testing principles*
> *for table API & SQL*. Writing SQL's IT case through sql client might be
> one of them. If we can keep
> improving the testing principles we have, I believe we will have a much
> more reliable SQL engine in the
> future, which can attract more users to the Flink community.
>
> Best,
> Kurt
>
>
> On Thu, Jun 18, 2020 at 5:14 PM Jark Wu <im...@gmail.com> wrote:
>
> > +1 for #1
> > I think this is what we are currently doing, that forward SQL statements
> to
> > TableEnv#executeSql, e.g. FLINK-17113, FLINK-18059.
> > But IMO the SQL CLI specific statements (EXIT, QUIT) should still stay
> only
> > in SQL CLI.
> >
> > Another idea is that, the reviewer/committer should check tests are both
> > added for SQL CLI and TableEnv if it is a new statement.
> >
> > Best,
> > Jark
> >
> > On Thu, 18 Jun 2020 at 15:40, Rui Li <li...@gmail.com> wrote:
> >
> > > Thanks Jingsong for bringing up the discussion. Perhaps we can do both
> #1
> > > and #2? I like the idea that SQL Client should just forward SQL
> > statements
> > > to TableEnvironment. IIUC, with TableEnvironment::executeSql in place,
> > most
> > > statements can be forwarded. Only a very limited set of statements need
> > to
> > > be handled by SQL Client, like SET and EXIT.
> > >
> > > On Thu, Jun 18, 2020 at 3:19 PM Jingsong Li <ji...@gmail.com>
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I'd like to start a discussion for the new features lacking support
> of
> > > > Sql-client.
> > > >
> > > > I've seen the new DDL syntax that SQL client lacks support for many
> > > times.
> > > > For every new DDL syntax, we need to add support in sql-client. Add
> > > > a corresponding SqlCommand in sql-client, otherwise this DDL is still
> > > > not working in sql-client.
> > > >
> > > > But it looks like developers always forgot to add support in
> > sql-client.
> > > > Lots of DDL features just be added in parser and planner, but lack
> > > > sql-client support, so users will wait for the next release. Just
> like:
> > > > https://issues.apache.org/jira/browse/FLINK-7151
> > > > https://issues.apache.org/jira/browse/FLINK-17198
> > > > https://issues.apache.org/jira/browse/FLINK-15468
> > > > https://issues.apache.org/jira/browse/FLINK-15175
> > > >
> > > > How to solve this?
> > > > I think we have two options:
> > > >
> > > > 1. Unify the parser in sql-client and TableEnvironment truly. Really
> > make
> > > > sql-client have all abilities from TableEnvironment. sql-client just
> > > > forward sql to TableEnvironment. Can it be?
> > > > 2. A new testing framework mechanism: We can make sql-related tests
> > more
> > > > "up front", we can move e2e tests (I think we are doing now) and it
> > cases
> > > > to sql-client oriented. This may require a new testing framework
> > > mechanism,
> > > > for example, we can do something like hive sql testing [1] to
> > > > sql-client oriented. In this way, the testing can cover more
> horizontal
> > > and
> > > > vertical and it is easy to migrate tests from other systems too. And
> I
> > > > think, Flink's DDLs are enough stronger to support pure SQLs testing.
> > > >
> > > > What do you think?
> > > >
> > > > [1]https://github.com/apache/hive/tree/master/ql/src/test/queries
> > > >
> > > > Best,
> > > > Jingsong
> > > >
> > >
> > >
> > > --
> > > Best regards!
> > > Rui Li
> > >
> >
>


-- 
Best, Jingsong Lee