Thanks Kurt and Godfrey for the explanation, It makes sense to me that renaming `from(tableName)` to `fromCatalog(tableName)`. However, I still think `sqlQuery(query)` is clear and works well. Is it necessary to change it?
We removed `sql(query)` and introduced `sqlQuery(query)`, we removed `scan(tableName)` and introduced `from(tableName)`, and now we want to remove them again. Users will feel like the interface is very unstable, that really frustrates users. I think we should be cautious to remove interface and only when it is necessary. Best, Jark On Thu, 13 Feb 2020 at 20:58, godfrey he <godfre...@gmail.com> wrote: > hi kurt,jark,jingsong > > Regarding to "fromQuery", I agree with kurt. In addition, I think `Table > from(String tableName)` should be renamed to `Table fromCatalog(String > tableName)`. > > Regarding to the "DmlBatch", DML contains "INSERT", "UPDATE", "DELETE", and > they can be executed in a same batch in the future. So we can add > "addUpdate" method and "addDelete" method to support them. > > Regarding to the "Inserts addInsert", maybe we can add a "DmlBatchBuilder". > > open to more discussion > > Best, > godfrey > > > > Kurt Young <ykt...@gmail.com> 于2020年2月13日周四 下午4:56写道: > > > Regarding to "fromQuery" is confusing users with "Table from(String > > tableName)", I have > > a just opposite opinion. I think this "fromXXX" pattern can make users > > quite clear when they > > want to get a Table from TableEnvironment. Similar interfaces will also > > include like "fromElements". > > > > Regarding to the name of DmlBatch, I think it's mainly for > > future flexibility, in case we can support > > other statement in a single batch. If that happens, the name "Inserts" > will > > be weird. > > > > Best, > > Kurt > > > > > > On Thu, Feb 13, 2020 at 4:03 PM Jark Wu <imj...@gmail.com> wrote: > > > > > I agree with Jingsong. > > > > > > +1 to keep `sqlQuery`, it's clear from the method name and return type > > that > > > it accepts a SELECT query and returns a logic representation `Table`. > > > The `fromQuery` is a little confused users with the `Table from(String > > > tableName)` method. > > > > > > Regarding to the `DmlBatch`, I agree with Jingsong, AFAIK, the purpose > of > > > `DmlBatch` is used to batching insert statements. > > > Besides, DML terminology is not commonly know among users. So what > about > > > `InsertsBatching startBatchingInserts()` ? > > > > > > Best, > > > Jark > > > > > > On Thu, 13 Feb 2020 at 15:50, Jingsong Li <jingsongl...@gmail.com> > > wrote: > > > > > > > Hi Godfrey, > > > > > > > > Thanks for updating. +1 sketchy. > > > > > > > > I have no idea to change "sqlQuery" to "fromQuery", I think > "sqlQuery" > > is > > > > OK, It's not that confusing with return values. > > > > > > > > Can we change the "DmlBatch" to "Inserts"? I don't see any other > > needs. > > > > "Dml" seems a little weird. > > > > It is better to support "Inserts addInsert" too. Users can > > > > "inserts.addInsert().addInsert()...." > > > > > > > > I try to match the new interfaces with the old interfaces simply. > > > > - "startInserts -> addInsert" replace old "sqlUpdate(insert)" and > > > > "insertInto". > > > > - "executeStatement" new one, execute all kinds of sqls immediately. > > > > Including old "sqlUpdate(DDLs)". > > > > > > > > Best, > > > > Jingsong Lee > > > > > > > > On Wed, Feb 12, 2020 at 11:10 AM godfreyhe <godfre...@gmail.com> > > wrote: > > > > > > > > > Hi everyone, > > > > > > > > > > I'd like to resume the discussion for FlIP-84 [0]. I had updated > the > > > > > document, the mainly changes are: > > > > > > > > > > 1. about "`void sqlUpdate(String sql)`" section > > > > > a) change "Optional<ResultTable> executeSql(String sql) throws > > > > Exception" > > > > > to "ResultTable executeStatement(String statement, String jobName) > > > throws > > > > > Exception". The reason is: "statement" is a more general concept > than > > > > > "sql", > > > > > e.g. "show xx" is not a sql command (refer to [1]), but is a > > statement > > > > > (just > > > > > like JDBC). "insert" statement also has return value which is the > > > > affected > > > > > row count, we can unify the return type to "ResultTable" instead of > > > > > "Optional<ResultTable>". > > > > > b) add two sub-interfaces for "ResultTable": "RowResultTable" is > > used > > > > for > > > > > non-streaming select statement and will not contain change flag; > > > > > "RowWithChangeFlagResultTable" is used for streaming select > statement > > > and > > > > > will contain change flag. > > > > > > > > > > 2) about "Support batch sql execute and explain" section > > > > > introduce "DmlBatch" to support both sql and Table API (which is > > > borrowed > > > > > from the ideas Dawid mentioned in the slack) > > > > > > > > > > interface TableEnvironment { > > > > > DmlBatch startDmlBatch(); > > > > > } > > > > > > > > > > interface DmlBatch { > > > > > /** > > > > > * add insert statement to the batch > > > > > */ > > > > > void addInsert(String insert); > > > > > > > > > > /** > > > > > * add Table with given sink name to the batch > > > > > */ > > > > > void addInsert(String sinkName, Table table); > > > > > > > > > > /** > > > > > * execute the dml statements as a batch > > > > > */ > > > > > ResultTable execute(String jobName) throws Exception > > > > > > > > > > /** > > > > > * Returns the AST and the execution plan to compute the result of > > the > > > > > batch > > > > > dml statement. > > > > > */ > > > > > String explain(boolean extended); > > > > > } > > > > > > > > > > 3) about "Discuss a parse method for multiple statements execute in > > SQL > > > > > CLI" > > > > > section > > > > > add the pros and cons for each solution > > > > > > > > > > 4) update the "Examples" section and "Summary" section based on the > > > above > > > > > changes > > > > > > > > > > Please refer the design doc[1] for more details and welcome any > > > feedback. > > > > > > > > > > Bests, > > > > > godfreyhe > > > > > > > > > > > > > > > [0] > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/19-mdYJjKirh5aXCwq1fDajSaI09BJMMT95wy_YhtuZk/edit > > > > > [1] > https://www.geeksforgeeks.org/sql-ddl-dql-dml-dcl-tcl-commands/ > > > > > > > > > > > > > > > > > > > > -- > > > > > Sent from: > > > > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/ > > > > > > > > > > > > > > > > > -- > > > > Best, Jingsong Lee > > > > > > > > > >