[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-432132394 Thank you for your review and comments. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-432085516 done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-430953524 @vvysotskyi setter method have been removed, the formula of `maxIndex` should be same as before. [DRILL-6722](https://issues.apache.org/jira/projects/DRILL/issues/DRILL-6722?filter=allopenissues=created+DESC%2C+priority+DESC%2C+updated+DESC) 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. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-430141004 @vvysotskyi All tests have been passed, could you please take a look? thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-429313733 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. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL
functions with constant values
URL: https://github.com/apache/drill/pull/1481#issuecomment-429264607
In order to collect setter methods of nested class, some changes will be
made:
1. `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.
2. 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 is missing. some casts have to be
added
> Map> nestedClassFunctions =
new HashMap();
> this.nestedClassFunctions.put("constant355", new
BiConsumer() {
> @Override
> public void accept(ValueHolder constant355, String name) {
> (innerClassField).setConstant355(((IntHolder) constant355),
"constant355");
> }
> });
3. 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.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-428927289 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. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL
functions with constant values
URL: https://github.com/apache/drill/pull/1481#issuecomment-428915244
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;
}`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-428900022 @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](https://issues.apache.org/jira/browse/HADOOP-7050) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-428425133 @vvysotskyi thanks for your testing, I'm working on this, it may take some time This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-428140346 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 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-428065577 @arina-ielchiieva thanks for your advice. The tests are not passed with `mvn clean install` on master, got the same errors on branch DRILL-6763. Single Test `TestImpersonationMetadata#testShowFilesInWSWithNoPermissionsForQueryUser` failed as on the branch DRILL-6763. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-427806636 the tests without `-T 8` still failed. Could you please run one of my failed tests: `org.apache.drill.exec.impersonation.TestImpersonationMetadata#testShowFilesInWSWithNoPermissionsForQueryUser` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-427783274 @vvysotskyi Sorry for my misunderstanding. yes, simply ran with `mvn -T 8 clean install`, the tests cann't be passed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-427734567 @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 janio 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. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-427007830 @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 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-425032585 @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 3900` , 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 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-424647113 @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? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values URL: https://github.com/apache/drill/pull/1481#issuecomment-424612233 @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: 1. toggle the state of exec.optimize_function_compilation 2. 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';` 3. 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';` 4. see the profile of the second time in the above This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
