Yingyi Bu has posted comments on this change. Change subject: Support Change Feeds and Ingestion of Records with MetaData ......................................................................
Patch Set 11: (23 comments) https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java File asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java: Line 38: import org.apache.asterix.optimizer.rules.MetaFunctionToMetaVariableRule; It seems the imports are not lexically ordered. Format the code? https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/MetaFunctionToMetaVariableRule.java File asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/MetaFunctionToMetaVariableRule.java: Line 49: private boolean hasApplied = false; Remove unnecessary finals. Line 74: final DataSourceScanOperator scanOp = (DataSourceScanOperator) op; remove unnecessary finals. Line 250: this.fieldAccessExpressions = fieldAccessExpressions; it's no longer field-access expressions, right? https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java File asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: Line 91: import org.apache.asterix.optimizer.rules.UnnestToDataScanRule; translator shouldn't depend on a rewriting rule. Line 196: "Unable to load dataset " + clffs.getDatasetName() + " since it is a read only dataset"); "read only" is not entirely correct. "Unable to load dataset " + clffs.getDatasetName() + " since it is a read only dataset"); -> "Unable to load dataset " + clffs.getDatasetName() + " since a dataset with the meta part is only supported by feed ingestion."); Line 228: UnnestToDataScanRule.prepareVarAndExpression(keyFieldName, payloadVar, pkVars, pkExprs, varRefsForLoading, Move that method to a util class (in org.apache.asterix.translator.util) from UnnestToDataScanRule. Line 253: UnnestToDataScanRule.prepareVarAndExpression(additionalFilteringField, payloadVar, additionalFilteringVars, Move that method to a util class (in org.apache.asterix.translator.util) from UnnestToDataScanRule. Line 374: + targetDatasource.getDataset().getDatasetName() + " since it a read only dataset"); Change the wording, similar to load. Line 387: + targetDatasource.getDataset().getDatasetName() + " since it a read only dataset"); Change the wording, similar to load. Line 408: + targetDatasource.getDataset().getDatasetName() + " since it a read only dataset"); Change the wording, similar to load. Line 449: project.getVariables().add(metaAndKeysAssign.getVariables().get(0)); "metaAndKeysAssign.getVariables().get(0)" --> metaVar Line 455: new ArrayList<Mutable<ILogicalExpression>>()); Why this empty assign is needed? Line 464: UnnestToDataScanRule.replaceMetaWithMetaKey(assignExpr); In principle, the translator shouldn't have dependency to rewriting rules... https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java File asterix-app/src/test/java/org/apache/asterix/test/optimizer/OptimizerTest.java: Line 142: final String queryFileShort = queryFile.getPath().substring(PATH_QUERIES.length()) remove unnecessary final. https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-app/src/test/resources/runtimets/queries/feeds/feeds_02/feeds_02.1.ddl.aql File asterix-app/src/test/resources/runtimets/queries/feeds/feeds_02/feeds_02.1.ddl.aql: Line 20: * Description : Create a feed dataset that uses the feed simulator adapter. WS Line 21: Begin ingestion and verify contents of the dataset post completion. WS https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-app/src/test/resources/runtimets/testsuite.xml File asterix-app/src/test/resources/runtimets/testsuite.xml: Line 128: <!-- WS https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java File asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java: Line 26: public void start(IFrameWriter writer) throws HyracksDataException; I guess stop(), pause(), and resume() probably would better present in the interface, so that conceptually IDataflowController can have a full lifecycle. Otherwise, I'm not sure what is the contract of the interface and how the interface will be useful if it can only "start". https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataParserFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataParserFactory.java: Line 60: public void setMetaType(ARecordType metaType); Do we really need the two set methods? setRecordType and setMetaType? I assume both of them won't change for a specific IDataParserFactory instance. It would be nice to keep them as immutable members, and hence not part of the interface. One can only pass them in the constructors. This probably will make the code easier to understand (for me). https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-external-data/src/main/java/org/apache/asterix/external/provider/LookupReaderFactoryProvider.java File asterix-external-data/src/main/java/org/apache/asterix/external/provider/LookupReaderFactoryProvider.java: Line 34: final String inputFormat = HDFSUtils.getInputFormatClassName(configuration); unnecessary finals. https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-external-data/src/main/java/org/apache/asterix/external/provider/ParserFactoryProvider.java File asterix-external-data/src/main/java/org/apache/asterix/external/provider/ParserFactoryProvider.java: Line 41: final String parserFactoryName = configuration.get(ExternalDataConstants.KEY_DATA_PARSER); unnecessary final https://asterix-gerrit.ics.uci.edu/#/c/621/11/asterix-installer/src/test/resources/integrationts/library/queries/library-feeds/feed_ingest/feed_ingest.1.ddl.aql File asterix-installer/src/test/resources/integrationts/library/queries/library-feeds/feed_ingest/feed_ingest.1.ddl.aql: Line 55: create dataset TweetsFeedIngest(TweetOutputType) WS -- To view, visit https://asterix-gerrit.ics.uci.edu/621 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: If136a03d424970132dfb09f0dda56e160d4c0078 Gerrit-PatchSet: 11 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ildar Absalyamov <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
