abdullah alamoudi has posted comments on this change.

Change subject: [ASTERIXDB-2194][COMP] Introduce datasource functions
......................................................................


Patch Set 6:

(10 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroducePrimaryIndexForAggregationRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/IntroducePrimaryIndexForAggregationRule.java:

PS6, Line 184:  if (scanOperator instanceof DataSourceScanOperator) {
             :             DataSourceScanOperator dss = 
(DataSourceScanOperator) scanOperator;
             :             DataSource ds = (DataSource) dss.getDataSource();
             :             if (ds.getDatasourceType() != 
DataSource.Type.INTERNAL_DATASET) {
             :                 return false;
             :             }
             :         }
> While you are at it, look at line 276.
I don't think it can ever happen... If there is a dataset datasource with the 
dataset being null, then it should be caught earlier


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsDatasource.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsDatasource.java:

PS6, Line 19: optimizer
> It doesn't seem that this should be part of the optimizer.
Done


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsFunction.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsFunction.java:

PS6, Line 19: optimizer
> It doesn't seem that this should be part of the optimizer.
Done


PS6, Line 31: DatasetComponentsFunction
> The name is a little confusing, maybe we could be more explicit about the f
Done


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsReader.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/util/DatasetComponentsReader.java:

PS6, Line 19: optimizer
> This also seems to be a runtime component.
Done


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/dataset-resources/dataset-resources.3.query.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/dataset-resources/dataset-resources.3.query.sqlpp:

PS6, Line 20: Dataset
> It would be nice if we could use identifier resolution here.
I am not sure what that means


PS6, Line 20: dataset_resources
> I think that this should be an internal function until we decide on a more 
Done


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java
File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java:

PS6, Line 210: For function datasources
> Why do we have specific code for function datasources in the GenericAdapter
This is just to re-use as much of existing infrastructure as we can/should?

This is supposed to be used for internal function datasources since we don't 
create configurations and let the adapter factory figure out data source and 
data parser but instead, we create those and just set them here.

Thoughts?


https://asterix-gerrit.ics.uci.edu/#/c/2216/6/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/IFunctionToDataSourceTransformer.java
File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/IFunctionToDataSourceTransformer.java:

PS6, Line 27: IFunctionToDataSourceTransformer
> Is this a transformer or a rewriter?
Done


PS6, Line 42: AbstractFunctionCallExpression
> Can we get this from the operator?
Haha. Okay Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcf807ac713a21e8f4d59868525467386e801303
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to