Luo Chen has submitted this change and it was merged. Change subject: [ASTERIXDB-2280][IDX] Fix Index on Optional Nested Field ......................................................................
[ASTERIXDB-2280][IDX] Fix Index on Optional Nested Field - user model changes: no - storage format changes: no - interface changes: no Details: - Fix the creation of indexes on optional nested field by handling AUnionType properly. - Fix the nullability check of primary keys. Change-Id: If098ff4a45db3c4e8b65d098cbb0940c8b7ab845 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2477 Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> --- M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.1.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.2.update.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.3.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.4.query.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nestrecords/nested-optional-pk/nested-optional-pk.1.ddl.sqlpp M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java M asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java 9 files changed, 238 insertions(+), 6 deletions(-) Approvals: Anon. E. Moose #1000171: Jenkins: Verified; ; Verified Dmitry Lychagin: Looks good to me, approved Objections: Jenkins: Violations found diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java index 4b4b2b0..d5ebc6e 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ValidateUtil.java @@ -131,11 +131,19 @@ } else { partitioningExprTypes = KeyFieldTypeUtil.getKeyTypes(recType, metaRecType, partitioningExprs, keySourceIndicators); - for (int fidx = 0; fidx < partitioningExprTypes.size(); ++fidx) { - IAType fieldType = partitioningExprTypes.get(fidx); + for (int i = 0; i < partitioningExprs.size(); i++) { + List<String> partitioningExpr = partitioningExprs.get(i); + IAType fieldType = partitioningExprTypes.get(i); if (fieldType == null) { throw new CompilationException(ErrorCode.COMPILATION_FIELD_NOT_FOUND, - RecordUtil.toFullyQualifiedName(partitioningExprs.get(fidx))); + RecordUtil.toFullyQualifiedName(partitioningExpr)); + } + boolean nullable = KeyFieldTypeUtil.chooseSource(keySourceIndicators, i, recType, metaRecType) + .isSubFieldNullable(partitioningExpr); + if (nullable) { + // key field is nullable + throw new CompilationException(ErrorCode.COMPILATION_PRIMARY_KEY_CANNOT_BE_NULLABLE, + RecordUtil.toFullyQualifiedName(partitioningExpr)); } switch (fieldType.getTypeTag()) { case TINYINT: @@ -155,7 +163,7 @@ break; case UNION: throw new CompilationException(ErrorCode.COMPILATION_PRIMARY_KEY_CANNOT_BE_NULLABLE, - RecordUtil.toFullyQualifiedName(partitioningExprs.get(fidx))); + RecordUtil.toFullyQualifiedName(partitioningExpr)); default: throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_PRIMARY_KEY_TYPE, fieldType.getTypeTag()); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.1.ddl.sqlpp new file mode 100644 index 0000000..f650e0a --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.1.ddl.sqlpp @@ -0,0 +1,47 @@ +/* + * 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. + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + + +create type test.MyRecordtmp as +{ + id : bigint, + point : point, + kwds : string, + line1 : line, + line2 : line, + poly1 : polygon, + poly2 : polygon, + rec : rectangle +}; + +create type test.MyRecord as +{ + pid: uuid, + nested : MyRecordtmp? +}; + +create dataset MyDatatmp(MyRecordtmp) primary key id; + +create dataset MyData(MyRecord) primary key pid autogenerated; + diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.2.update.sqlpp new file mode 100644 index 0000000..8199dbd --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.2.update.sqlpp @@ -0,0 +1,28 @@ +/* + * 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. + */ + +use test; + + +load dataset MyDatatmp using localfs ((`path`=`asterix_nc1://data/spatial/spatialData.json`),(`format`=`adm`)); + +insert into MyData +select element {'nested':c} +from MyDatatmp as c +; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.3.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.3.ddl.sqlpp new file mode 100644 index 0000000..5683fe0 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.3.ddl.sqlpp @@ -0,0 +1,24 @@ +/* + * 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. + */ + +use test; + + +create index rtree_index_point on MyData (nested.point) type rtree; + diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.4.query.sqlpp new file mode 100644 index 0000000..93665a4 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nested-index/index-selection/rtree-secondary-index-optional/rtree-secondary-index-optional.4.query.sqlpp @@ -0,0 +1,27 @@ +/* + * 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. + */ + +use test; + + +select element {'id':o.nested.id} +from MyData as o +where test.`spatial-intersect`(o.nested.point,test.`create-polygon`([4.0,1.0,4.0,4.0,12.0,4.0,12.0,1.0])) +order by o.nested.id +; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nestrecords/nested-optional-pk/nested-optional-pk.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nestrecords/nested-optional-pk/nested-optional-pk.1.ddl.sqlpp new file mode 100644 index 0000000..c4af7d8 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/nestrecords/nested-optional-pk/nested-optional-pk.1.ddl.sqlpp @@ -0,0 +1,37 @@ +/* + * 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. + */ + +drop dataverse test if exists; +create dataverse test; + +use test; + + +create type test.MyRecordtmp as +{ + id : bigint, + point : point +}; + +create type test.MyRecord as +{ + nested : MyRecordtmp? +}; + +create dataset MyData(MyRecord) primary key nested.id; diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml index 95c059d..e545c3b 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml @@ -4399,6 +4399,11 @@ <output-dir compare="Text">rtree-secondary-index-open</output-dir> </compilation-unit> </test-case> + <test-case FilePath="nested-index/index-selection"> + <compilation-unit name="rtree-secondary-index-optional"> + <output-dir compare="Text">rtree-secondary-index-open</output-dir> + </compilation-unit> + </test-case> </test-group> <test-group name="nested-index/external-indexing"> <test-case FilePath="nested-index/external-indexing"> @@ -4581,6 +4586,12 @@ <output-dir compare="Text">nestrecord</output-dir> </compilation-unit> </test-case> + <test-case FilePath="nestrecords"> + <compilation-unit name="nested-optional-pk"> + <output-dir compare="Text">nested-optional-pk</output-dir> + <expected-error>ASX1021: The primary key field "nested.id" cannot be nullable</expected-error> + </compilation-unit> + </test-case> </test-group> <test-group name="null-missing"> <test-case FilePath="null-missing"> diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java index bf19b03..3159030 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.asterix.common.config.DatasetConfig.IndexType; +import org.apache.asterix.common.exceptions.AsterixException; import org.apache.asterix.common.exceptions.CompilationException; import org.apache.asterix.common.exceptions.ErrorCode; import org.apache.asterix.common.transactions.IRecoveryManager.ResourceType; @@ -153,8 +154,18 @@ ARecordType recType) throws AlgebricksException { Pair<IAType, Boolean> keyPairType = null; IAType subType = recType; + boolean nullable = false; for (int i = 0; i < fieldName.size(); i++) { - subType = ((ARecordType) subType).getFieldType(fieldName.get(i)); + if (subType instanceof AUnionType) { + nullable = nullable || ((AUnionType) subType).isUnknownableType(); + subType = ((AUnionType) subType).getActualType(); + } + if (subType instanceof ARecordType) { + subType = ((ARecordType) subType).getFieldType(fieldName.get(i)); + } else { + throw AsterixException.create(ErrorCode.COMPILATION_ILLEGAL_STATE, "Unexpected type " + fieldType); + } + if (subType == null) { keyPairType = Index.getNonNullableType(fieldType); break; @@ -163,13 +174,16 @@ if (subType != null) { keyPairType = Index.getNonNullableKeyFieldType(fieldName, recType); } + keyPairType.second = keyPairType.second || nullable; return keyPairType; } public static Pair<IAType, Boolean> getNonNullableKeyFieldType(List<String> expr, ARecordType recType) throws AlgebricksException { IAType keyType = Index.keyFieldType(expr, recType); - return getNonNullableType(keyType); + Pair<IAType, Boolean> pair = getNonNullableType(keyType); + pair.second = pair.second || recType.isSubFieldNullable(expr); + return pair; } private static IAType keyFieldType(List<String> expr, ARecordType recType) throws AlgebricksException { 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 baaed59..26cbf1f 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 @@ -28,7 +28,10 @@ import org.apache.asterix.common.annotations.IRecordTypeAnnotation; import org.apache.asterix.common.exceptions.AsterixException; +import org.apache.asterix.common.exceptions.CompilationException; +import org.apache.asterix.common.exceptions.ErrorCode; import org.apache.asterix.om.base.IAObject; +import org.apache.asterix.om.utils.NonTaggedFormatUtil; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -226,6 +229,39 @@ } /** + * + * @param subFieldName + * The full pathname of the field + * @return The nullability of the field + * @throws AlgebricksException + */ + public boolean isSubFieldNullable(List<String> subFieldName) throws AlgebricksException { + IAType subRecordType = getFieldType(subFieldName.get(0)); + for (int i = 1; i < subFieldName.size(); i++) { + if (subRecordType == null) { + // open field is nullable + return true; + } + if (subRecordType.getTypeTag().equals(ATypeTag.UNION)) { + if (NonTaggedFormatUtil.isOptional(subRecordType)) { + return true; + } + subRecordType = ((AUnionType) subRecordType).getActualType(); + if (subRecordType.getTypeTag() != ATypeTag.OBJECT) { + throw new AsterixException( + "Field accessor is not defined for values of type " + subRecordType.getTypeTag()); + } + } + if (!(subRecordType instanceof ARecordType)) { + throw CompilationException.create(ErrorCode.COMPILATION_ILLEGAL_STATE, + "Illegal field type " + subRecordType.getTypeTag() + " when checking field nullability"); + } + subRecordType = ((ARecordType) subRecordType).getFieldType(subFieldName.get(i)); + } + return subRecordType == null || NonTaggedFormatUtil.isOptional(subRecordType); + } + + /** * Returns the field type of the field name if it exists, otherwise null. * * @param fieldName -- To view, visit https://asterix-gerrit.ics.uci.edu/2477 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: If098ff4a45db3c4e8b65d098cbb0940c8b7ab845 Gerrit-PatchSet: 13 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Luo Chen <cl...@uci.edu> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Luo Chen <cl...@uci.edu> Gerrit-Reviewer: Taewoo Kim <taew...@uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>