Till Westmann has posted comments on this change.

Change subject: Add Default Dataverse and AnyObject Datatype
......................................................................


Patch Set 6:

(5 comments)

I think that we should not remove the expected AST. The tests only continue to 
work after the removal, because the result comparison in ParserTestExecutor is 
broken. If I apply the patch from https://asterix-gerrit.ics.uci.edu/#/c/1184 
to this branch I get 15 failures that I don't get be applying the patch to 
master. So I think that the existing ASTs should be kept (and be put back to 
use by fixing the test framework ASAP).

https://asterix-gerrit.ics.uci.edu/#/c/1174/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java:

Line 1186:                     MetadataBuiltinEntities.DEFAULT_DATAVERSE_NAME + 
" dataverse can't be dropped");
That's right, but we need to have a way to clean it up. Please create an issue 
for this.


https://asterix-gerrit.ics.uci.edu/#/c/1174/6/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/SmokeParserTest.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/sqlpp/SmokeParserTest.java:

Line 51:     private static final String PATH_ACTUAL = "target" + 
File.separator + "parserts" + SEPARATOR;
Can we cleanup the .gitignore now?


https://asterix-gerrit.ics.uci.edu/#/c/1174/6/asterixdb/asterix-app/src/test/resources/runtimets/queries/types/any-object/any-object.2.update.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/types/any-object/any-object.2.update.aql:

Line 18:  */
Do we need this file?


https://asterix-gerrit.ics.uci.edu/#/c/1174/6/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java:

Line 326:             throw new MetadataException("Unable to instantiate 
builtin Adapter", e);
Do we have a test for this?


Line 338:             throw new MetadataException("Unable to instantiate 
builtin Merge Policy Factory", e);
Do we have a test for this?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1174
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie02c74c8b005dce73888afd4dfe19ede2c60a8e8
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to