You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by godfreyhe <go...@gmail.com> on 2020/02/12 03:10:03 UTC

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

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/

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Benchao Li <li...@gmail.com>.
hi godfrey,

Thanks for your information, I didn't see that part before.
Now the FLIP LGTM, thanks for driving this effort.

godfrey he <go...@gmail.com> 于2020年3月2日周一 上午9:51写道:

> Hi Benchao,
>
> I think the document has contained both parts: the behavior is explained
> when introducing `executeStatement` method, and asynchronous execution
> methods is explained in the appendix.
>
> Bests,
> Godfrey
>
> Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午10:09写道:
>
> > Hi godfrey,
> >
> > Thanks for your explanation.
> >
> > Do we need to clarify this in the FLIP? Maybe this confuses other users
> as
> > well.
> >
> > godfrey he <go...@gmail.com> 于2020年2月28日周五 下午4:54写道:
> >
> > > Hi Benchao,
> > >
> > > > I have one question about this FLIP:
> > > > executeStatement  accepts DML, what if it's a streaming DML ?
> > > >    does it submit the job to cluster directly and blocks forever?
> > what's
> > > > the behavior for the next statements?
> > > `executeStatement` is a synchronous method, will execute the statement
> > once
> > > calling this method and return the result until the job is finished.
> > > We will introduce asynchronous method like `executeStatementAsync` in
> the
> > > future.
> > >
> > > > nit: there's a typo in "the table describing the result for each kind
> > of
> > > > statement", "*Result Scheam" -> "Result Schema"*
> > > Thanks for the reminding, I will fix it now.
> > >
> > > Bests,
> > > Godfrey
> > >
> > > Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午4:00写道:
> > >
> > > > Hi Terry,
> > > >
> > > > Thanks for the propose, and sorry for joining the party late.
> > > >
> > > > I have one question about this FLIP:
> > > > executeStatement  accepts DML, what if it's a streaming DML ?
> > > >     does it submit the job to cluster directly and blocks forever?
> > what's
> > > > the behavior for the next statements?
> > > >
> > > > nit: there's a typo in "the table describing the result for each kind
> > of
> > > > statement", "*Result Scheam" -> "Result Schema"*
> > > >
> > > >
> > > > godfrey he <go...@gmail.com> 于2020年2月18日周二 下午4:41写道:
> > > >
> > > > > Thanks Kurt and Jark for explanation, I now also think we should
> make
> > > the
> > > > > TableEnvironment interface more statable and should not change
> > > "sqlQuery"
> > > > > method and "from" method.
> > > > >
> > > > > Hi Jingsong. Regarding to the "DmlBatch", I totally agree with
> > > advantages
> > > > > of "addBatch" method. However, there are two more questions need to
> > > > solve:
> > > > > one is how users write multi-sink programs in a Table API ? and
> > another
> > > > is
> > > > > how users explain multi-sink program in both SQL and Table API ?
> > > > > Currently, "DmlBatch" class can solve those questions. (the main
> > > > > disadvantages is Inconsistent with the current interface)
> > > > >
> > > > > Bests,
> > > > > godfrey
> > > > >
> > > > > Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:
> > > > >
> > > > > > Hi Kurt and Godfrey,
> > > > > >
> > > > > > Thank you for your explanation.
> > > > > >
> > > > > > Regarding to the "DmlBatch",
> > > > > > I see there are some description for JDBC Statement.addBatch in
> the
> > > > > > document.
> > > > > > What do you think about introducing "addBatch" to the TableEnv
> > > instead
> > > > of
> > > > > > introducing a new class?
> > > > > > The advantage is:
> > > > > > - Consistent with JDBC statement.
> > > > > > - Consistent with current interface, what we need do is just
> modify
> > > > > method
> > > > > > name.
> > > > > >
> > > > > > Best,
> > > > > > Jingsong Lee
> > > > > >
> > > > > >
> > > > > > On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com>
> > wrote:
> > > > > >
> > > > > > > I don't think we should change `from` to `fromCatalog`,
> > especially
> > > > > `from`
> > > > > > > is just
> > > > > > > introduced in 1.10. I agree with Jark we should change
> interface
> > > only
> > > > > > when
> > > > > > > necessary,
> > > > > > > e.g. the semantic is broken or confusing. So I'm +1 to keep
> > > > `sqlQuery`
> > > > > as
> > > > > > > it is.
> > > > > > >
> > > > > > > Best,
> > > > > > > Kurt
> > > > > > >
> > > > > > >
> > > > > > > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com>
> > wrote:
> > > > > > >
> > > > > > > > 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 <
> godfreyhe@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 <yk...@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 <
> imjark@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 <
> > > > > > jingsonglee0@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 <
> > > > > > godfreyhe@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
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best, Jingsong Lee
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Benchao Li
> > > > School of Electronics Engineering and Computer Science, Peking
> > University
> > > > Tel:+86-15650713730
> > > > Email: libenchao@gmail.com; libenchao@pku.edu.cn
> > > >
> > >
> >
> >
> > --
> >
> > Benchao Li
> > School of Electronics Engineering and Computer Science, Peking University
> > Tel:+86-15650713730
> > Email: libenchao@gmail.com; libenchao@pku.edu.cn
> >
>


