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 <imj...@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 <lirui.fu...@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 <jingsongl...@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 > > >
