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