-- 

Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:+86-15650713730
Email: libenchao@gmail.com; libenchao@pku.edu.cn

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by godfrey he <go...@gmail.com>.
Hi Benchao,

I think the document has contained both parts: the behavior is explained
when introducing `executeStatement` method, and asynchronous execution
methods is explained in the appendix.

Bests,
Godfrey

Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午10:09写道:

> Hi godfrey,
>
> Thanks for your explanation.
>
> Do we need to clarify this in the FLIP? Maybe this confuses other users as
> well.
>
> godfrey he <go...@gmail.com> 于2020年2月28日周五 下午4:54写道:
>
> > Hi Benchao,
> >
> > > I have one question about this FLIP:
> > > executeStatement  accepts DML, what if it's a streaming DML ?
> > >    does it submit the job to cluster directly and blocks forever?
> what's
> > > the behavior for the next statements?
> > `executeStatement` is a synchronous method, will execute the statement
> once
> > calling this method and return the result until the job is finished.
> > We will introduce asynchronous method like `executeStatementAsync` in the
> > future.
> >
> > > nit: there's a typo in "the table describing the result for each kind
> of
> > > statement", "*Result Scheam" -> "Result Schema"*
> > Thanks for the reminding, I will fix it now.
> >
> > Bests,
> > Godfrey
> >
> > Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午4:00写道:
> >
> > > Hi Terry,
> > >
> > > Thanks for the propose, and sorry for joining the party late.
> > >
> > > I have one question about this FLIP:
> > > executeStatement  accepts DML, what if it's a streaming DML ?
> > >     does it submit the job to cluster directly and blocks forever?
> what's
> > > the behavior for the next statements?
> > >
> > > nit: there's a typo in "the table describing the result for each kind
> of
> > > statement", "*Result Scheam" -> "Result Schema"*
> > >
> > >
> > > godfrey he <go...@gmail.com> 于2020年2月18日周二 下午4:41写道:
> > >
> > > > Thanks Kurt and Jark for explanation, I now also think we should make
> > the
> > > > TableEnvironment interface more statable and should not change
> > "sqlQuery"
> > > > method and "from" method.
> > > >
> > > > Hi Jingsong. Regarding to the "DmlBatch", I totally agree with
> > advantages
> > > > of "addBatch" method. However, there are two more questions need to
> > > solve:
> > > > one is how users write multi-sink programs in a Table API ? and
> another
> > > is
> > > > how users explain multi-sink program in both SQL and Table API ?
> > > > Currently, "DmlBatch" class can solve those questions. (the main
> > > > disadvantages is Inconsistent with the current interface)
> > > >
> > > > Bests,
> > > > godfrey
> > > >
> > > > Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:
> > > >
> > > > > Hi Kurt and Godfrey,
> > > > >
> > > > > Thank you for your explanation.
> > > > >
> > > > > Regarding to the "DmlBatch",
> > > > > I see there are some description for JDBC Statement.addBatch in the
> > > > > document.
> > > > > What do you think about introducing "addBatch" to the TableEnv
> > instead
> > > of
> > > > > introducing a new class?
> > > > > The advantage is:
> > > > > - Consistent with JDBC statement.
> > > > > - Consistent with current interface, what we need do is just modify
> > > > method
> > > > > name.
> > > > >
> > > > > Best,
> > > > > Jingsong Lee
> > > > >
> > > > >
> > > > > On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com>
> wrote:
> > > > >
> > > > > > I don't think we should change `from` to `fromCatalog`,
> especially
> > > > `from`
> > > > > > is just
> > > > > > introduced in 1.10. I agree with Jark we should change interface
> > only
> > > > > when
> > > > > > necessary,
> > > > > > e.g. the semantic is broken or confusing. So I'm +1 to keep
> > > `sqlQuery`
> > > > as
> > > > > > it is.
> > > > > >
> > > > > > Best,
> > > > > > Kurt
> > > > > >
> > > > > >
> > > > > > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com>
> wrote:
> > > > > >
> > > > > > > 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 <go...@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 <yk...@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 <im...@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 <
> > > > > jingsonglee0@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 <
> > > > > godfreyhe@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
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best, Jingsong Lee
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > Benchao Li
> > > School of Electronics Engineering and Computer Science, Peking
> University
> > > Tel:+86-15650713730
> > > Email: libenchao@gmail.com; libenchao@pku.edu.cn
> > >
> >
>
>
> --
>
> Benchao Li
> School of Electronics Engineering and Computer Science, Peking University
> Tel:+86-15650713730
> Email: libenchao@gmail.com; libenchao@pku.edu.cn
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Benchao Li <li...@gmail.com>.
Hi godfrey,

