Hussain Towaileb has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/3360 )

Change subject: [WIP] Introduce data generator data source functions
......................................................................


Patch Set 6:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorDatasource.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorDatasource.java:

https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorDatasource.java@30
PS6, Line 30: random
> Could we take the word "random" out again? It shouldn't be really random :)
Done in all classes (Change not uploaded yet).


https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java:

https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java@116
PS6, Line 116: IllegalArgumentException
> We should have a better error here. Providing a non-existing table name cur
Indeed, I put that as a temporary exception.
I'm currently searching for the best one to use.

Also, the message needs to be moved to the properties/translation file thingie 
so we don't have hardcoded messages, correct?


https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorRewriter.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorRewriter.java:

https://asterix-gerrit.ics.uci.edu/#/c/3360/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorRewriter.java@101
PS6, Line 101:     private double getScalingFactor(AsterixConstantValue 
scalingFactorArgumentValue) throws TypeMismatchException {
> This should be equivalent (and much shorter):
Done. (Change not uploaded yet)

I will check around to see if we have such a utility. I checked the Sum 
function which has a similar thing, it is handling it manually as well. Maybe I 
should create a getDoubleFromNumberIATypes() in one of the Util classes.

I also created a separate small method for each one to silence the SonarQube. 
(for now)



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

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb6bd6f697628395c70008e6f730bc5ca403da5e
Gerrit-Change-Number: 3360
Gerrit-PatchSet: 6
Gerrit-Owner: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Anon. E. Moose (1000171)
Gerrit-Reviewer: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Comment-Date: Sun, 28 Apr 2019 06:59:39 +0000
Gerrit-HasComments: Yes

Reply via email to