Till Westmann 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? 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? 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/ 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 that's worth it, though. 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 the exception wrapping in evaluate and save some exception wrapping in subclasses (and utility classes like StringTrimmer). 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/ ? 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/ ? 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(); > MAJOR SonarQube violation: That's indeed a good point. Line 118: patternGenerator == null ? inputPatternString : patternGenerator Pull the conditional expression out of the then and the else branch? 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/ ? 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.appendUtf8StringPointable instead of going though 1 java char at a time? -- 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-HasComments: Yes