Thanks for your explanation.

Do we need to clarify this in the FLIP? Maybe this confuses other users as
well.

godfrey he <go...@gmail.com> 于2020年2月28日周五 下午4:54写道:

> Hi Benchao,
>
> > I have one question about this FLIP:
> > executeStatement  accepts DML, what if it's a streaming DML ?
> >    does it submit the job to cluster directly and blocks forever? what's
> > the behavior for the next statements?
> `executeStatement` is a synchronous method, will execute the statement once
> calling this method and return the result until the job is finished.
> We will introduce asynchronous method like `executeStatementAsync` in the
> future.
>
> > nit: there's a typo in "the table describing the result for each kind of
> > statement", "*Result Scheam" -> "Result Schema"*
> Thanks for the reminding, I will fix it now.
>
> Bests,
> Godfrey
>
> Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午4:00写道:
>
> > Hi Terry,
> >
> > Thanks for the propose, and sorry for joining the party late.
> >
> > I have one question about this FLIP:
> > executeStatement  accepts DML, what if it's a streaming DML ?
> >     does it submit the job to cluster directly and blocks forever? what's
> > the behavior for the next statements?
> >
> > nit: there's a typo in "the table describing the result for each kind of
> > statement", "*Result Scheam" -> "Result Schema"*
> >
> >
> > godfrey he <go...@gmail.com> 于2020年2月18日周二 下午4:41写道:
> >
> > > Thanks Kurt and Jark for explanation, I now also think we should make
> the
> > > TableEnvironment interface more statable and should not change
> "sqlQuery"
> > > method and "from" method.
> > >
> > > Hi Jingsong. Regarding to the "DmlBatch", I totally agree with
> advantages
> > > of "addBatch" method. However, there are two more questions need to
> > solve:
> > > one is how users write multi-sink programs in a Table API ? and another
> > is
> > > how users explain multi-sink program in both SQL and Table API ?
> > > Currently, "DmlBatch" class can solve those questions. (the main
> > > disadvantages is Inconsistent with the current interface)
> > >
> > > Bests,
> > > godfrey
> > >
> > > Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:
> > >
> > > > Hi Kurt and Godfrey,
> > > >
> > > > Thank you for your explanation.
> > > >
> > > > Regarding to the "DmlBatch",
> > > > I see there are some description for JDBC Statement.addBatch in the
> > > > document.
> > > > What do you think about introducing "addBatch" to the TableEnv
> instead
> > of
> > > > introducing a new class?
> > > > The advantage is:
> > > > - Consistent with JDBC statement.
> > > > - Consistent with current interface, what we need do is just modify
> > > method
> > > > name.
> > > >
> > > > Best,
> > > > Jingsong Lee
> > > >
> > > >
> > > > On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com> wrote:
> > > >
> > > > > I don't think we should change `from` to `fromCatalog`, especially
> > > `from`
> > > > > is just
> > > > > introduced in 1.10. I agree with Jark we should change interface
> only
> > > > when
> > > > > necessary,
> > > > > e.g. the semantic is broken or confusing. So I'm +1 to keep
> > `sqlQuery`
> > > as
> > > > > it is.
> > > > >
> > > > > Best,
> > > > > Kurt
> > > > >
> > > > >
> > > > > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:
> > > > >
> > > > > > 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 <go...@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 <yk...@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 <im...@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 <
> > > > jingsonglee0@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 <
> > > > godfreyhe@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
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Best, Jingsong Lee
> > > >
> > >
> >
> >
> > --
> >
> > Benchao Li
> > School of Electronics Engineering and Computer Science, Peking University
> > Tel:+86-15650713730
> > Email: libenchao@gmail.com; libenchao@pku.edu.cn
> >
>


