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 ---- --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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/ --- |
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. --- |
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/ --- |
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? --- |
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 --- |
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 --- |
Free forum by Nabble | Edit this page |