DRILL-6763: Codegen optimization of SQL functions with constant values #1481

lushuifeng · 2018-09-26T04:06:15Z

Here is the descriptions of the change:
~~1. add system option exec.optimize_function_compilation to toggle to state of this functionality.~~
2. codegen is changed by declaring setter method in EvaluationVisitor#visitXXXconstants
3. the member declared in step2 is initialized when the instance of the class is created in XXBatch and others
4. attachment is the code of the same query mentioned in DRILL-6763 generated by setting the value of exec.optimize_function_compilation to true
@arina-ielchiieva @vdiravka Would you please take a look?
What kind of unit tests should be added?
query.txt

vvysotskyi · 2018-09-26T06:38:47Z

@lushuifeng, thanks for the contribution!

Could you please provide the time of execution of the query with and without your change?

Please note, that Drill has its own framework for scalar replacement and passing value holders to the methods as arguments may break a scalar replacement for those holders.

Also, Drill can generate too large classes. To avoid problems with the class constant pool overflow was implemented splitting the classes into the smaller ones. Is this change incorporates or at least does not break it?

lushuifeng · 2018-09-26T07:21:36Z

@vvysotskyi Just setters are added, I think others should work as before.

The setup time of Filter changes with or without my change, following my step:

toggle the state of exec.optimize_function_compilation
query with sql, this one has no cache(the code generated by different state of exec.optimize_function_compilation is not the same):
SELECT full_name, hire_date FROM cp.employee.json where last_name = 'Bernard' and hire_date >= '1990-01-01 00:00:00.0';
change the constants in sql, hire_date is changed:
SELECT full_name, hire_date FROM cp.employee.json where last_name = 'Bernard' and hire_date >= '1990-01-02 00:00:00.0';
see the profile of the second time in the above

vvysotskyi · 2018-09-26T08:52:09Z

@lushuifeng, thanks, looks great!

But could you please also provide results for JDK and Janino compilers with and without this feature.
And it would be good to know the results for other literals, except strings and more complex queries.

Are there any cases, where this feature breaks something, or cause performance degradation? If no, then no need to provide the option for disabling this functionality.

Regarding unit tests, it may be written considering compiler limitation for method size - somehow may be passed huge string literals, so query should fail without these changes. This test should be sufficient.

lushuifeng · 2018-09-26T09:22:44Z

@vvysotskyi other tests will be provided later.
This change have been tested in out prod env for a month, no failure and performance degradation are found as far as I know.
Is it better to provide the option to disable this feature by setting the default value to true?
Could you please give more details about the unit tests?

vvysotskyi · 2018-09-26T11:42:20Z

If this fix just improves the performance without any consequences, then there is no need to disable it or specify an option for that.
But in any case, it is useful to see numbers.

Regarding the unit test, generate a query with large string literals, so they will be specified in the methods in generated code, and query should fail with "code too large" compile exception.

lushuifeng · 2018-09-27T09:59:18Z

@vvysotskyi unit tests are added according to your advice.
What should I do due to the error in CI of drill-jdbc-all-1.15.0-SNAPSHOT.jar size (39000409) too large. Max. is 39000000 , the max size is changed temporarily, is it appropriate to change in the PR?
The option will be kept for a while to provide more sql tests

arina-ielchiieva · 2018-09-27T11:29:03Z

@lushuifeng regarding jdbc-all I guess we need to exclude unneeded libs from it to reduce the size or bump the size if there is nothing to exclude. I believe this effects all new PRs so this should be done in separate Jira prior to new PRs.

vvysotskyi · 2018-09-27T12:01:20Z

@lushuifeng, jdbc-all jar size will be reduced in the scope of #1168.
@sachouche, what is the status of that PR?

sachouche · 2018-09-27T15:50:16Z

PR #1168 is ready; the remaining part is some advanced testing as requested by @parthchandra (using SQL clients) to ensure that all runtime dependencies are there. It seems the Drill internal unit tests do not provide such guarantees. I didn't have the cycles to perform such testing.

vvysotskyi

@lushuifeng, have you run all unit tests with your fix since Travis runs some of them?