-- 

Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:+86-15650713730
Email: libenchao@gmail.com; libenchao@pku.edu.cn

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by godfrey he <go...@gmail.com>.
Hi Benchao,

> I have one question about this FLIP:
> executeStatement  accepts DML, what if it's a streaming DML ?
>    does it submit the job to cluster directly and blocks forever? what's
> the behavior for the next statements?
`executeStatement` is a synchronous method, will execute the statement once
calling this method and return the result until the job is finished.
We will introduce asynchronous method like `executeStatementAsync` in the
future.

> nit: there's a typo in "the table describing the result for each kind of
> statement", "*Result Scheam" -> "Result Schema"*
Thanks for the reminding, I will fix it now.

Bests,
Godfrey

Benchao Li <li...@gmail.com> 于2020年2月28日周五 下午4:00写道:

> Hi Terry,
>
> Thanks for the propose, and sorry for joining the party late.
>
> I have one question about this FLIP:
> executeStatement  accepts DML, what if it's a streaming DML ?
>     does it submit the job to cluster directly and blocks forever? what's
> the behavior for the next statements?
>
> nit: there's a typo in "the table describing the result for each kind of
> statement", "*Result Scheam" -> "Result Schema"*
>
>
> godfrey he <go...@gmail.com> 于2020年2月18日周二 下午4:41写道:
>
> > Thanks Kurt and Jark for explanation, I now also think we should make the
> > TableEnvironment interface more statable and should not change "sqlQuery"
> > method and "from" method.
> >
> > Hi Jingsong. Regarding to the "DmlBatch", I totally agree with advantages
> > of "addBatch" method. However, there are two more questions need to
> solve:
> > one is how users write multi-sink programs in a Table API ? and another
> is
> > how users explain multi-sink program in both SQL and Table API ?
> > Currently, "DmlBatch" class can solve those questions. (the main
> > disadvantages is Inconsistent with the current interface)
> >
> > Bests,
> > godfrey
> >
> > Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:
> >
> > > Hi Kurt and Godfrey,
> > >
> > > Thank you for your explanation.
> > >
> > > Regarding to the "DmlBatch",
> > > I see there are some description for JDBC Statement.addBatch in the
> > > document.
> > > What do you think about introducing "addBatch" to the TableEnv instead
> of
> > > introducing a new class?
> > > The advantage is:
> > > - Consistent with JDBC statement.
> > > - Consistent with current interface, what we need do is just modify
> > method
> > > name.
> > >
> > > Best,
> > > Jingsong Lee
> > >
> > >
> > > On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com> wrote:
> > >
> > > > I don't think we should change `from` to `fromCatalog`, especially
> > `from`
> > > > is just
> > > > introduced in 1.10. I agree with Jark we should change interface only
> > > when
> > > > necessary,
> > > > e.g. the semantic is broken or confusing. So I'm +1 to keep
> `sqlQuery`
> > as
> > > > it is.
> > > >
> > > > Best,
> > > > Kurt
> > > >
> > > >
> > > > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:
> > > >
> > > > > 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 <go...@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 <yk...@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 <im...@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 <
> > > jingsonglee0@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 <
> > > godfreyhe@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
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best, Jingsong Lee
> > >
> >
>
>
> --
>
> Benchao Li
> School of Electronics Engineering and Computer Science, Peking University
> Tel:+86-15650713730
> Email: libenchao@gmail.com; libenchao@pku.edu.cn
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Benchao Li <li...@gmail.com>.
Hi Terry,

