[CALCITE-1581] UDTF like in hive #1138

pengzhiwei2018 · 2019-03-28T15:51:18Z

No description provided.

chunweilei · 2019-03-29T02:24:12Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

@@ -1320,6 +1331,175 @@ protected SqlNode performUnconditionalRewrites(
    return node;
  }

+  private SqlNode performHiveUdtfRewrite(SqlNode node) {
+    Map<SqlSelect, UdtfInfo> selectUdtfInfo = new HashMap<>();


Can you add some comments?

done! Thanks for the review.

chunweilei · 2019-03-29T02:25:08Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+    Map<SqlSelect, UdtfInfo> selectUdtfInfo = new HashMap<>();
+     SqlNode rewirte = performHiveUdtfRewriteInternal(node, selectUdtfInfo);
+   //  System.out.println("before:\n" + node.toString() +" \nafter:\n" + rewirte.toString());
+    return node;


Should remove this useless comment.

Sorry to push this test code.remove now!

chunweilei · 2019-03-29T02:27:37Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+  private SqlNode performHiveUdtfRewriteInternal(SqlNode current,
+                                                 Map<SqlSelect, UdtfInfo> selectUdtfInfos) {
+    System.out.println("start rewrite:" + current);
+


We don't print anything in general.

Sorry for push this test code,remove now!

chunweilei · 2019-03-29T02:29:19Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+      // join the from node with the table function.
+      if (udtfInfo != null && select.getFrom() != null) {
+        SqlBasicCall joinRight = createLateralTable(udtfInfo);
+        SqlNode newFrom = new SqlJoin(


Maybe use SqlCall instead of SqlBasicCall is better.

well,the createLateralTable() method return a "as" node, so it must be a SqlBasicCall.

yuqi1129 · 2019-03-29T12:43:58Z

core/src/main/java/org/apache/calcite/runtime/CalciteResource.java

+  @BaseMessage("''{0}'' should be a table function")
+  ExInst<SqlValidatorException> exceptTableFunction(String name);
+
+  @BaseMessage("Only one table function allowed in select list")


yuqi1129 · 2019-03-29T13:22:27Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+    for (int i = 0; i < info.fieldNames.size(); i++) {
+      operands[2 + i] = info.fieldNames.get(i);
+    }
+    SqlBasicCall lateralTableAs = new SqlBasicCall(asOp, operands, SqlParserPos.ZERO);


can directly return new SqlBasicCall(asOp, operands, SqlParserPos.ZERO);

agreed! Thanks for you review!

yuqi1129 · 2019-03-29T13:37:07Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+            tableFunctionInfo.node = (SqlBasicCall) udtfNode;
+            tableFunctionInfo.selectIndex = i;
+            tableFunctionInfo.fieldNames = (SqlNodeList) aliasNode;
+            tableFunctionInfo.tableName = "_table_function_" + nextTableFunctionNameId++;


make "_table_function_" a constant string may be better like
public static final String TABLE_FUNCTION_PREFIX = "_table_function_"
The global variables nextTableFunctionNameId once use once in this function is a little odd

I agree with you than make "table_function" as constant. But nextTableFunctionNameId should be a global variable in SqlValidatorImpl, as we may reuse the SqlValidatorImpl to validator different SqlNode. It is better to differentiate the table function in different SqlNode.

michaelmior · 2019-05-27T17:19:23Z

@pengzhiwei2018 Could you take a look at the conflicts here?

pengzhiwei2018 · 2019-05-28T02:43:34Z

@pengzhiwei2018 Could you take a look at the conflicts here?

Thanks for start this review! Done for it!

chunweilei

LGTM

danny0405 · 2019-10-29T09:30:24Z

core/src/main/codegen/templates/Parser.jj

-    final SqlIdentifier id;
+    SqlIdentifier id;
+    final List<SqlNode> ids = new ArrayList();
+    final Span s = span();


Rename ids to columnAliases and id to columnAlias ?

Thanks for you suggestions.

danny0405 · 2019-10-30T01:32:16Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+    aliases.addAll(aliasList);
+
+    String tableAlias = TABLE_FUNCTION_PREFIX + nextGeneratedId++;
+    // Expand the table function alias


Can we just use the function name as the alias prefix, the TABLE_FUNCTION_PREFIX seems unnecessary.

Thanks for your suggestion. done!

danny0405 · 2019-10-30T01:35:53Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

+    scopes.put(functionCall, tableScope);
+    // Associate the select with the table function
+    selectTableFunctions.put(parentSelect, functionCall);
+


I'm skeptical about this member selectTableFunctions, the SqlValidator never has such mapping, it handles all the states to the "Namespace" and "scope", i'm wondering if we can move this mapping into each scope, because each select has a scope. Thus, we can keep the validator code clean.

I think selectTableFunctions is the same to the whereScopes or groupByScopes in the SqlValidatorImpl.

jinxing64 · 2019-10-31T06:47:39Z

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

@@ -294,6 +299,9 @@
  // Flag saying if we enable the implicit type coercion.
  private boolean enableTypeCoercion;

+  // Maping the table function and the select node.


typo -- "Mapping between select node and table function. "

jinxing64 · 2019-10-31T11:06:55Z

core/src/main/codegen/templates/Parser.jj

+            <RPAREN> {
+             if (!this.conformance.allowSelectTableFunction()) {
+               throw SqlUtil.newContextException(getPos(),
+                     RESOURCE.notAllowTableFunctionInSelect());


indent: 4 spaces

jinxing64 · 2019-10-31T11:07:27Z

core/src/main/codegen/templates/Parser.jj

+               throw SqlUtil.newContextException(getPos(),
+                     RESOURCE.notAllowTableFunctionInSelect());
+             }
+             e = SqlStdOperatorTable.AS.createCall(s.end(e), e, new SqlNodeList(ids, s.end(e)));


Though less than 100 characters, but shall we break the line ?

jinxing64 · 2019-10-31T11:24:36Z

core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java

+    assert asIdentifier instanceof SqlIdentifier
+        || (asIdentifier instanceof SqlNodeList
+            && validator.getConformance().allowSelectTableFunction());
+


I'm confused. In validateSelectList, this change has already rewriten the select items by handleTableFunctionInSelect, how is it possible that SqlAsOperator does validation here ?
Can you explain or give a test that the assertion can be failed ?

jinxing64 · 2019-10-31T11:26:29Z

core/src/main/java/org/apache/calcite/sql/SqlAsOperator.java

+      SqlIdentifier id = (SqlIdentifier) ids.get(i);
+      if (!id.isSimple()) {
+        throw validator.newValidationError(id,
+              RESOURCE.aliasMustBeSimpleIdentifier());


indent -- 4 spaces

Thanks for your suggestion.

jinxing64 · 2019-10-31T11:36:12Z

core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java

@@ -408,6 +409,13 @@
   * false otherwise.
   */
  boolean allowPluralTimeUnits();
+
+  /**
+   * Whether SELECT can contain a table function.


How about SELECT -> select list ?

jinxing64 · 2019-10-31T11:37:23Z

core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java

+  SQL_SERVER_2008,
+
+  /** Conformance value that instructs Calcite to use SQL semantics
+   * consistent with Hive version. */


Remove the word of version or specify the version.

remove unused empty line remove test code & add comment add comment check for more than one table function in select fix test sql make _table_function_ as a constant support table function without from add comment fix conflict move rewrite to SqlToRelConverter fix code style modify comment fix doc issue remove used blank add more test case fix format fix some format issue fix compile failture fix compile error use SqlUtil#newContextException format code

danny0405 · 2019-12-02T04:12:19Z

Thanks @pengzhiwei2018 for the update, maybe i can find some time to review the latest change in this week ~

chunweilei · 2020-07-30T04:23:48Z

@pengzhiwei2018 , could you please resolve conflicts? I think it needs a final review and I would like to do that.

Close apache#1138

danny0405 · 2020-08-04T03:08:56Z

Close because there is already another PR.

Close apache#1138

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from 20fbc8a to 8d17df4 Compare March 28, 2019 16:12

pengzhiwei2018 closed this Mar 28, 2019

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from 8d17df4 to 11c067f Compare March 28, 2019 16:15

pengzhiwei2018 reopened this Mar 28, 2019

chunweilei reviewed Mar 29, 2019

View reviewed changes

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 4 times, most recently from ed821b5 to 635914d Compare March 29, 2019 05:45

yuqi1129 reviewed Mar 29, 2019

View reviewed changes

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 8 times, most recently from 86684b8 to 48942ba Compare April 3, 2019 09:17

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from 18fd91c to 486c3c2 Compare April 16, 2019 03:57

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from d4704e9 to d649921 Compare May 28, 2019 02:41

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from b6a2dcc to 41ae5d1 Compare June 7, 2019 06:01

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from 41ae5d1 to 893d69d Compare June 17, 2019 02:58

pengzhiwei2018 requested a review from danny0405 August 22, 2019 10:04

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from 5a9d3d3 to 892ab01 Compare August 22, 2019 10:27

chunweilei reviewed Aug 23, 2019

View reviewed changes

chunweilei approved these changes Aug 27, 2019

View reviewed changes

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from a90cd98 to 2e176f0 Compare August 28, 2019 04:00

julianhyde force-pushed the master branch from 2c19098 to f3758a6 Compare September 16, 2019 17:44

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from 2e176f0 to 86f26ae Compare September 26, 2019 06:05

pengzhiwei2018 force-pushed the dev_udtf_table_function branch 2 times, most recently from 31bba48 to beaf125 Compare October 29, 2019 07:09

danny0405 requested changes Oct 30, 2019

View reviewed changes

jinxing64 reviewed Oct 31, 2019

View reviewed changes

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from beaf125 to 7d1a5bc Compare November 29, 2019 07:43

pengzhiwei2018 force-pushed the dev_udtf_table_function branch from 27692d3 to ed35496 Compare November 29, 2019 10:28

danny0405 force-pushed the master branch 2 times, most recently from 80f411d to ca27fe9 Compare November 30, 2019 07:52

vlsi force-pushed the master branch 2 times, most recently from 49cb002 to 8768a23 Compare December 29, 2019 12:07

hsyuan force-pushed the master branch from e42a79b to a0ef3c9 Compare March 29, 2020 16:53

vlsi force-pushed the master branch from e4458fa to 5462be9 Compare July 17, 2020 20:20

chunweilei added the needs-a-final-review This patch looks good from at least one PMC or committer, but still needs a specialist final review label Jul 30, 2020

chunweilei pushed a commit to chunweilei/calcite that referenced this pull request Jul 31, 2020

[CALCITE-1581] Support UDTF like Hive

62ee42f

Close apache#1138

chunweilei mentioned this pull request Jul 31, 2020

[CALCITE-1581] Support UDTF like Hive #2092

Open

danny0405 closed this Aug 4, 2020

chunweilei pushed a commit to chunweilei/calcite that referenced this pull request Aug 4, 2020

[CALCITE-1581] Support UDTF like Hive

da5fe76

Close apache#1138

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CALCITE-1581] UDTF like in hive #1138

[CALCITE-1581] UDTF like in hive #1138

pengzhiwei2018 commented Mar 28, 2019

chunweilei Mar 29, 2019

pengzhiwei2018 Mar 29, 2019

chunweilei Mar 29, 2019

pengzhiwei2018 Mar 29, 2019

chunweilei Mar 29, 2019

pengzhiwei2018 Mar 29, 2019

chunweilei Mar 29, 2019 •

edited

Loading

pengzhiwei2018 Mar 29, 2019

yuqi1129 Mar 29, 2019

yuqi1129 Mar 29, 2019

pengzhiwei2018 Mar 30, 2019

yuqi1129 Mar 29, 2019

pengzhiwei2018 Mar 30, 2019

michaelmior commented May 27, 2019

pengzhiwei2018 commented May 28, 2019

chunweilei left a comment

danny0405 Oct 29, 2019

pengzhiwei2018 Nov 29, 2019

danny0405 Oct 30, 2019

jinxing64 Oct 31, 2019

pengzhiwei2018 Nov 29, 2019

danny0405 Oct 30, 2019

pengzhiwei2018 Nov 29, 2019

jinxing64 Oct 31, 2019

jinxing64 Oct 31, 2019

jinxing64 Oct 31, 2019

jinxing64 Oct 31, 2019

jinxing64 Oct 31, 2019

pengzhiwei2018 Nov 29, 2019

jinxing64 Oct 31, 2019

jinxing64 Oct 31, 2019

danny0405 commented Dec 2, 2019

chunweilei commented Jul 30, 2020

danny0405 commented Aug 4, 2020

[CALCITE-1581] UDTF like in hive #1138

[CALCITE-1581] UDTF like in hive #1138

Conversation

pengzhiwei2018 commented Mar 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunweilei Mar 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelmior commented May 27, 2019

pengzhiwei2018 commented May 28, 2019

chunweilei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danny0405 commented Dec 2, 2019

chunweilei commented Jul 30, 2020

danny0405 commented Aug 4, 2020

chunweilei Mar 29, 2019 •

edited

Loading