vvysotskyi · 2018-10-03T10:13:17Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -135,6 +140,7 @@ public static MappingSet getDefaultMapping() {
    this.evaluationVisitor = eval;
    this.model = model;
    this.optionManager = optionManager;
+    setters = Maps.newHashMap();


Please replace with the constructor.

vvysotskyi · 2018-10-04T09:27:19Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java

-      generator.getEvalBlock().assign(out.getValue(), JExpr.lit(e.getLong()));
-      return out;
+      MajorType majorType = e.getMajorType();
+      JType holderType = generator.getHolderType(majorType);


Common logic from these methods can be moved to the separate one.

vvysotskyi · 2018-10-04T09:34:01Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java

-      return new HoldingContainer(majorType, var, null, null);
+      generator.declareSetterMethod(
+        var,
+        buffer -> ValueHolderHelper.getVarDecimalHolder(buffer, e.getBigDecimal().toString()));


My comment is not connected to this change, but it would be good to change ValueHolderHelper.getVarDecimalHolder method to take BigDecimal instance instead of String since in all places, where it is used, the string is obtained from the BigDecimal and inside this method BigDecimal is created using this string.

vvysotskyi · 2018-10-04T09:37:21Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java

-      return new HoldingContainer(majorType, var, null, null);
+      generator.declareSetterMethod(
+        var,
+        buffer -> ValueHolderHelper.getVarCharHolder(buffer, e.value));


Please replace e.value with e.getString().

vvysotskyi · 2018-10-04T09:58:57Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ValueVectorHashHelper.java

@@ -72,6 +73,7 @@ public Hash64 getHash64(LogicalExpression[] hashFieldsExp, TypedFieldId[] hashFi
    ClassGenerator<Hash64> cg = codeGenerator.getRoot();
    setupBuild64Hash(cg, GetHashIncomingBuildColMapping, recordBatch, hashFieldsExp, hashFieldIds);
    Hash64 hash64 = context.getImplementationClass(codeGenerator);
+    CodeGenMemberInjector.injectMembers(cg, hash64, context);


This method call should be added to every place where context.getImplementationClass() method is called. It would be good to move injectMembers inside of getImplementationClass() if it is possible or somehow combine them.

vvysotskyi · 2018-10-04T10:23:15Z

...ava-exec/src/main/java/org/apache/drill/exec/physical/impl/common/CodeGenMemberInjector.java

+          Class.forName(method.params().get(0).type().binaryName()));
+        setMethod.invoke(instance, setter.getValue().apply(context.getManagedBuffer()));
+      } catch (Exception e) {
+        Throwables.propagate(e);


Throwables.propagate() is deprecated. Please replace it with the combination of Throwables.throwIfUnchecked(e) and new RuntimeException(e) as it is recommended in its javadoc.

But anyway, is it necessary to wrap unchecked exceptions into RuntimeException? If no, then only ReflectiveOperationException may be specified in the catch block and wrapped into RuntimeException.

vvysotskyi · 2018-10-04T10:42:23Z

exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java

+  @Test
+  public void testHugeStringConstantCompilation() throws Exception {
+    try {
+      testNoResult("alter session set `%s`='JDK'", ClassCompilerSelector.JAVA_COMPILER_OPTION);


Please replace with setSessionOption() here and below.

vvysotskyi · 2018-10-04T10:50:37Z

exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java

+    StringBuilder sb = new StringBuilder("select *\n")
+      .append("from cp.`employee.json`\n")
+      .append("where last_name ='")
+      .append(longText).append("'");


Please move the second append to the new line.

lushuifeng · 2018-10-04T12:56:09Z

@vvysotskyi thanks for your suggestions.
I didn't know that Travis only run some of the test.
I'm sorry I've not run all the tests yet, I would like to run them as soon as possible

lushuifeng · 2018-10-08T06:44:30Z

@vvysotskyi thanks for your comments. Changes are made as you suggested.
As you mentioned early, some tests are made to test this functionality:
Both jdk and janino compiler works fine.
Constants with different types(string, double, long, decimal, time, timestamp and so on) are tested.
But some tests beside the Travis are not passed, a lot of No default schema selected errors occured, it seems that these errors are not connected to this change.

vvysotskyi

@lushuifeng, did you use mvn clean install to run all the tests?

vvysotskyi · 2018-10-08T08:57:10Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java

-      HoldingContainer out = generator.declare(e.getMajorType());
-      generator.getEvalBlock().assign(out.getValue(), JExpr.lit(e.getLong()));
-      return out;
+      JVar var = declareClassField(generator, e.getMajorType(), "long");


I meant to move all this logic to the separate method and pass ClassGenerator, MajorType and Function, for example, to receive something like this:

@Override public HoldingContainer visitLongConstant(LongExpression e, ClassGenerator<?> generator) throws RuntimeException { return getHoldingContainer(generator, e.getMajorType(), buffer -> ValueHolderHelper.getBigIntHolder(e.getLong())); } private HoldingContainer getHoldingContainer(ClassGenerator<?> generator, MajorType majorType,Function<DrillBuf, ? extends ValueHolder> function) { JType holderType = generator.getHolderType(majorType); JVar var = generator.declareClassField("value", holderType); generator.declareSetterMethod( var, function); return createHoldingContainer(majorType, var); }

vvysotskyi · 2018-10-08T08:59:12Z

exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestLargeFileCompilation.java

@@ -228,4 +247,14 @@ public void testNestedLoopJoin() throws Exception {
      testNoResult("drop table if exists %s", tableName);
    }
  }
+
+  @Test
+  public void testHugeStringConstantCompilation() throws Exception {


Please add a unit test for Janino also.

...ava-exec/src/main/java/org/apache/drill/exec/physical/impl/common/CodeGenMemberInjector.java

lushuifeng · 2018-10-08T10:15:52Z

@vvysotskyi Sorry for my misunderstanding.
yes, simply ran with mvn -T 8 clean install, the tests cann't be passed.

vvysotskyi

I'm not sure that tests will pass with -T 8, but they should pass with the default threads count.

vvysotskyi · 2018-10-08T10:49:58Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -536,6 +543,19 @@ public JVar declareClassField(String prefix, JType t, JExpression init) {
    return clazz.field(JMod.NONE, t, prefix + index++, init);
  }

+  public JMethod declareSetterMethod(JVar var, Function<DrillBuf, ? extends ValueHolder> function) {


Please add Javadoc here also.

lushuifeng · 2018-10-08T11:59:01Z

the tests without -T 8 still failed.
Could you please run one of my failed tests: org.apache.drill.exec.impersonation.TestImpersonationMetadata#testShowFilesInWSWithNoPermissionsForQueryUser

arina-ielchiieva · 2018-10-08T12:23:12Z

@lushuifeng a good practice to have clean run on master and then on the branch with your changes thus you will know the reason of the failures. Try to run tests on master first.

vvysotskyi · 2018-10-08T15:47:40Z

exec/jdbc-all/pom.xml

@@ -511,7 +511,7 @@
                  This is likely due to you adding new dependencies to a java-exec and not updating the excludes in this module. This is important as it minimizes the size of the dependency of Drill application users.

                  </message>
-                  <maxsize>39000000</maxsize>
+                  <maxsize>39100000</maxsize>


Please rebase onto the master and revert this change, since there was PR, were excluded some classes from the JAR.

…apache#1481)

lushuifeng · 2018-10-09T05:34:13Z

@arina-ielchiieva thanks for your advice.
The tests are not passed with mvn clean install on master, got the same errors as on branch DRILL-6763.
Single Test TestImpersonationMetadata#testShowFilesInWSWithNoPermissionsForQueryUser failed as on the branch DRILL-6763.

vvysotskyi · 2018-10-09T09:57:23Z

This fix causes the next unit tests failures:

Failed tests: 
  TestLargeFileCompilation.testJDKHugeStringConstantCompilation:257->BaseTestQuery.resetSessionOption:375 Failed to reset session option `exec.java_compiler`, Error: org.apache.drill.common.exceptions.UserException: SYSTEM ERROR: IllegalArgumentException: Attempted to send a message when connection is no longer valid.

Query submission to Drillbit failed.

[Error Id: 6a397072-825d-4550-a39e-ea4f6eafdc6d ]

Tests in error: 
  TestLargeFileCompilation.testProject:191->BaseTestQuery.testNoResult:384 » Rpc
  TestLargeFileCompilation.testTOP_N_SORT:177->BaseTestQuery.testNoResult:358->BaseTestQuery.testNoResult:384 » Rpc
  TestLargeFileCompilation.testEXTERNAL_SORT:171->BaseTestQuery.testNoResult:358->BaseTestQuery.testNoResult:384 » Rpc
  TestLargeFileCompilation.testPARQUET_WRITER:156->BaseTestQuery.testNoResult:358->BaseTestQuery.testNoResult:384 » Rpc
  TestLargeFileCompilation.testHashJoin:206->BaseTestQuery.testNoResult:358->BaseTestQuery.testNoResult:384 » Rpc
  TestHashJoin.testHashJoinExprInCondition:252 » Rpc org.apache.drill.common.exc...

Tests run: 3545, Failures: 1, Errors: 6, Skipped: 156

lushuifeng · 2018-10-09T10:22:52Z

There is no such tests on master, but also failed with the same errors on branch DRILL-6763.
Comment-out these two new tests and rerun all tests in TestLargeFileCompilation got the same error as above

vvysotskyi · 2018-10-09T12:29:50Z

@lushuifeng, since these tests failures are absent on the latest master, they caused by your changes. Could you please fix them? Also, some of the failures in TestLargeFileCompilation may be caused by the previously failed unit tests from this class.

lushuifeng · 2018-10-10T03:23:47Z

@vvysotskyi thanks for your testing, I'm working on this, it may take some time

lushuifeng · 2018-10-11T10:12:07Z

@vvysotskyi Could you please take a look at these changes?

commit 8b44957 fix the compilation error if there are nested splitting classes
commit 5edecc4 and 413891a fix the compilation error of the innerClasses like BatchHolder and OutgoingRecordBatch contain constant values

I've just found that previous errors in my tests mvn clean install are because of there is a DOT in my user name

vvysotskyi · 2018-10-11T10:32:02Z

@lushuifeng, I have a question regarding nested splitting classes: these setters will be added only to the top level class, or they will be separated between both: top and nested classes?

lushuifeng · 2018-10-11T11:09:43Z

Both top and nested classes will be added since the new class member is in inner class if there is a inner class, the inner class member is initialized by invoking top class member function:

top:
public void setConstant5(IntHolder intHolder) { this.innerClassField.setConstant5(intHolder) }
inner:
public void setConstant5(IntHolder intHolder) { this.constant5 = intHolder; }

vvysotskyi · 2018-10-11T11:29:46Z

@lushuifeng, then this change will break code compilation for the case when a lot of constants were used.

Previously, these constants were separated between the outer and nested classes.
But with these changes, additional items in the class constant pool will be used for setter methods[1], and in the outer class will be present all setters, so it will cause the overflow of the class constant pool.

[1] https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.6

lushuifeng · 2018-10-11T11:55:07Z

I agree with you, it will cause the overflow of the class constant pool in extreme case, that is to say a projection or a filter contains thousands (or maybe larger) of constants

The good news is all tests in TestLargeFileCompilation are succeeded with this change, shall we add an option to disable this functionality or maybe there is a good way to avoid?

I'd do some tests to find out how much constants will cause the overflow of the class constant pool.

vvysotskyi · 2018-10-11T13:08:37Z

I don't think that this is a good idea to add an option. In the case when we have a combination of these two problems, option won't help.

I think it is possible to enhance the way for collecting setter methods for example to map full [nested] class names and its setters, so after that receive nested class instances and call setters for them in CodeGenMemberInjector.injectMembers().

lushuifeng · 2018-10-12T09:28:49Z

In order to collect setter methods of nested class, some changes will be made:

public void setConstant4(IntHolder constant4) will be changed to public void setConstant4(IntHolder constant4, String name), the second param name is to store the name of the first param, here is the string "constant4", unfortunately the second param is useless unless there is a nested class.
methods will be collected during __DRILL_INIT__, Lambda is not supported in JCodeModel 2.6, it seems that anonymousClass in JCodeModel 2.6 can't be narrowed, the <ValueHolder, String> is missing. some casts have to be added

Map<String, BiConsumer<ValueHolder, String>> nestedClassFunctions = new HashMap();
this.nestedClassFunctions.put("constant355", new BiConsumer<ValueHolder, String>() {
     @Override
      public void accept(ValueHolder constant355, String name) {
         (innerClassField).setConstant355(((IntHolder) constant355), "constant355");
      }
 });

InnerClassField is referenced in anonymousClass, keyword FINAL should be added to its mods, so it should be initialized in constructor not in __DRILL_INIT__

What is your suggestions? @vvysotskyi thanks.

vvysotskyi · 2018-10-12T10:28:53Z

@lushuifeng, I meant to replace
Map<JMethod, Function<DrillBuf, ? extends ValueHolder>> setters
with something like this:
Map<JMethod, Pair<String, Function<DrillBuf, ? extends ValueHolder>>> setters,
where the String is the name of nested class field, or the name, which corresponds to the full path to this class, for example like this: nestedClassField.nestedNestedClassField.nestedNestedNestedClassField. So in the CodeGenMemberInjector.injectMembers(), required nested class instance may be received using its name and setter method may be called on it.

lushuifeng · 2018-10-12T12:47:56Z

I think it is the way I'm doing besides you wrap two params (DrillBuf and String) into a Pair for the first point. The method in CodeGenMemberInjector.injectMembers() will get two params instead of old one.
Have you tried about anonymousClass in JCodeModel 2.6, it is a little confused to me that the narrow function not work correctly.

lushuifeng · 2018-10-16T08:04:14Z

@vvysotskyi All tests have been passed, could you please take a look? thanks!

vvysotskyi · 2018-10-16T11:06:07Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

+   * @return the depth of nested class, setter method
+   */
+  public Pair<Integer, JMethod> declareSetterMethod(JVar var, Function<DrillBuf, ? extends ValueHolder> function) {
+    JMethod setter = clazz.method(JMod.PUBLIC, void.class, "set" + StringUtils.capitalize(var.name()));


Please move the initialization to the else block below and put code which uses this var after the if...else block.

Is It better by NOT declaring the setter function, these fields can be accessed by reflection directly, so the number of constants in constant pool can be reduced

vvysotskyi · 2018-10-16T11:23:00Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -646,7 +687,7 @@ public void preparePlainJava() {
        Class<?> p = params[i];
        childNew.arg(shim.param(model._ref(p), "arg" + i));
      }
-      shim.body()._return(childNew);
+      shim.body()._return(JExpr._this().invoke("injectMembers").arg(childNew));


Could you please explain what is the reason for this change?

The innerClasses like BatchHolder may have the constants, BatchHolder instance is created via newBatchHolder dynamically when processing recordBatch, that is the difference between innerClasses and nested splitting classes.

The innerClasses member fields is initialized by invoking injectMembers(innerClassInstance) which is override generated by this code as shown in
protected BatchHolder newBatchHolder(int batchRowCount) { return this.injectMembers(new BatchHolder(batchRowCount)); }

vvysotskyi · 2018-10-16T11:29:37Z

exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java

+  }
+
+  protected BatchHolder injectMembers(BatchHolder batchHolder) {
+    CodeGenMemberInjector.injectMembers(cg, batchHolder, context);


Is it possible to move this method call to the place, general for other similar classes?

The ClassGenerator and FragmentContext is needed in this method for general cases, but not for test classes, see example in ExampleTemplateWithInner.java

newBatchHolder is override by generated code can be implemented as follows to remove above method
protected BatchHolder newBatchHolder(int batchRowCount) { return CodeGenMemberInjector.injectMembers(cg, new BatchHolder(batchRowCount), context); }
but the cg and context params are hard coded, not flexable. I think it is better to make a function call to hide the implementation which may differ in each template.

vvysotskyi · 2018-10-16T11:32:51Z

...ava-exec/src/main/java/org/apache/drill/exec/physical/impl/common/CodeGenMemberInjector.java

+   */
+  public static void injectMembers(ClassGenerator<?> cg, Object instance, FragmentContext context) {
+    Map<Integer, Object> cachedInstances = new HashMap<>();
+    for (Map.Entry<Pair<Integer, JMethod>, Function<DrillBuf, ? extends ValueHolder>> setter : cg.getSetters().entrySet()) {


We may change the type of collection which used for storing setter methods. Can be used Guava's Table, so we will be able to omit usage of cachedInstances map.

Sorry I don't quite follow you since the instance is created somewhere else not in setter collection

vvysotskyi · 2018-10-16T11:47:38Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -87,6 +95,8 @@
  private LinkedList<SizedJBlock>[] blocks;
  private LinkedList<SizedJBlock>[] oldBlocks;

+  private JVar innerClassField;
+
  /**
   * Assumed that field has 3 indexes within the constant pull: index of the CONSTANT_Fieldref_info +


Please note that with introducing new methods in the class, this formula should be revised to take them into account.

lushuifeng · 2018-10-18T10:11:39Z

@vvysotskyi setter method have been removed, the formula of maxIndex should be same as before.

DRILL-6722 seems to be resolved with this change by accident, the root cause is not found yet.

Test TestLargeFileCompilation#testProject has 10% improvement in the running time compare to master without cached classes.

lushuifeng · 2018-10-22T10:00:44Z

@vvysotskyi Could you please take a look, thanks.

…apache#1481)

vvysotskyi

@lushuifeng, overall looks good, just single comment.

Please squash commits and rebase onto the master.

vvysotskyi · 2018-10-22T11:05:02Z

exec/java-exec/src/main/java/org/apache/drill/exec/expr/ClassGenerator.java

@@ -151,7 +161,7 @@ public static MappingSet getDefaultMapping() {
      // from the JDK Modifier values to the JCodeModel JMod values: the
      // values are different.

-      int mods = JMod.PRIVATE + JMod.FINAL;
+      int mods = JMod.PUBLIC + JMod.FINAL;


Are this change and change below required for the current approach (without using setter methods for inner classes)?

…apache#1481)

lushuifeng · 2018-10-23T04:19:07Z

done

vvysotskyi

Thanks for fixing this issue, +1

lushuifeng · 2018-10-23T07:42:24Z

Thank you for your review and comments.

…apache#1481) closes apache#1481

vvysotskyi reviewed Oct 4, 2018

View reviewed changes

vvysotskyi reviewed Oct 8, 2018

View reviewed changes

lushuifeng force-pushed the DRILL-6763 branch from 72e82bf to 1b2705e Compare October 9, 2018 02:21

lushuifeng added a commit to lushuifeng/drill that referenced this pull request Oct 9, 2018

DRILL-6763: Codegen optimization of SQL functions with constant values(…

1b2705e

…apache#1481)

vvysotskyi reviewed Oct 16, 2018

View reviewed changes

vvysotskyi pushed a commit to vvysotskyi/drill that referenced this pull request Oct 22, 2018

DRILL-6763: Codegen optimization of SQL functions with constant values(…

9fd9b1a

…apache#1481)

vvysotskyi reviewed Oct 22, 2018

View reviewed changes

lushuifeng force-pushed the DRILL-6763 branch from ceeebba to c9abbf7 Compare October 23, 2018 02:59

lushuifeng added a commit to lushuifeng/drill that referenced this pull request Oct 23, 2018

DRILL-6763: Codegen optimization of SQL functions with constant values(…

c9abbf7

…apache#1481)

DRILL-6763: Codegen optimization of SQL functions with constant values(…

cf896b1

…apache#1481)

lushuifeng force-pushed the DRILL-6763 branch from c9abbf7 to cf896b1 Compare October 23, 2018 03:22

vvysotskyi approved these changes Oct 23, 2018

View reviewed changes

Ben-Zvi closed this in 18e09a1 Oct 28, 2018

mattpollack pushed a commit to mattpollack/drill that referenced this pull request Feb 25, 2019

DRILL-6763: Codegen optimization of SQL functions with constant values(…

b34da71

…apache#1481) closes apache#1481

DRILL-6763: Codegen optimization of SQL functions with constant values #1481

DRILL-6763: Codegen optimization of SQL functions with constant values #1481

Conversation

lushuifeng commented Sep 26, 2018 • edited

vvysotskyi commented Sep 26, 2018 • edited

lushuifeng commented Sep 26, 2018

vvysotskyi commented Sep 26, 2018

lushuifeng commented Sep 26, 2018

vvysotskyi commented Sep 26, 2018 • edited

lushuifeng commented Sep 27, 2018

arina-ielchiieva commented Sep 27, 2018

vvysotskyi commented Sep 27, 2018

sachouche commented Sep 27, 2018

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lushuifeng commented Oct 4, 2018

lushuifeng commented Oct 8, 2018 • edited

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lushuifeng commented Oct 8, 2018

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lushuifeng commented Oct 8, 2018

arina-ielchiieva commented Oct 8, 2018

Choose a reason for hiding this comment

lushuifeng commented Oct 9, 2018 • edited

vvysotskyi commented Oct 9, 2018

lushuifeng commented Oct 9, 2018

vvysotskyi commented Oct 9, 2018

lushuifeng commented Oct 10, 2018

lushuifeng commented Oct 11, 2018

vvysotskyi commented Oct 11, 2018

lushuifeng commented Oct 11, 2018

vvysotskyi commented Oct 11, 2018

lushuifeng commented Oct 11, 2018

vvysotskyi commented Oct 11, 2018

lushuifeng commented Oct 12, 2018

vvysotskyi commented Oct 12, 2018 • edited

lushuifeng commented Oct 12, 2018

lushuifeng commented Oct 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lushuifeng commented Oct 18, 2018

lushuifeng commented Oct 22, 2018

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lushuifeng commented Oct 23, 2018

vvysotskyi left a comment

Choose a reason for hiding this comment

lushuifeng commented Oct 23, 2018

lushuifeng commented Sep 26, 2018 •

edited

vvysotskyi commented Sep 26, 2018 •

edited

vvysotskyi commented Sep 26, 2018 •

edited

lushuifeng commented Oct 8, 2018 •

edited

lushuifeng commented Oct 9, 2018 •

edited

vvysotskyi commented Oct 12, 2018 •

edited