[CALCITE-6022] Support "CREATE TABLE LIKE" DDL by macroguo-ghy · Pull Request #3442 · apache/calcite

macroguo-ghy · 2023-09-26T06:38:54Z

No description provided.

LakeShen · 2023-09-28T07:27:22Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+import java.util.stream.Collectors;
+
+/**
+ * Parse tree for {@code CREATE TABLE LIKE} statement.


It is better to give a CREATE TABLE LIKE sql example.

Thanks for your suggestion. But I haven't seen other DDL SqlNode contains any examples. And I think user can aware of how to use CREATE TABLE LIKE syntax by referring to reference.md.

LakeShen · 2023-09-28T07:32:54Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+        .collect(Collectors.toSet());
+  }
+
+  @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {


Could we add the sql dialect transform unit test for SqlCreateTableLike?

I think in Calcite, all DDL in calcite-server will be execute by calcite instead of executing in other adapter(you can refer to org.apache.calcite.prepare.CalcitePrepareImpl#prepare2_).
So in THIS jira, we don't need to care about if the DDL command can execute in other dbms. Maybe we can log a new jira to discuss "Dialect for DDL".

LakeShen · 2023-09-28T07:40:10Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

    return Pair.of(schema, name);
  }

+  static Table table(CalcitePrepare.Context context, SqlIdentifier id) {


Could we give some code comments for this method?It is better to help others understand this method.

LakeShen · 2023-09-28T07:42:15Z

@macroguo-ghy ,thank you for opening this PR, left a couple of comments!

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

macroguo-ghy

Hi @LakeShen @JiajunBernoulli, thanks for your review and suggestion. I reply to your comment. Please help me review again if you have time.

macroguo-ghy · 2023-10-01T07:18:22Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+import java.util.stream.Collectors;
+
+/**
+ * Parse tree for {@code CREATE TABLE LIKE} statement.


Thanks for your suggestion. But I haven't seen other DDL SqlNode contains any examples. And I think user can aware of how to use CREATE TABLE LIKE syntax by referring to reference.md.

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

macroguo-ghy · 2023-10-01T07:56:38Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+        .collect(Collectors.toSet());
+  }
+
+  @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {


I think in Calcite, all DDL in calcite-server will be execute by calcite instead of executing in other adapter(you can refer to org.apache.calcite.prepare.CalcitePrepareImpl#prepare2_).
So in THIS jira, we don't need to care about if the DDL command can execute in other dbms. Maybe we can log a new jira to discuss "Dialect for DDL".

macroguo-ghy · 2023-10-01T08:03:46Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

    return Pair.of(schema, name);
  }

+  static Table table(CalcitePrepare.Context context, SqlIdentifier id) {


JiajunBernoulli · 2023-10-02T06:34:45Z

server/src/main/codegen/includes/parserImpls.ftl

+    )
+}
+
+SqlCreate SqlCreateTableLike(Span s, boolean replace, boolean ifNotExists, SqlIdentifier id) :


You calling SqlCreateTableLike by SqlCreateTable .

SqlCreateTable is added to createStatementParserMethods

Can we add SqlCreateTableLike to createStatementParserMethods?

Fix this. Could you please review this PR again? @JiajunBernoulli

JiajunBernoulli · 2023-10-09T09:54:22Z

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

    (
 <#-- additional literal parser methods are included here -->
 <#list (parser.createStatementParserMethods!default.parser.createStatementParserMethods) as method>
+        <#if method = "SqlCreateTableLike">


Oh, this makes things too complicated.

Can we add it in createStatementParserMethods?

I'm afraid not, we need to use LOOKAHEAD to distinguish CREATE TABLE and CREATE TABLE ... LIKE.
create = ${method}(s, replace), in this line , ${method} will be replaced by createStatementParserMethod, so we can not use LOOKAHEAD(...) SqlCreateTableLike replace SqlCreateTableLike. This will result in compilation errors.

Sorry, I misled you.

Please drop the last commit(Add SqlCreateTableLike to createStatementParserMethods).

I will merge it at the weekends.

OK, I reverted and squashed the commits.

macroguo-ghy · 2023-10-11T11:17:38Z

Hi @JiajunBernoulli, can you review this PR again? I believe we can merge it before 1.36.0 release.

JiajunBernoulli · 2023-10-14T09:05:31Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+ */
+public class SqlCreateTableLike extends SqlCreate {
+  private static final SqlOperator OPERATOR =
+      new SqlSpecialOperator("CREATE TABLE LIKE", SqlKind.CREATE_TABLE);


We need SqlKind.CREATE_TABLE_LIKE because SqlCreateTableLike not extends SqlCreateTable.

User maybe mistake if them are both SqlKind.CREATE_TABLE.

JiajunBernoulli · 2023-10-14T09:14:07Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

  }

