Yingyi Bu has posted comments on this change.

Change subject: Add several builtin functions.
......................................................................


Patch Set 11:

(11 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch-sql-like/q19_discounted_revenue/q19_discounted_revenue.3.query.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/tpch-sql-like/q19_discounted_revenue/q19_discounted_revenue.3.query.sqlpp:

Line 29:     where ((p.p_partkey = l.l_partkey) and (((p.p_brand = 'Brand#12') 
and tpch.regexp_contains(p.p_container,'SM CASE||SM BOX||SM PACK||SM PKG') and 
(l.l_quantity >= 1) and (l.l_quantity <= 11) and (p.p_size >= 1) and (p.p_size 
<= 5) and tpch.regexp_contains(l.l_shipmode,'AIR||AIR REG') and 
(l.l_shipinstruct = 'DELIVER IN PERSON')) or ((p.p_brand = 'Brand#23') and 
tpch.regexp_contains(p.p_container,'MED BAG||MED BOX||MED PKG||MED PACK') and 
(l.l_quantity >= 10) and (l.l_quantity <= 20) and (p.p_size >= 1) and (p.p_size 
<= 10) and tpch.regexp_contains(l.l_shipmode,'AIR||AIR REG') and 
(l.l_shipinstruct = 'DELIVER IN PERSON')) or ((p.p_brand = 'Brand#34') and 
tpch.regexp_contains(p.p_container,'LG CASE||LG BOX||LG PACK||LG PKG') and 
(l.l_quantity >= 20) and (l.l_quantity <= 30) and (p.p_size >= 1) and (p.p_size 
<= 15) and tpch.regexp_contains(l.l_shipmode,'AIR||AIR REG') and 
(l.l_shipinstruct = 'DELIVER IN PERSON'))))
> Can we/should we remove the dataverse from these function names?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppBuiltinFunctionRewriteVisitor.java
File 
asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppBuiltinFunctionRewriteVisitor.java:

Line 21: import static 
org.apache.asterix.lang.sqlpp.util.FunctionMapUtil.normalizedListInputFunctions;
> revert this file?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppListInputFunctionRewriteVisitor.java
File 
asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppListInputFunctionRewriteVisitor.java:

Line 33:  * This visitor rewrites severael variable-arg user-facing functions 
to their coressponding
> s/severael/several/
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractBinaryStringStringEval.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractBinaryStringStringEval.java:

Line 69:     private void writeResult(IPointable resultPointable) throws 
AlgebricksException {
> We could pull this up one level (if we had one level more) - not sure if th
The method is only for outputting strings, and uses the fields resultStrPtr 
defined in this class.


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractUnaryStringStringEval.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/AbstractUnaryStringStringEval.java:

Line 87:             throws AlgebricksException;
> If this would be allowed to throw IOException as well, then we could handle
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/StringTrim2Descriptor.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/StringTrim2Descriptor.java:

Line 59:                     private StringTrimer stringTrimer = new 
StringTrimer(resultBuilder, resultArray);
> s/stringTrimer/stringTrimmer/ ?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/StringTrimDescriptor.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/StringTrimDescriptor.java:

Line 59:                     private StringTrimer stringTrimer = new 
StringTrimer(resultBuilder, resultArray, " ");
> s/stringTrimer/stringTrimmer/ ?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/utils/RegExpMatcher.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/utils/RegExpMatcher.java:

Line 56:     private final StringBuffer resultBuf = new StringBuffer();
> That's indeed a good point.
Matcher.appendReplacement requires a StringBuffer.


Line 118:                         patternGenerator == null ? inputPatternString 
: patternGenerator
> Pull the conditional expression out of the then and the else branch?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/utils/StringTrimer.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/utils/StringTrimer.java:

Line 36: public class StringTrimer {
> s/StringTrimer/StringTrimmer/ ?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1104/11/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/UTF8StringPointable.java
File 
hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/UTF8StringPointable.java:

Line 563:         while (startIndex < endIndex) {
> Could we just copy the bytes here by using UTF8StringBuilder.appendUtf8Stri
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I26351af22f67d66b56176f55b29a4e7ff63583f7
Gerrit-PatchSet: 11
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Yingyi Bu <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to