Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 5F5972009F4 for ; Thu, 26 May 2016 19:08:40 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 5AC77160A18; Thu, 26 May 2016 17:08:40 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 516BB160A17 for ; Thu, 26 May 2016 19:08:39 +0200 (CEST) Received: (qmail 41012 invoked by uid 500); 26 May 2016 17:08:38 -0000 Mailing-List: contact notifications-help@asterixdb.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@asterixdb.apache.org Delivered-To: mailing list notifications@asterixdb.apache.org Received: (qmail 41003 invoked by uid 99); 26 May 2016 17:08:38 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 26 May 2016 17:08:38 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 237521A61F2 for ; Thu, 26 May 2016 17:08:38 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.126 X-Spam-Level: ** X-Spam-Status: No, score=2.126 tagged_above=-999 required=6.31 tests=[MISSING_HEADERS=1.207, SPF_FAIL=0.919] autolearn=disabled Received: from mx2-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id 3S6JInVyBbuP for ; Thu, 26 May 2016 17:08:35 +0000 (UTC) Received: from unhygienix.ics.uci.edu (unhygienix.ics.uci.edu [128.195.14.130]) by mx2-lw-eu.apache.org (ASF Mail Server at mx2-lw-eu.apache.org) with ESMTP id 856EE5F24D for ; Thu, 26 May 2016 17:08:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by unhygienix.ics.uci.edu (Postfix) with ESMTP id F13FE241D2C; Thu, 26 May 2016 10:08:45 -0700 (PDT) Date: Thu, 26 May 2016 10:08:45 -0700 From: "Murtadha Hubail (Code Review)" CC: Jenkins , Till Westmann , Yingyi Bu Reply-To: hubailmor@gmail.com X-Gerrit-MessageType: merged Subject: Change in asterixdb[master]: ASTERIXDB-1419: Fix type checking for CollectionType X-Gerrit-Change-Id: Ibf11d6c59ae00fe6d21fed8d75f199ee4ac9852c X-Gerrit-ChangeURL: X-Gerrit-Commit: bd1d98ce49ae6b7b2272e2025c07d15ed6d37376 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/2.8.4 Message-Id: <20160526170845.F13FE241D2C@unhygienix.ics.uci.edu> archived-at: Thu, 26 May 2016 17:08:40 -0000 Murtadha Hubail has submitted this change and it was merged. Change subject: ASTERIXDB-1419: Fix type checking for CollectionType ...................................................................... ASTERIXDB-1419: Fix type checking for CollectionType Change-Id: Ibf11d6c59ae00fe6d21fed8d75f199ee4ac9852c Reviewed-on: https://asterix-gerrit.ics.uci.edu/886 Reviewed-by: Jenkins Tested-by: Jenkins Reviewed-by: Yingyi Bu --- A asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_1/issue_1419_drop_type_with_collection_1.1.ddl.aql A asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_2/issue_1419_drop_type_with_collection_2.1.ddl.aql M asterixdb/asterix-app/src/test/resources/metadata/testsuite.xml M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AUnionType.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractCollectionType.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractComplexType.java 8 files changed, 148 insertions(+), 21 deletions(-) Approvals: Yingyi Bu: Looks good to me, approved Jenkins: Looks good to me, but someone else must approve; Verified diff --git a/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_1/issue_1419_drop_type_with_collection_1.1.ddl.aql b/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_1/issue_1419_drop_type_with_collection_1.1.ddl.aql new file mode 100644 index 0000000..22b4d02 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_1/issue_1419_drop_type_with_collection_1.1.ddl.aql @@ -0,0 +1,39 @@ +/* + * 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. + */ +/* + * Description : drop a type containing a collection + * Expected Res : Success + * Date : 25 May 2016 + * Issue : 1419 + */ + +drop dataverse test if exists; +create dataverse test; +use dataverse test; + +create type typeSnapshot if not exists as open { + countyID: int32, + timeBin: interval, + tweetCount: int32, + retweetCount: int32, + users: [int64], + top50HashTags: [{string:int32}] +} + +drop type typeSnapshot; diff --git a/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_2/issue_1419_drop_type_with_collection_2.1.ddl.aql b/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_2/issue_1419_drop_type_with_collection_2.1.ddl.aql new file mode 100644 index 0000000..55691ee --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/issue_1419_drop_type_with_collection_2/issue_1419_drop_type_with_collection_2.1.ddl.aql @@ -0,0 +1,40 @@ +/* + * 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. + */ +/* + * Description : Drop a used type + * Expected Res : Failure + * Date : 25 May 2016 + * Issue : 1419 + */ + +drop dataverse test if exists; +create dataverse test; +use dataverse test; + +create type subType as { + id:int32, + list:[string] +} + +create type superType as { + superid:int32, + superlist:[subType] +} + +drop type subType; diff --git a/asterixdb/asterix-app/src/test/resources/metadata/testsuite.xml b/asterixdb/asterix-app/src/test/resources/metadata/testsuite.xml index 7a85d57..8f35293 100644 --- a/asterixdb/asterix-app/src/test/resources/metadata/testsuite.xml +++ b/asterixdb/asterix-app/src/test/resources/metadata/testsuite.xml @@ -323,6 +323,17 @@ temp_dataset + + + issue_1419_drop_type_with_collection + + + + + none + Error: Cannot drop type test.subType being used by type test.superType_superlist + + diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java index 54ec084..b82c9ee 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.rmi.RemoteException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.apache.asterix.common.api.IAsterixAppRuntimeContext; @@ -77,6 +78,9 @@ import org.apache.asterix.om.base.AMutableString; import org.apache.asterix.om.base.AString; import org.apache.asterix.om.types.ARecordType; +import org.apache.asterix.om.types.ATypeTag; +import org.apache.asterix.om.types.AUnionType; +import org.apache.asterix.om.types.AbstractComplexType; import org.apache.asterix.om.types.BuiltinType; import org.apache.asterix.om.types.IAType; import org.apache.asterix.transaction.management.opcallbacks.SecondaryIndexModificationOperationCallback; @@ -716,7 +720,7 @@ } private void confirmDatatypeIsUnused(JobId jobId, String dataverseName, String datatypeName) - throws MetadataException { + throws MetadataException, RemoteException { confirmDatatypeIsUnusedByDatatypes(jobId, dataverseName, datatypeName); confirmDatatypeIsUnusedByDatasets(jobId, dataverseName, datatypeName); } @@ -734,33 +738,48 @@ } private void confirmDatatypeIsUnusedByDatatypes(JobId jobId, String dataverseName, String datatypeName) - throws MetadataException { + throws MetadataException, RemoteException { //If any datatype uses this type, throw an error //TODO: Currently this loads all types into memory. This will need to be fixed for large numbers of types + Datatype dataTypeToBeDropped = getDatatype(jobId, dataverseName, datatypeName); + assert dataTypeToBeDropped != null; + IAType typeToBeDropped = dataTypeToBeDropped.getDatatype(); List datatypes = getAllDatatypes(jobId); - for (Datatype type : datatypes) { - if (!type.getDataverseName().equals(dataverseName)) { + for (Datatype dataType : datatypes) { + //skip types in different dataverses as well as the type to be dropped itself + if (!dataType.getDataverseName().equals(dataverseName) + || dataType.getDatatype().getTypeName().equals(datatypeName)) { continue; } - ARecordType recType = (ARecordType) type.getDatatype(); - for (IAType subType : recType.getFieldTypes()) { - if (subType.getTypeName().equals(datatypeName)) { - throw new MetadataException("Cannot drop type " + dataverseName + "." + datatypeName - + " being used by type " + dataverseName + "." + recType.getTypeName()); - } + + AbstractComplexType recType = (AbstractComplexType) dataType.getDatatype(); + if (recType.containsType(typeToBeDropped)) { + throw new MetadataException("Cannot drop type " + dataverseName + "." + datatypeName + + " being used by type " + dataverseName + "." + recType.getTypeName()); } } } private List getNestedComplexDatatypeNamesForThisDatatype(JobId jobId, String dataverseName, - String datatypeName) throws Exception { + String datatypeName) throws MetadataException, RemoteException { //Return all field types that aren't builtin types Datatype parentType = getDatatype(jobId, dataverseName, datatypeName); - ARecordType recType = (ARecordType) parentType.getDatatype(); - List nestedTypes = new ArrayList(); - for (IAType subType : recType.getFieldTypes()) { - if (!(subType instanceof BuiltinType)) { - nestedTypes.add(subType.getTypeName()); + + List subTypes = null; + if (parentType.getDatatype().getTypeTag() == ATypeTag.RECORD) { + ARecordType recType = (ARecordType) parentType.getDatatype(); + subTypes = Arrays.asList(recType.getFieldTypes()); + } else if (parentType.getDatatype().getTypeTag() == ATypeTag.UNION) { + AUnionType recType = (AUnionType) parentType.getDatatype(); + subTypes = recType.getUnionList(); + } + + List nestedTypes = new ArrayList<>(); + if (subTypes != null) { + for (IAType subType : subTypes) { + if (!(subType instanceof BuiltinType)) { + nestedTypes.add(subType.getTypeName()); + } } } return nestedTypes; @@ -977,8 +996,10 @@ try { while (rangeCursor.hasNext()) { rangeCursor.next(); - sb.append(TupleUtils.printTuple(rangeCursor.getTuple(), new ISerializerDeserializer[] { - AqlSerializerDeserializerProvider.INSTANCE.getSerializerDeserializer(BuiltinType.ASTRING), + sb.append(TupleUtils.printTuple(rangeCursor.getTuple(), + new ISerializerDeserializer[] { + AqlSerializerDeserializerProvider.INSTANCE.getSerializerDeserializer( + BuiltinType.ASTRING), AqlSerializerDeserializerProvider.INSTANCE.getSerializerDeserializer(BuiltinType.ASTRING), AqlSerializerDeserializerProvider.INSTANCE .getSerializerDeserializer(BuiltinType.ASTRING) })); @@ -995,7 +1016,7 @@ private void searchIndex(JobId jobId, IMetadataIndex index, ITupleReference searchKey, IValueExtractor valueExtractor, List results) - throws MetadataException, IndexException, IOException { + throws MetadataException, IndexException, IOException { IBinaryComparatorFactory[] comparatorFactories = index.getKeyBinaryComparatorFactory(); String resourceName = index.getFile().toString(); IIndex indexInstance = datasetLifecycleManager.getIndex(resourceName); diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java index 163421d..3cbac09 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java @@ -310,4 +310,14 @@ } return typeList; } + + @Override + public boolean containsType(IAType type) { + for (IAType aType : fieldTypes) { + if (aType.getTypeName().equals(type.getTypeName())) { + return true; + } + } + return false; + } } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AUnionType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AUnionType.java index 3c29e7f..8e92b81 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AUnionType.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AUnionType.java @@ -56,9 +56,10 @@ return isMissableType() || isNullableType(); } - private boolean containsType(IAType t) { + @Override + public boolean containsType(IAType type) { for (int index = 0; index < unionList.size(); ++index) { - if (unionList.get(index) != null && unionList.get(index).equals(t)) { + if (unionList.get(index) != null && unionList.get(index).equals(type)) { return true; } } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractCollectionType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractCollectionType.java index 6e2ffef..88a4f93 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractCollectionType.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractCollectionType.java @@ -63,4 +63,8 @@ } } + @Override + public boolean containsType(IAType type) { + return isTyped() && itemType.getTypeName().equals(type.getTypeName()); + } } diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractComplexType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractComplexType.java index 1f43e38..9eddd7f 100644 --- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractComplexType.java +++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/AbstractComplexType.java @@ -45,4 +45,5 @@ return this.deepEqual((IAObject) object); } + public abstract boolean containsType(IAType type); } -- To view, visit https://asterix-gerrit.ics.uci.edu/886 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibf11d6c59ae00fe6d21fed8d75f199ee4ac9852c Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail Gerrit-Reviewer: Jenkins Gerrit-Reviewer: Murtadha Hubail Gerrit-Reviewer: Till Westmann Gerrit-Reviewer: Yingyi Bu