Steven Jacobs has posted comments on this change. Change subject: Enabled Datasets to use Datatypes from foreign Dataverses ......................................................................
Patch Set 10: (5 comments) I think I addressed all of your concerns. My two big comments: 1) Please see previous code review comments for full discussions on tests, but the summary is that I found and removed duplicate test files from Meta data tests 2) Please note that this review doesn't allow types to use foreign types. Maybe this is something that someone would want later but it's not considered here. I am posting the new patchset now https://asterix-gerrit.ics.uci.edu/#/c/558/10/asterix-app/src/main/java/org/apache/asterix/file/ExternalIndexingOperations.java File asterix-app/src/main/java/org/apache/asterix/file/ExternalIndexingOperations.java: Line 254: * ======= > Conflict was not resolved correctly. Done Line 293: * <<<<<<< HEAD > Conflict was not resolved correctly. Done https://asterix-gerrit.ics.uci.edu/#/c/558/10/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java: Line 731: throws MetadataException, RemoteException { > Please, add a failing test case for this one. They are already in this review. I have one for dropping a type used within the same dataverse and one for dropping a type used by another dataverse Line 743: throws MetadataException, RemoteException { > This method has a bunch of issues: 1. This code change doesn't allow types to use foreign types, but only datasets to use foreign types, so it's not an issue yet 2. Added a TODO 3. I did add a test case. Once again, types are still not allowed to use foreign subtypes 4. They will always be the same for now. https://asterix-gerrit.ics.uci.edu/#/c/558/10/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java File asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java: Line 2055: * > white space. Done -- To view, visit https://asterix-gerrit.ics.uci.edu/558 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I24dbc04dcb2a4126fc8361ebe3104877a0d1f2bb Gerrit-PatchSet: 10 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
