[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use TableInfo and St...

classic Classic list List threaded Threaded
55 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use TableInfo and St...

qiuchenjian-2
GitHub user jackylk opened a pull request:

    https://github.com/apache/carbondata/pull/1571

    [CARBONDATA-1811] [CTAS] Use TableInfo and StructType when creating table

    This PR contains refactory for CarbonCreateTableCommand and CarbonAlterTableAddColumnCommand, they should use TableInfo and StructType to create the table.
    This PR is required to implement CREATE TABLE AS SELECT syntax.
   
     - [X] Any interfaces changed?
     No
     - [X] Any backward compatibility impacted?
     No
     - [X] Document update required?
    NO
     - [X] Testing done
    Yes
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    This is refactor of parser and commands, it is impossible to break into smaller sub task.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jackylk/incubator-carbondata tableinfo_addcolumn2

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1571.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1571
   
----
commit b293303f6b3f80ecaf0db1bbad95a3015d3db8c3
Author: Jacky Li <[hidden email]>
Date:   2017-11-26T17:11:33Z

    use TableInfo
   
    rebase

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use TableInfo and StructTyp...

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1471/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1891/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1472/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1520/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/390/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1922/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1529/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1928/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1568/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1968/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1585/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1982/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1984/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1587/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use StructType as sc...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1571#discussion_r154070506
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/TableProperty.java ---
    @@ -0,0 +1,209 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.metadata;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +import java.util.Map;
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants;
    +import org.apache.carbondata.core.metadata.datatype.DataType;
    +import org.apache.carbondata.core.metadata.datatype.DataTypes;
    +import org.apache.carbondata.core.metadata.datatype.StructField;
    +import org.apache.carbondata.core.metadata.datatype.StructType;
    +import org.apache.carbondata.core.metadata.schema.table.MalformedCarbonCommandException;
    +import org.apache.carbondata.core.util.CarbonUtil;
    +
    +/**
    + * This class encapsulate the definition of sortColumns, dictionaryColumns, noInvertedIndexColumns
    + * in the table.
    + * These columns are extracted from schema and table property map when creating instance
    + * of this class.
    + * It is used when creating table and alter table add column.
    + */
    +public class TableProperty {
    --- End diff --
   
    I think better rename this file to either `TablePropertiesInfo` or `TablePropertiesWrapper` as it is deriving information from properties.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1571: [CARBONDATA-1811] [CTAS] Use StructType as schema wh...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1571
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1612/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use StructType as sc...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1571#discussion_r154072485
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/TableProperty.java ---
    @@ -0,0 +1,209 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.metadata;
    +
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +import java.util.Map;
    +
    +import org.apache.carbondata.core.constants.CarbonCommonConstants;
    +import org.apache.carbondata.core.metadata.datatype.DataType;
    +import org.apache.carbondata.core.metadata.datatype.DataTypes;
    +import org.apache.carbondata.core.metadata.datatype.StructField;
    +import org.apache.carbondata.core.metadata.datatype.StructType;
    +import org.apache.carbondata.core.metadata.schema.table.MalformedCarbonCommandException;
    +import org.apache.carbondata.core.util.CarbonUtil;
    +
    +/**
    + * This class encapsulate the definition of sortColumns, dictionaryColumns, noInvertedIndexColumns
    + * in the table.
    + * These columns are extracted from schema and table property map when creating instance
    + * of this class.
    + * It is used when creating table and alter table add column.
    + */
    +public class TableProperty {
    +
    +  private Map<String, String> tableProperties;
    +  private List<String> sortColumns;
    +  private List<String> noInvertedIndexColumns;
    +  private List<String> dictionaryColumns;
    +
    +  /**
    +   * This is called in case of create table
    +   */
    +  public TableProperty(StructType schema, Map<String, String> tableProperties)
    +      throws MalformedCarbonCommandException {
    +    if (tableProperties == null) {
    +      throw new IllegalArgumentException("table property should not be null");
    +    }
    +    this.tableProperties = tableProperties;
    +    this.noInvertedIndexColumns = extractNoInvertedIndexColumns();
    +    this.dictionaryColumns = extractDictionaryColumns();
    +    this.sortColumns = extractSortColumns(schema);
    +  }
    +
    +  /**
    +   * This is called in case of alter table add column
    +   */
    +  public TableProperty(
    +      List<StructField> newFields,
    +      List<String> existingSortColumns,
    +      Map<String, String> existingTableProperties,
    +      Map<String, String> alterTableProperties) {
    +    if (existingSortColumns == null) {
    +      throw new IllegalArgumentException("sort columns should not be null");
    +    }
    +    if (existingTableProperties == null) {
    +      throw new IllegalArgumentException("table property should not be null");
    +    }
    +
    +    // update the existing table property according to alter table property
    +    for (Map.Entry<String, String> entry : alterTableProperties.entrySet()) {
    +      String newValue = entry.getValue();
    +      String existingValue = existingTableProperties.get(entry.getKey());
    +      if (existingValue != null) {
    +        existingTableProperties.put(entry.getKey(), existingValue + "," + newValue);
    --- End diff --
   
    Why it is required to be appended with old value?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use StructType as sc...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1571#discussion_r154077005
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/ArrayType.java ---
    @@ -30,4 +30,13 @@
       public boolean isComplexType() {
         return true;
       }
    +
    +  @Override
    +  public int getNumOfChild() {
    +    return 1;
    --- End diff --
   
    isn't it elementype.getNumOfChild


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1571: [CARBONDATA-1811] [CTAS] Use StructType as sc...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1571#discussion_r154086411
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/ColumnTableRelation.java ---
    @@ -0,0 +1,55 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.metadata.schema.table;
    +
    +public class ColumnTableRelation {
    +  private String parentColumnName;
    +  private String parentColumnId;
    +  private String parentTableName;
    +  private String parentDatabaseName;
    +  private String parentTableId;
    +
    +  public ColumnTableRelation(String parentColumnName, String parentColumnId, String parentTableName,
    --- End diff --
   
    what is the difference with `ParentColumnTableRelation` class? why don't you use here


---
123