+  /** Return the table from the given {@code} context and {@code} name. */
+  static Table table(CalcitePrepare.Context context, SqlIdentifier name) {


Used only once.

It doesn't seem necessary to abstract it as a method.

JiajunBernoulli · 2023-10-14T09:16:57Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

+        final boolean includingDefaults =
+            optionSet.contains(SqlCreateTableLike.LikeOption.DEFAULTS)
+                || optionSet.contains(SqlCreateTableLike.LikeOption.ALL);
+        ief = new NullInitializerExpressionFactory() {


Need class name and document, not anonymous class.

JiajunBernoulli · 2023-10-14T09:20:28Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

    }
  }

+  @Test void testCreateTableLike() throws Exception {


Need one JIRA link and document.

JiajunBernoulli · 2023-10-14T09:24:25Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+      }
+
+      try {
+        x = s.executeUpdate("insert into t2 values (3, 4, 5, 6)");


We can use assertThrows.

JiajunBernoulli · 2023-10-14T09:24:43Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+      try {
+        x = s.executeUpdate("insert into t2 values (3, 4, 5)");
+        fail("expected error, got " + x);
+      } catch (SQLException e) {


assertThrows

JiajunBernoulli · 2023-10-14T09:40:46Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+        assertThat(r.getInt("H"), is(3));
+        assertThat(r.wasNull(), is(false));
+        assertThat(r.getInt("I"), is(4));
+        assertThat(r.getInt("J"), is(0)); // excluding generated column


Why is 0, not null?

Because java.sql.ResultSet#getInt(java.lang.String) will return 0 if the value is SQL NULL. I add assertThat(r.wasNull(), is(true)) to clarify.

macroguo-ghy

Thanks for your reviews, @JiajunBernoulli ! I have pushed a new commit, and I noticed that Julian updating the lint rule, so I have made a git rebase.

macroguo-ghy · 2023-10-14T16:01:36Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+        assertThat(r.getInt("H"), is(3));
+        assertThat(r.wasNull(), is(false));
+        assertThat(r.getInt("I"), is(4));
+        assertThat(r.getInt("J"), is(0)); // excluding generated column


Because java.sql.ResultSet#getInt(java.lang.String) will return 0 if the value is SQL NULL. I add assertThat(r.wasNull(), is(true)) to clarify.

macroguo-ghy · 2023-10-14T16:02:16Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

+        final boolean includingDefaults =
+            optionSet.contains(SqlCreateTableLike.LikeOption.DEFAULTS)
+                || optionSet.contains(SqlCreateTableLike.LikeOption.ALL);
+        ief = new NullInitializerExpressionFactory() {


macroguo-ghy · 2023-10-14T16:02:25Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

    }
  }

+  @Test void testCreateTableLike() throws Exception {


macroguo-ghy · 2023-10-14T16:02:34Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+      }
+
+      try {
+        x = s.executeUpdate("insert into t2 values (3, 4, 5, 6)");


macroguo-ghy · 2023-10-14T16:02:41Z

server/src/test/java/org/apache/calcite/test/ServerTest.java

+      try {
+        x = s.executeUpdate("insert into t2 values (3, 4, 5)");
+        fail("expected error, got " + x);
+      } catch (SQLException e) {


macroguo-ghy · 2023-10-14T16:03:02Z

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java

+ */
+public class SqlCreateTableLike extends SqlCreate {
+  private static final SqlOperator OPERATOR =
+      new SqlSpecialOperator("CREATE TABLE LIKE", SqlKind.CREATE_TABLE);


macroguo-ghy · 2023-10-14T16:03:31Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

  }

+  /** Return the table from the given {@code} context and {@code} name. */
+  static Table table(CalcitePrepare.Context context, SqlIdentifier name) {


JiajunBernoulli · 2023-10-15T06:28:44Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

+      }
+    }
+
+    if (pair.left.plus().getTable(pair.right) != null) {


Check can be first, we can moved them to line 568.
And then add one line comment to introduce InitializerExpressionFactory.

JiajunBernoulli · 2023-10-15T06:29:51Z

server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java

+  public void execute(SqlCreateTableLike create,
+      CalcitePrepare.Context context) {
+    final Pair<CalciteSchema, String> pair = schema(context, true, create.name);
+    final JavaTypeFactory typeFactory = context.getTypeFactory();


typeFactory and rowType can be moved to last because them are used by last code.

JiajunBernoulli · 2023-10-15T06:31:08Z

PR is already good, and the little suggestion can be made when you squash commits.

macroguo-ghy · 2023-10-15T15:02:33Z

OK, I have squashed the commits. Thanks for your reviews! @LakeShen @JiajunBernoulli

JiajunBernoulli · 2023-10-16T02:05:11Z

@macroguo-ghy , Commit message should same as JIRA summary.

Support "CREATE TABLE ... LIKE" DDL in server module

macroguo-ghy · 2023-10-16T02:08:32Z

Sorry, my mistake. I have edited the commit message. @JiajunBernoulli

sonarqubecloud · 2023-10-16T02:29:20Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

83.4% Coverage
0.0% Duplication

macroguo-ghy force-pushed the 6022-like branch from 656bd26 to 12441cc Compare September 26, 2023 11:11

JiajunBernoulli self-assigned this Sep 27, 2023

LakeShen reviewed Sep 28, 2023

View reviewed changes

JiajunBernoulli reviewed Sep 29, 2023

View reviewed changes

core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTableLike.java Show resolved Hide resolved

macroguo-ghy commented Oct 1, 2023

View reviewed changes

LakeShen approved these changes Oct 1, 2023

View reviewed changes

JiajunBernoulli reviewed Oct 2, 2023

View reviewed changes

macroguo-ghy force-pushed the 6022-like branch from b8fe034 to c3376c1 Compare October 8, 2023 07:11

JiajunBernoulli reviewed Oct 9, 2023

View reviewed changes

macroguo-ghy force-pushed the 6022-like branch from c3376c1 to 1c9fa5c Compare October 14, 2023 07:56

JiajunBernoulli reviewed Oct 14, 2023

View reviewed changes

macroguo-ghy force-pushed the 6022-like branch from 1c9fa5c to e550f60 Compare October 14, 2023 15:56

macroguo-ghy commented Oct 14, 2023

View reviewed changes

JiajunBernoulli approved these changes Oct 15, 2023

View reviewed changes

JiajunBernoulli reviewed Oct 15, 2023

View reviewed changes

macroguo-ghy force-pushed the 6022-like branch from 3a4b1fe to 14d9772 Compare October 15, 2023 13:56

JiajunBernoulli added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 16, 2023

[CALCITE-6022] Support "CREATE TABLE ... LIKE" DDL in server module

593a398

macroguo-ghy force-pushed the 6022-like branch from 14d9772 to 593a398 Compare October 16, 2023 02:07

JiajunBernoulli merged commit 23b7931 into apache:main Oct 19, 2023

Conversation

macroguo-ghy commented Sep 26, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LakeShen Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LakeShen commented Sep 28, 2023

Uh oh!

Uh oh!

macroguo-ghy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macroguo-ghy Oct 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macroguo-ghy Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macroguo-ghy commented Oct 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LakeShen Sep 28, 2023 •

edited

Loading

macroguo-ghy Oct 8, 2023 •

edited

Loading

macroguo-ghy Oct 9, 2023 •

edited

Loading