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
>

Reply via email to