Thanks for the propose, and sorry for joining the party late.

I have one question about this FLIP:
executeStatement  accepts DML, what if it's a streaming DML ?
    does it submit the job to cluster directly and blocks forever? what's
the behavior for the next statements?

nit: there's a typo in "the table describing the result for each kind of
statement", "*Result Scheam" -> "Result Schema"*


godfrey he <go...@gmail.com> 于2020年2月18日周二 下午4:41写道:

> Thanks Kurt and Jark for explanation, I now also think we should make the
> TableEnvironment interface more statable and should not change "sqlQuery"
> method and "from" method.
>
> Hi Jingsong. Regarding to the "DmlBatch", I totally agree with advantages
> of "addBatch" method. However, there are two more questions need to solve:
> one is how users write multi-sink programs in a Table API ? and another is
> how users explain multi-sink program in both SQL and Table API ?
> Currently, "DmlBatch" class can solve those questions. (the main
> disadvantages is Inconsistent with the current interface)
>
> Bests,
> godfrey
>
> Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:
>
> > Hi Kurt and Godfrey,
> >
> > Thank you for your explanation.
> >
> > Regarding to the "DmlBatch",
> > I see there are some description for JDBC Statement.addBatch in the
> > document.
> > What do you think about introducing "addBatch" to the TableEnv instead of
> > introducing a new class?
> > The advantage is:
> > - Consistent with JDBC statement.
> > - Consistent with current interface, what we need do is just modify
> method
> > name.
> >
> > Best,
> > Jingsong Lee
> >
> >
> > On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com> wrote:
> >
> > > I don't think we should change `from` to `fromCatalog`, especially
> `from`
> > > is just
> > > introduced in 1.10. I agree with Jark we should change interface only
> > when
> > > necessary,
> > > e.g. the semantic is broken or confusing. So I'm +1 to keep `sqlQuery`
> as
> > > it is.
> > >
> > > Best,
> > > Kurt
> > >
> > >
> > > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:
> > >
> > > > 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 <go...@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 <yk...@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 <im...@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 <
> > jingsonglee0@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 <
> > godfreyhe@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
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> > Best, Jingsong Lee
> >
>


-- 

Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:+86-15650713730
Email: libenchao@gmail.com; libenchao@pku.edu.cn

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by godfrey he <go...@gmail.com>.
Thanks Kurt and Jark for explanation, I now also think we should make the
TableEnvironment interface more statable and should not change "sqlQuery"
method and "from" method.

