[BEAM-4196][SQL] Support complex types in DDL #5276

akedin · 2018-05-04T00:32:25Z

Support complex types in DDL

Supported syntax:

CREATE TABLE tableName (
  f_array2 ARRAY<INTEGER>,
  f_array3 ARRAY<ARRAY<INTEGER>>,
  f_map MAP<INTEGER, MAP<VARCHAR, VARCHAR>>,
  f_row ROW( f_int1 INTEGER, f_str1 VARCHAR),
  f_row2 ROW< f_int1 INTEGER, f_str1 VARCHAR>
)

Follow this checklist to help us incorporate your contribution quickly and easily:

akedin · 2018-05-04T00:32:45Z

R: @apilloud @kennknowles @xumingmin @xumingming

apilloud

This is awesome! You made it sound like it was weeks out when we talked earlier. It saddens me how much this moves us away from calcite/server but I can't say it is the wrong decision.

LGTM

apilloud · 2018-05-04T02:29:04Z

...extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/parser/SqlDdlNodes.java

 public class SqlDdlNodes {
  private SqlDdlNodes() {}

-  /** Creates a CREATE TABLE. */


This annoys me. I don't like this extra indirection in calcite but I also don't like deviating from calcite unnecessaraly. Not much you can do about that.

akedin · 2018-05-08T02:25:47Z

run java precommit

akedin · 2018-05-08T02:26:31Z

thread leak test fails: #5300 BEAM-4088

kennknowles

LGTM after some more tests

kennknowles · 2018-05-08T12:30:35Z

sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/BeamSqlCliTest.java

+        + "name varchar COMMENT 'name', \n"
+        + "age int COMMENT 'age', \n"
+        + "tags MAP<VARCHAR, VARCHAR>, \n"
+        + "nestedMap MAP<INTEGER, MAP<VARCHAR, INTEGER>> \n"


Can you also test:

maps inside arrays

arrays inside maps

rows inside arrays

rows inside maps

maps inside rows

arrays inside rows

nested nested rows

... etc ...

At some point we might want to use QuickCheck/SmallCheck; that is one of the best ways to test valid syntax.

kennknowles · 2018-05-08T12:30:35Z

sdks/java/extensions/sql/src/main/codegen/includes/parserImpls.ftl

+        fieldType = SimpleType()
+    )
+    [
+        collectionTypeName = CollectionTypeName()


This is to support INT ARRAY in that syntax for it? TBH I don't think we need to start with that, even though it is "standard" all the related dialects don't use that syntax.

And I didn't realize JavaCC generated LL(k) parsers. That's unfortunate. So I can see why the grammar has to be factored like this. But what this doesn't support is (INT ARRAY) ARRAY or the no-parens version INT ARRAY ARRAY.

I suppose it is fine to leave as-is, but probably want to file something about the limitation, or tell me that I am wrong. And lots of tests. And also feel free to remove if you don't want the complication.

Yes this clause is for the postfix array support. I left it mostly because it was already kinda there, just limited to MULTISETS, not arrays. We don't support either at the moment, and I would rather remove it for now for consistency

kennknowles · 2018-05-08T12:30:36Z

...ions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/parser/BeamSqlParserTest.java

        "create table person (\n"
            + "id int COMMENT 'id', \n"
-            + "name varchar(31) COMMENT 'name') \n"
+            + "name varchar COMMENT 'name') \n"


OK, I commented on the CLI test, but this is the better place for the parser tests.

Adding QuickCheck tests

kennknowles · 2018-05-08T12:30:36Z

...ensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/parser/SqlCreateTable.java

  public List<SqlNode> getOperandList() {
-    return ImmutableNullableList.of(name, columnList, type, comment, location, tblProperties);
+    throw new UnsupportedOperationException(
+        "Getting operands CREATE TABLE is unsupported at the moment");


Just curious - where is it used? You can't really do much except copy the node if you don't know what kind of node it is, so just asking for my own education how Calcite (or BeamSQL) uses this.

It's mostly used by implementations of the SqlCall to access their parameters. SqlCreate itself doesn't really need it but we might (or might not) need something like this for JDBC integration, depending on how CREATE TABLE parsing will be implemented there

…e types Create Schema.Fields instead of SqlDataTypeSpec, this will allow us to parse any types directly into Schema types, not limiting ourselves to primitive types supported by SqlDataTypeSpec

Add support for declaring fields of arrays of primitive types

Parse Row fields

Column is only used in Table for DDL and similar use cases. It seems better to use Schema in these places instead to avoid going back and forth between them

Support ARRAY<type> definition

Support MAP<primitiveTypeKey, typeValue>

Add support for Row<fields>

Add random arbitrary field type generation with QuickCheck to verify correctness of schema creation by CREATE TABLE

akedin · 2018-05-08T23:41:19Z

Updated:

removed postfix array support;
added BeamDdlNestedTypesTest which tests things with QuickCheck. Examples:

create table tablename ( fieldName MAP<DOUBLE, ARRAY<ARRAY<ARRAY<MAP<BOOLEAN, MAP<DECIMAL, TINYINT>>>>>> ) TYPE 'text' LOCATION '/home/admin/person'

create table tablename ( fieldName MAP<BOOLEAN, MAP<FLOAT, ARRAY<BOOLEAN>>> ) TYPE 'text' LOCATION '/home/admin/person'

create table tablename ( fieldName ARRAY<ARRAY<FLOAT>> ) TYPE 'text' LOCATION '/home/admin/person'


create table tablename ( fieldName MAP<INTEGER, MAP<INTEGER, MAP<INTEGER, MAP<SMALLINT, ROW<field_0 INTEGER,field_1 TIMESTAMP,field_2 TIMESTAMP,field_3 TIMESTAMP>>>>> ) TYPE 'text' LOCATION '/home/admin/person'


create table tablename ( fieldName MAP<BOOLEAN, ROW<field_0 INTEGER,field_1 DECIMAL,field_2 VARCHAR>> ) TYPE 'text' LOCATION '/home/admin/person'

akedin · 2018-05-09T18:01:13Z

run java precommit

akedin · 2018-05-09T18:39:36Z

whoa it's green

kennknowles · 2018-05-09T18:54:51Z

Awesome. That's next-level test assurance.

apilloud approved these changes May 4, 2018

View reviewed changes

akedin force-pushed the support-complex-types branch 2 times, most recently from 8db89ba to acf5061 Compare May 7, 2018 18:05

kennknowles approved these changes May 8, 2018

View reviewed changes

kennknowles reviewed May 8, 2018

View reviewed changes

akedin added 11 commits May 8, 2018 09:17

[SQL] Change DDL parser to create Beam Schema types instead of Calcit…

a9e9d4b

…e types Create Schema.Fields instead of SqlDataTypeSpec, this will allow us to parse any types directly into Schema types, not limiting ourselves to primitive types supported by SqlDataTypeSpec

[SQL] Support arrays of primitive types in DDL

e510a0f

Add support for declaring fields of arrays of primitive types

[SQL] Parse Row fields

c425907

Parse Row fields

[SQL] Remove Column. Replace with Schema

5b66828

Column is only used in Table for DDL and similar use cases. It seems better to use Schema in these places instead to avoid going back and forth between them

[SQL] Validate Table schemas in BeamSqlCliTest

ef6e413

[SQL] Add support for angular brackets array definition

ae8fe9c

Support ARRAY<type> definition

[SQL] Add support for Maps in DDL

e8eda1a

Support MAP<primitiveTypeKey, typeValue>

[SQL] Add support for angular brackets Row syntax

5143ec3

Add support for Row<fields>

[SQL] Fix rebase conflicts

8678e3d

[SQL] Add QuickCheck and tests for DDL schema verification

2df7bab

Add random arbitrary field type generation with QuickCheck to verify correctness of schema creation by CREATE TABLE

[SQL] Remove support for postfix array declaration in DDL

8b349e3

akedin force-pushed the support-complex-types branch from acf5061 to 8b349e3 Compare May 8, 2018 23:33

kennknowles merged commit da8fa25 into apache:master May 9, 2018

[BEAM-4196][SQL] Support complex types in DDL #5276

[BEAM-4196][SQL] Support complex types in DDL #5276

Uh oh!

Conversation

akedin commented May 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akedin commented May 4, 2018

Uh oh!

apilloud left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akedin commented May 8, 2018

Uh oh!

akedin commented May 8, 2018

Uh oh!

kennknowles left a comment

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!

akedin commented May 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akedin commented May 9, 2018

Uh oh!

akedin commented May 9, 2018

Uh oh!

kennknowles commented May 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akedin commented May 4, 2018 •

edited

Loading

akedin commented May 8, 2018 •

edited

Loading