+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 <ykt...@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 <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 > > > > > > -- Best, Jingsong Lee