Hi Jingsong. Regarding to the "DmlBatch", I totally agree with advantages
of "addBatch" method. However, there are two more questions need to solve:
one is how users write multi-sink programs in a Table API ? and another is
how users explain multi-sink program in both SQL and Table API ?
Currently, "DmlBatch" class can solve those questions. (the main
disadvantages is Inconsistent with the current interface)

Bests,
godfrey

Jingsong Li <ji...@gmail.com> 于2020年2月15日周六 下午9:09写道:

> Hi Kurt and Godfrey,
>
> Thank you for your explanation.
>
> Regarding to the "DmlBatch",
> I see there are some description for JDBC Statement.addBatch in the
> document.
> What do you think about introducing "addBatch" to the TableEnv instead of
> introducing a new class?
> The advantage is:
> - Consistent with JDBC statement.
> - Consistent with current interface, what we need do is just modify method
> name.
>
> Best,
> Jingsong Lee
>
>
> On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com> wrote:
>
> > I don't think we should change `from` to `fromCatalog`, especially `from`
> > is just
> > introduced in 1.10. I agree with Jark we should change interface only
> when
> > necessary,
> > e.g. the semantic is broken or confusing. So I'm +1 to keep `sqlQuery` as
> > it is.
> >
> > Best,
> > Kurt
> >
> >
> > On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:
> >
> > > 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 <go...@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 <yk...@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 <im...@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 <
> jingsonglee0@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 <
> godfreyhe@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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
> --
> Best, Jingsong Lee
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Jingsong Li <ji...@gmail.com>.
Hi Kurt and Godfrey,

Thank you for your explanation.

Regarding to the "DmlBatch",
I see there are some description for JDBC Statement.addBatch in the
document.
What do you think about introducing "addBatch" to the TableEnv instead of
introducing a new class?
The advantage is:
- Consistent with JDBC statement.
- Consistent with current interface, what we need do is just modify method
name.

Best,
Jingsong Lee


On Sat, Feb 15, 2020 at 4:48 PM Kurt Young <yk...@gmail.com> wrote:

> I don't think we should change `from` to `fromCatalog`, especially `from`
> is just
> introduced in 1.10. I agree with Jark we should change interface only when
> necessary,
> e.g. the semantic is broken or confusing. So I'm +1 to keep `sqlQuery` as
> it is.
>
> Best,
> Kurt
>
>
> On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:
>
> > 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 <go...@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 <yk...@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 <im...@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 <ji...@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 <go...@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
> > > > > >
> > > > >
> > > >
> > >
> >
>


-- 
Best, Jingsong Lee

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Kurt Young <yk...@gmail.com>.
I don't think we should change `from` to `fromCatalog`, especially `from`
is just
introduced in 1.10. I agree with Jark we should change interface only when
necessary,
e.g. the semantic is broken or confusing. So I'm +1 to keep `sqlQuery` as
it is.

Best,
Kurt


On Sat, Feb 15, 2020 at 3:59 PM Jark Wu <im...@gmail.com> wrote:

> 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 <go...@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 <yk...@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 <im...@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 <ji...@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 <go...@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
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Jark Wu <im...@gmail.com>.
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 <go...@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 <yk...@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 <im...@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 <ji...@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 <go...@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
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by godfrey he <go...@gmail.com>.
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 <yk...@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 <im...@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 <ji...@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 <go...@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
> > >
> >
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Kurt Young <yk...@gmail.com>.
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 <im...@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 <ji...@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 <go...@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
> >
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Jark Wu <im...@gmail.com>.
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 <ji...@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 <go...@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
>

Re: [DISCUSS] FLIP-84: Improve & Refactor execute/sqlQuery/sqlUpdate APIS of TableEnvironment

Posted by Jingsong Li <ji...@gmail.com>.
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 <go...@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