Hi Jark,

good question. Actually, there was an ERROR kind that could have been enabled via a config option. Such that everything ends up in the TableResult. But @Kurt had some concerns which is why we didn't add this kind of result yet.

Regards,
Timo


On 25.03.20 12:00, Jark Wu wrote:
Hi Godfrey,

The changes sounds good to me. +1 to start another voting.

A minor question: does the ResultKind contain an ERROR kind?

Best,
Jark


On Wed, 25 Mar 2020 at 18:51, Timo Walther <twal...@apache.org> wrote:

Hi Godfrey,

thanks for starting the discussion on the mailing list. And sorry again
for the late reply to FLIP-84. I have updated the Google doc one more
time to incorporate the offline discussions.

  From Dawid's and my view, it is fine to postpone the multiline support
to a separate method. This can be future work even though we will need
it rather soon.

If there are no objections, I suggest to update the FLIP-84 again and
have another voting process.

Thanks,
Timo


On 25.03.20 11:17, godfrey he wrote:
Hi community,
Timo, Fabian and Dawid have some feedbacks about FLIP-84[1]. The
feedbacks
are all about new introduced methods. We had a discussion yesterday, and
most of feedbacks have been agreed upon. Here is the conclusions:

*1. about proposed methods in `TableEnvironment`:*

the original proposed methods:

TableEnvironment.createDmlBatch(): DmlBatch
TableEnvironment.executeStatement(String statement): ResultTable

the new proposed methods:

// we should not use abbreviations in the API, and the term "Batch" is
easily confused with batch/streaming processing
TableEnvironment.createStatementSet(): StatementSet

// every method that takes SQL should have `Sql` in its name
// supports multiline statement ???
TableEnvironment.executeSql(String statement): TableResult

// new methods. supports explaining DQL and DML
TableEnvironment.explainSql(String statement, ExplainDetail... details):
String


*2. about proposed related classes:*

the original proposed classes:

interface DmlBatch {
      void addInsert(String insert);
      void addInsert(String targetPath, Table table);
      ResultTable execute() throws Exception ;
      String explain(boolean extended);
}

public interface ResultTable {
      TableSchema getResultSchema();
      Iterable<Row> getResultRows();
}

the new proposed classes:

interface StatementSet {
      // every method that takes SQL should have `Sql` in its name
      // return StatementSet instance for fluent programming
      addInsertSql(String statement): StatementSet

      // return StatementSet instance for fluent programming
      addInsert(String tablePath, Table table): StatementSet

      // new method. support overwrite mode
      addInsert(String tablePath, Table table, boolean overwrite):
StatementSet

      explain(): String

      // new method. supports adding more details for the result
      explain(ExplainDetail... extraDetails): String

      // throw exception ???
      execute(): TableResult
}

interface TableResult {
      getTableSchema(): TableSchema

      // avoid custom parsing of an "OK" row in programming
      getResultKind(): ResultKind

      // instead of `get` make it explicit that this is might be
triggering
an expensive operation
      collect(): Iterable<Row>

      // for fluent programming
      print(): Unit
}

enum ResultKind {
      SUCCESS, // for DDL, DCL and statements with a simple "OK"
      SUCCESS_WITH_CONTENT, // rows with important content are available
(DML, DQL)
}


*3. new proposed methods in `Table`*

`Table.insertInto()` will be deprecated, and the following methods are
introduced:

Table.executeInsert(String tablePath): TableResult
Table.executeInsert(String tablePath, boolean overwrite): TableResult
Table.explain(ExplainDetail... details): String
Table.execute(): TableResult

There are two issues need further discussion, one is whether
`TableEnvironment.executeSql(String statement): TableResult` needs to
support multiline statement (or whether `TableEnvironment` needs to
support
multiline statement), and another one is whether `StatementSet.execute()`
needs to throw exception.

please refer to the feedback document [2] for the details.

Any suggestions are warmly welcomed!

[1]

https://wiki.apache.org/confluence/pages/viewpage.action?pageId=134745878
[2]

https://docs.google.com/document/d/1ueLjQWRPdLTFB_TReAyhseAX-1N3j4WYWD0F02Uau0E/edit

Best,
Godfrey





Reply via email to