date:20200326

[GitHub] [calcite] DonnyZone commented on issue #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

DonnyZone commented on issue #1862: [CALCITE-3864] Implement Concat function
URL: https://github.com/apache/calcite/pull/1862#issuecomment-604812196
 
 
   In [CALCITE-3235](https://issues.apache.org/jira/browse/CALCITE-3235), 
`CONCAT` function with var args is added but not implemented (i.e., the query 
can only pass validation).
   This PR focuses on:
   (1) adding Oracle's `CONCAT` function, it accepts only two operands
   (2) implementing `CONCAT` function in runtime
   (3) fixing the return type inference issue
   
   Personally, I think "Implement CONCAT function" is ok. 
   @chunweilei @wenhuitang Do you have any suggestions to cover the topics 
above?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on issue #1876: [CALCITE-3878] Make ArrayList creation with initial capacity when size is fixed

2020-03-26 Thread GitBox

neoremind commented on issue #1876: [CALCITE-3878] Make ArrayList creation with 
initial capacity when size is fixed
URL: https://github.com/apache/calcite/pull/1876#issuecomment-604806498
 
 
   @chunweilei Thanks for reminding as well. I have replied Julian in JIRA.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] liyafan82 commented on a change in pull request #1873: [CALCITE-3872] Simplify expressions with unary minus

2020-03-26 Thread GitBox

liyafan82 commented on a change in pull request #1873: [CALCITE-3872] Simplify 
expressions with unary minus
URL: https://github.com/apache/calcite/pull/1873#discussion_r399025357
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
 ##
 @@ -2265,8 +2265,8 @@ private void assertTypeAndToString(
 checkSimplify(coalesce(gt(nullInt, nullInt), trueLiteral),
 "true");
 checkSimplify(coalesce(unaryPlus(nullInt), unaryPlus(vInt())),
-"+(?0.int0)");
-checkSimplifyUnchanged(coalesce(unaryPlus(vInt(1)), unaryPlus(vInt(;
+"?0.int0");
 
 Review comment:
   Sounds good. I have added some test cases for decimal. Please take a look.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1862: [CALCITE-3864] 
Implement Concat function
URL: https://github.com/apache/calcite/pull/1862#discussion_r399014279
 
 

 ##
 File path: 
core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
 ##
 @@ -2169,6 +2173,31 @@ protected static Calendar getCalendarNotTooNear(int 
timeUnit) {
 "VARCHAR(5) NOT NULL");
 tester.checkNull("x'ff' || cast(null as varbinary)");
 tester.checkNull(" cast(null as ANY) || cast(null as ANY) ");
+tester.checkString("cast('a' as varchar) || cast('b' as varchar) "
++ "|| cast('c' as varchar)", "abc", "VARCHAR NOT NULL");
+
+final SqlTester tester1 = tester(SqlLibrary.MYSQL);
+final SqlTester tester2 = tester(SqlLibrary.POSTGRESQL);
+final SqlTester tester3 = tester(SqlLibrary.ORACLE);
+for (SqlTester sqlTester: ImmutableList.of(tester1, tester2)) {
+  sqlTester.setFor(SqlLibraryOperators.CONCAT_FUNCTION);
+  sqlTester.checkString("concat('a', 'b', 'c')", "abc",
+  "VARCHAR(3) NOT NULL");
+  sqlTester.checkString("concat(cast('a' as varchar), cast('b' as 
varchar), "
+  + "cast('c' as varchar))", "abc", "VARCHAR NOT NULL");
+  sqlTester.checkNull("concat('a', 'b', cast(null as char(2)))");
+  sqlTester.checkNull("concat(cast(null as ANY), 'b', cast(null as 
char(2)))");
+}
+tester3.setFor(SqlLibraryOperators.CONCAT2);
+tester3.checkString("concat(cast('fe' as char(2)), cast('df' as 
varchar(65535)))",
+"fedf", "VARCHAR NOT NULL");
+tester3.checkString("concat(cast('fe' as char(2)), cast('df' as varchar))",
 
 Review comment:
   Please add a test case whose parameter is empty.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] hsyuan commented on issue #1876: [CALCITE-3878] Make ArrayList creation with initial capacity when size is fixed

2020-03-26 Thread GitBox

hsyuan commented on issue #1876: [CALCITE-3878] Make ArrayList creation with 
initial capacity when size is fixed
URL: https://github.com/apache/calcite/pull/1876#issuecomment-604794008
 
 
   I didn't notice it. Thanks for reminding.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1866: [CALCITE-3726] Documentation for Declaring Objects For Types Defined …

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1866: [CALCITE-3726] 
Documentation for Declaring Objects For Types Defined …
URL: https://github.com/apache/calcite/pull/1866#discussion_r399013717
 
 

 ##
 File path: site/_docs/reference.md
 ##
 @@ -2804,6 +2805,44 @@ To enable, include `calcite-server.jar` in your class 
path, and add
 to the JDBC connect string (see connect string property
 [parserFactory]({{ site.apiRoot 
}}/org/apache/calcite/config/CalciteConnectionProperty.html#PARSER_FACTORY)).
 
+
+
+ Declaring Objects For Types Defined In Schema
+After an object type is defined and installed in the schema, you can use it to 
declare objects in any SQL block. For example, you can use the object type to 
specify the datatype of an attribute, column, variable, bind variable, record 
field, table element, formal parameter, or function result. At run time, 
instances of the object type are created; that is, objects of that type are 
instantiated. Each object can hold different values.
+
+Example: For declared types `address_typ` and `employee_typ`
+```SQL
+CREATE TYPE address_typ AS OBJECT (
+   street  VARCHAR2(30),
+   cityVARCHAR2(20),
+   state   CHAR(2),
+   postal_code VARCHAR2(6) );
+
+CREATE TYPE employee_typ AS OBJECT (
+  employee_id   NUMBER(6),
+  first_nameVARCHAR2(20),
+  last_name VARCHAR2(25),
+  email VARCHAR2(25),
+  phone_number  VARCHAR2(20),
+  hire_date DATE,
+  job_idVARCHAR2(10),
+  salaryNUMBER(8,2),
+  commission_pctNUMBER(2,2),
+  manager_idNUMBER(6),
+  department_id NUMBER(4),
+  address   address_typ
+);
+```
+
+We can declare objects of type `employee_typ` and `address_typ` :
+
+```SQL
+employee_typ(315, 'Francis', 'Logan', 'FLOGAN',
+'555.777.', '01-MAY-04', 'SA_MAN', 11000, .15, 101, 110,
+ address_typ('376 Mission', 'San Francisco', 'CA', '94222'))
+```
+
 
 Review comment:
   It seems not a valid SQL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL 
UNCOMPRESS function support
URL: https://github.com/apache/calcite/pull/1865#discussion_r399011539
 
 

 ##
 File path: 
core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
 ##
 @@ -5192,6 +5192,25 @@ private void checkNullOperand(SqlTester tester, String 
op) {
 "0700789c4bad48cc2dc84905000bc002ed", "VARBINARY NOT NULL");
   }
 
+  @Test public void testUncompress() {
+SqlTester sqlTester = tester(SqlLibrary.MYSQL);
+sqlTester.checkNull("UNCOMPRESS(NULL)");
+sqlTester.checkString("UNCOMPRESS(x'')", "", "VARCHAR");
+
+sqlTester.checkNull("UNCOMPRESS(x'1233')");
+
+sqlTester.checkString("UNCOMPRESS(COMPRESS('test'))",
+"test", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'1000789c4b4c44050033980611')",
+"", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'0600789c2b4ecc2dc849050008de0283' 
)",
+"sample", "VARCHAR");
+
sqlTester.checkString("UNCOMPRESS(x'0700789c4bad48cc2dc84905000bc002ed')",
+"example", "VARCHAR");
+  }
 
 Review comment:
   Done :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1870: [CALCITE-3846] EnumerableMergeJoin: wrong comparison of composite key with null values

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1870: [CALCITE-3846]  
EnumerableMergeJoin: wrong comparison of composite key with null values
URL: https://github.com/apache/calcite/pull/1870#discussion_r399011127
 
 

 ##
 File path: 
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableJoinTest.java
 ##
 @@ -220,6 +220,44 @@
 + "empid=150; name=Sebastian; dept_name=Sales; e_deptno=10; 
d_deptno=10");
   }
 
+  /** Test case for
+   * https://issues.apache.org/jira/browse/CALCITE-3846;>[CALCITE-3846]
+   * EnumerableMergeJoin: wrong comparison of composite key with null 
values. */
+  @Test public void testMergeJoinWithCompositeKeyAndNullValues() {
+tester(false, new JdbcTest.HrSchema())
+.query("?")
+.withHook(Hook.PLANNER, (Consumer) planner -> {
+  planner.addRule(EnumerableRules.ENUMERABLE_MERGE_JOIN_RULE);
+  planner.removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE);
+})
+.withRel(builder -> builder
+.scan("s", "emps")
+.sort(builder.field("deptno"), builder.field("commission"))
+.scan("s", "emps")
+.sort(builder.field("deptno"), builder.field("commission"))
+.join(JoinRelType.INNER,
+builder.and(
+builder.equals(
+builder.field(2, 0, "deptno"),
+builder.field(2, 1, "deptno")),
+builder.equals(
+builder.field(2, 0, "commission"),
+builder.field(2, 1, "commission"
+.project(
+builder.field("empid"))
+.build())
+.explainHookMatches("" // It is important that we have MergeJoin in 
the plan
++ "EnumerableCalc(expr#0..4=[{inputs}], empid=[$t0])\n"
 
 Review comment:
   The comment seems a little confusing. Why is it important?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1870: [CALCITE-3846] EnumerableMergeJoin: wrong comparison of composite key with null values

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1870: [CALCITE-3846]  
EnumerableMergeJoin: wrong comparison of composite key with null values
URL: https://github.com/apache/calcite/pull/1870#discussion_r399010788
 
 

 ##
 File path: 
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
 ##
 @@ -3836,6 +3837,7 @@ public void remove() {
 // extra predicate in case of non equi-join, in case of equi-join it will 
be null
 private final Predicate2 extraPredicate;
 private final Function2 resultSelector;
+private final Comparator comparator; // possibly null (compareTo to 
be used in that case)
 private boolean done;
 
 Review comment:
   Should we put the comment on the top?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone commented on issue #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

DonnyZone commented on issue #1862: [CALCITE-3864] Implement Concat function
URL: https://github.com/apache/calcite/pull/1862#issuecomment-604791073
 
 
   Close and reopen to trigger the checks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] wenhuitang opened a new pull request #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

wenhuitang opened a new pull request #1862: [CALCITE-3864] Implement Concat 
function
URL: https://github.com/apache/calcite/pull/1862
 
 
   Pull request for https://issues.apache.org/jira/browse/CALCITE-3864


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1866: [CALCITE-3726] Documentation for Declaring Objects For Types Defined …

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1866: [CALCITE-3726] 
Documentation for Declaring Objects For Types Defined …
URL: https://github.com/apache/calcite/pull/1866#discussion_r399010216
 
 

 ##
 File path: site/_docs/reference.md
 ##
 @@ -2290,7 +2290,32 @@ Note:
  Declaring Objects For Types Defined In Schema
 After an object type is defined and installed in the schema, you can use it to 
declare objects in any SQL block. For example, you can use the object type to 
specify the datatype of an attribute, column, variable, bind variable, record 
field, table element, formal parameter, or function result. At run time, 
instances of the object type are created; that is, objects of that type are 
instantiated. Each object can hold different values.
 
 Review comment:
   @danny0405 Updated, kindly check if it is the right place?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone closed pull request #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

DonnyZone closed pull request #1862: [CALCITE-3864] Implement Concat function
URL: https://github.com/apache/calcite/pull/1862
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone commented on issue #1405: [CALCITE-2772] Support varargs UDF for scalar function

2020-03-26 Thread GitBox

DonnyZone commented on issue #1405: [CALCITE-2772] Support varargs UDF for 
scalar function
URL: https://github.com/apache/calcite/pull/1405#issuecomment-604790132
 
 
   Hi @gr4ve, to solve the conflicts, you need to rebase this pr on latest 
calcite/master branch. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1837: [CALCITE-3835] Overloaded table functions fail with an assertion error if param types differ

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1837: [CALCITE-3835] 
Overloaded table functions fail with an assertion error if param types differ
URL: https://github.com/apache/calcite/pull/1837#discussion_r399008307
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/util/Smalls.java
 ##
 @@ -843,6 +843,30 @@ private void abc(StringBuilder sb, Object s) {
 }
   }
 
+  /** User-defined table-macro function with named and optional parameters. */
+  public static class AnotherTableMacroFunctionWithNamedParameters {
+public TranslatableTable eval(
+@Parameter(name = "R", optional = true) String r,
+@Parameter(name = "S") String s,
+@Parameter(name = "T", optional = true) Integer t,
+@Parameter(name = "S2", optional = true) String s2) {
+  final StringBuilder sb = new StringBuilder();
+  abc(sb, r);
+  abc(sb, s);
+  abc(sb, t);
 
 Review comment:
   How can we resolve "View\"(r=>'6', s=>'5')) since both 
`TableMacroFunctionWithNamedParameters`
and `AnotherTableMacroFunctionWithNamedParameters` match?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1837: [CALCITE-3835] Overloaded table functions fail with an assertion error if param types differ

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1837: [CALCITE-3835] 
Overloaded table functions fail with an assertion error if param types differ
URL: https://github.com/apache/calcite/pull/1837#discussion_r399008307
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/util/Smalls.java
 ##
 @@ -843,6 +843,30 @@ private void abc(StringBuilder sb, Object s) {
 }
   }
 
+  /** User-defined table-macro function with named and optional parameters. */
+  public static class AnotherTableMacroFunctionWithNamedParameters {
+public TranslatableTable eval(
+@Parameter(name = "R", optional = true) String r,
+@Parameter(name = "S") String s,
+@Parameter(name = "T", optional = true) Integer t,
+@Parameter(name = "S2", optional = true) String s2) {
+  final StringBuilder sb = new StringBuilder();
+  abc(sb, r);
+  abc(sb, s);
+  abc(sb, t);
 
 Review comment:
   How can we distinguish "View\"(r=>'6', s=>'5')) since both 
`TableMacroFunctionWithNamedParameters`
and `AnotherTableMacroFunctionWithNamedParameters` match?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] chunweilei commented on a change in pull request #1868: [CALCITE-3867] Support RelDistribution json serialization

2020-03-26 Thread GitBox

chunweilei commented on a change in pull request #1868: [CALCITE-3867] Support 
RelDistribution json serialization
URL: https://github.com/apache/calcite/pull/1868#discussion_r399006271
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
 ##
 @@ -190,8 +193,35 @@ public RelFieldCollation toFieldCollation(Map map) {
 return new RelFieldCollation(field, direction, nullDirection);
   }
 
-  public RelDistribution toDistribution(Object o) {
-return RelDistributions.ANY; // TODO:
+  public RelDistribution toDistribution(Map map) {
+final RelDistribution.Type type =
 
 Review comment:
   Every public method is a public API. But I don't have a strong objection 
about it since it is not a frequently used api.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] danny0405 commented on a change in pull request #1866: [CALCITE-3726] Documentation for Declaring Objects For Types Defined …

2020-03-26 Thread GitBox

danny0405 commented on a change in pull request #1866: [CALCITE-3726] 
Documentation for Declaring Objects For Types Defined …
URL: https://github.com/apache/calcite/pull/1866#discussion_r399003445
 
 

 ##
 File path: site/_docs/reference.md
 ##
 @@ -2290,7 +2290,32 @@ Note:
  Declaring Objects For Types Defined In Schema
 After an object type is defined and installed in the schema, you can use it to 
declare objects in any SQL block. For example, you can use the object type to 
specify the datatype of an attribute, column, variable, bind variable, record 
field, table element, formal parameter, or function result. At run time, 
instances of the object type are created; that is, objects of that type are 
instantiated. Each object can hold different values.
 
 Review comment:
   Why this part is under `JSON Functions`, i think we should move it below the 
`DDL Extensions` part as a top level.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1866: [CALCITE-3726] Documentation for Declaring Objects For Types Defined …

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1866: [CALCITE-3726] 
Documentation for Declaring Objects For Types Defined …
URL: https://github.com/apache/calcite/pull/1866#discussion_r399001957
 
 

 ##
 File path: site/_docs/reference.md
 ##
 @@ -2286,6 +2286,17 @@ Note:
 * If `ORDER BY` clause is provided, `JSON_ARRAYAGG` sorts the
   input rows into the specified order before performing aggregation.
 
+
+ Declaring Objects For Types Defined In Schema
+After an object type is defined and installed in the schema, you can use it to 
declare objects in any SQL block. For example, you can use the object type to 
specify the datatype of an attribute, column, variable, bind variable, record 
field, table element, formal parameter, or function result. At run time, 
instances of the object type are created; that is, objects of that type are 
instantiated. Each object can hold different values.
+
+e.g.:
+```SQL
+employee_typ(315, 'Francis', 'Logan', 'FLOGAN',
+'555.777.', '01-MAY-04', 'SA_MAN', 11000, .15, 101, 110,
 
 Review comment:
   Updated documentation :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] XuQianJin-Stars commented on a change in pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

XuQianJin-Stars commented on a change in pull request #1865: [CALCITE-3648] 
MySQL UNCOMPRESS function support
URL: https://github.com/apache/calcite/pull/1865#discussion_r399001523
 
 

 ##
 File path: 
core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
 ##
 @@ -5192,6 +5192,25 @@ private void checkNullOperand(SqlTester tester, String 
op) {
 "0700789c4bad48cc2dc84905000bc002ed", "VARBINARY NOT NULL");
   }
 
+  @Test public void testUncompress() {
+SqlTester sqlTester = tester(SqlLibrary.MYSQL);
+sqlTester.checkNull("UNCOMPRESS(NULL)");
+sqlTester.checkString("UNCOMPRESS(x'')", "", "VARCHAR");
+
+sqlTester.checkNull("UNCOMPRESS(x'1233')");
+
+sqlTester.checkString("UNCOMPRESS(COMPRESS('test'))",
+"test", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'1000789c4b4c44050033980611')",
+"", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'0600789c2b4ecc2dc849050008de0283' 
)",
+"sample", "VARCHAR");
+
sqlTester.checkString("UNCOMPRESS(x'0700789c4bad48cc2dc84905000bc002ed')",
+"example", "VARCHAR");
+  }
 
 Review comment:
   Can add tests based on error？
   `sqlTester.checkFails("UNCOMPRESS('test')","(?s).*Only ByteString.*", 
true);` 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] gr4ve commented on a change in pull request #1405: [CALCITE-2772] Support varargs UDF for scalar function

2020-03-26 Thread GitBox

gr4ve commented on a change in pull request #1405: [CALCITE-2772] Support 
varargs UDF for scalar function
URL: https://github.com/apache/calcite/pull/1405#discussion_r399000697
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/UdfTest.java
 ##
 @@ -198,6 +235,19 @@
 assertThat(after, is(before + 4));
   }
 
+  @Test public void testUserDefinedFunctionWithArgumentAssignment() throws 
Exception {
+final String sql = "select \"adhoc\".my_plus(x=>\"deptno\", y=>100) as p\n"
++ "from \"adhoc\".EMPLOYEES";
+final AtomicInteger c = Smalls.MyPlusFunction.INSTANCE_COUNT.get();
+final int before = c.get();
+withUdf().query(sql).returnsUnordered("P=110",
+"P=120",
+"P=110",
 
 Review comment:
   I have added more test cases for this PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone commented on a change in pull request #1870: [CALCITE-3846] EnumerableMergeJoin: wrong comparison of composite key with null values

2020-03-26 Thread GitBox

DonnyZone commented on a change in pull request #1870: [CALCITE-3846]  
EnumerableMergeJoin: wrong comparison of composite key with null values
URL: https://github.com/apache/calcite/pull/1870#discussion_r399000418
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java
 ##
 @@ -164,6 +165,17 @@ public Result implement(EnumerableRelImplementor 
implementor, Prefer pref) {
 leftResult.physType.project(joinInfo.leftKeys, JavaRowFormat.LIST);
 final PhysType rightKeyPhysType =
 rightResult.physType.project(joinInfo.rightKeys, JavaRowFormat.LIST);
+
+// Generate the appropriate key Comparator (keys must be sorted in 
ascending order, nulls last).
+final List fieldCollations = new ArrayList<>();
 
 Review comment:
   I just noticed another [PR](https://github.com/apache/calcite/pull/1876) on 
array's initial capacity:)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone commented on issue #1870: [CALCITE-3846] EnumerableMergeJoin: wrong comparison of composite key with null values

2020-03-26 Thread GitBox

DonnyZone commented on issue #1870: [CALCITE-3846]  EnumerableMergeJoin: wrong 
comparison of composite key with null values
URL: https://github.com/apache/calcite/pull/1870#issuecomment-604781955
 
 
   +1, LGTM


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] gr4ve commented on a change in pull request #1405: [CALCITE-2772] Support varargs UDF for scalar function

2020-03-26 Thread GitBox

gr4ve commented on a change in pull request #1405: [CALCITE-2772] Support 
varargs UDF for scalar function
URL: https://github.com/apache/calcite/pull/1405#discussion_r399000506
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/util/Smalls.java
 ##
 @@ -253,7 +254,7 @@ public RelDataType getRowType(RelDataTypeFactory 
typeFactory) {
   private long current = 0;
 
   public Object[] current() {
-return new Object[] {current};
+return new Object[]{current};
   }
 
 Review comment:
   These changes have been rolled back.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone commented on a change in pull request #1870: [CALCITE-3846] EnumerableMergeJoin: wrong comparison of composite key with null values

2020-03-26 Thread GitBox

DonnyZone commented on a change in pull request #1870: [CALCITE-3846]  
EnumerableMergeJoin: wrong comparison of composite key with null values
URL: https://github.com/apache/calcite/pull/1870#discussion_r399000418
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java
 ##
 @@ -164,6 +165,17 @@ public Result implement(EnumerableRelImplementor 
implementor, Prefer pref) {
 leftResult.physType.project(joinInfo.leftKeys, JavaRowFormat.LIST);
 final PhysType rightKeyPhysType =
 rightResult.physType.project(joinInfo.rightKeys, JavaRowFormat.LIST);
+
+// Generate the appropriate key Comparator (keys must be sorted in 
ascending order, nulls last).
+final List fieldCollations = new ArrayList<>();
 
 Review comment:
   Just noticed another [PR](https://github.com/apache/calcite/pull/1876) on 
array's initial capacity:)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1873: [CALCITE-3872] Simplify expressions with unary minus

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1873: [CALCITE-3872] 
Simplify expressions with unary minus
URL: https://github.com/apache/calcite/pull/1873#discussion_r39929
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
 ##
 @@ -2265,8 +2265,8 @@ private void assertTypeAndToString(
 checkSimplify(coalesce(gt(nullInt, nullInt), trueLiteral),
 "true");
 checkSimplify(coalesce(unaryPlus(nullInt), unaryPlus(vInt())),
-"+(?0.int0)");
-checkSimplifyUnchanged(coalesce(unaryPlus(vInt(1)), unaryPlus(vInt(;
+"?0.int0");
 
 Review comment:
   Should we add more test cases which covers decimal as well?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on issue #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor commented on issue #1865: [CALCITE-3648] MySQL UNCOMPRESS 
function support
URL: https://github.com/apache/calcite/pull/1865#issuecomment-604780316
 
 
   > hi, @ritesh-kapoor Can this PR add a description?
   
   Added.. Thanks


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor opened a new pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor opened a new pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS 
function support
URL: https://github.com/apache/calcite/pull/1865
 
 
   
https://dev.mysql.com/doc/refman/8.0/en/encryption-functions.html#function_uncompress
   
   e.g.:
   
   SELECT UNCOMPRESS(COMPRESS('any string'));
   -> 'any string'
   SELECT UNCOMPRESS('any string');
   -> NULL
   Uncompresses a string compressed by the COMPRESS() function. If the argument 
is not a compressed value, the result is NULL. This function uses zlib library 
for decompression 
(https://docs.oracle.com/javase/8/docs/api/java/util/zip/Inflater.html). 
Otherwise, the return value is always NULL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor closed pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor closed pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS 
function support
URL: https://github.com/apache/calcite/pull/1865
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL 
UNCOMPRESS function support
URL: https://github.com/apache/calcite/pull/1865#discussion_r398998393
 
 

 ##
 File path: 
core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
 ##
 @@ -5192,6 +5192,23 @@ private void checkNullOperand(SqlTester tester, String 
op) {
 "0700789c4bad48cc2dc84905000bc002ed", "VARBINARY NOT NULL");
   }
 
+  @Test public void testUncompress() {
+SqlTester sqlTester = tester(SqlLibrary.MYSQL);
+sqlTester.checkNull("UNCOMPRESS(NULL)");
+sqlTester.checkNull("UNCOMPRESS(x'')");
+
+sqlTester.checkString("UNCOMPRESS(COMPRESS('test'))",
+"test", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'1000789c4b4c44050033980611')",
+"", "VARCHAR");
+
+sqlTester.checkString("UNCOMPRESS(x'0600789c2b4ecc2dc849050008de0283' 
)",
 
 Review comment:
   The contract doestnot allow string type as an argument. Instead I have added 
another negative test case. Thanks :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL 
UNCOMPRESS function support
URL: https://github.com/apache/calcite/pull/1865#discussion_r398998091
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/runtime/CompressionFunctions.java
 ##
 @@ -62,4 +65,21 @@ public static ByteString compress(String data) {
 }
   }
 
+  /**
+   * MySql Decompression is based on zlib.
+   * https://docs.oracle.com/javase/8/docs/api/java/util/zip/Inflater.html;>Inflater
+   * is used to implement decompression.
+   */
+  public static String uncompress(ByteString data) {
+try {
+  if (data == null || data.length() == 0) {
 
 Review comment:
   Added new test case. Thanks :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on a change in pull request #1869: [CALCITE-3868] Remove redundant ruleSet and ruleNames in VolcanoPlanner

2020-03-26 Thread GitBox

neoremind commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r398979529
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
 ##
 @@ -275,7 +253,7 @@ void executeInstruction(
 LOGGER.trace("Applying rule class {}", instruction.ruleClass);
 if (instruction.ruleSet == null) {
   instruction.ruleSet = new LinkedHashSet<>();
-  for (RelOptRule rule : allRules) {
+  for (RelOptRule rule : mapDescToRule.values()) {
 
 Review comment:
   I agree, there is "if" check to determine what to be added. I didn't find 
that out clearly, sorry about the false alert.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] hsyuan commented on a change in pull request #1869: [CALCITE-3868] Remove redundant ruleSet and ruleNames in VolcanoPlanner

2020-03-26 Thread GitBox

hsyuan commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r398933698
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
 ##
 @@ -275,7 +253,7 @@ void executeInstruction(
 LOGGER.trace("Applying rule class {}", instruction.ruleClass);
 if (instruction.ruleSet == null) {
   instruction.ruleSet = new LinkedHashSet<>();
-  for (RelOptRule rule : allRules) {
+  for (RelOptRule rule : mapDescToRule.values()) {
 
 Review comment:
   It turns out the size instruction.ruleSet is undetermined, we don't know the 
exact size. So I will leave as it is.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vvysotskyi commented on a change in pull request #1837: [CALCITE-3835] Overloaded table functions fail with an assertion error if param types differ

2020-03-26 Thread GitBox

vvysotskyi commented on a change in pull request #1837: [CALCITE-3835] 
Overloaded table functions fail with an assertion error if param types differ
URL: https://github.com/apache/calcite/pull/1837#discussion_r398808399
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java
 ##
 @@ -568,36 +572,44 @@ private void addTableMacro(Connection connection, Method 
method) throws SQLExcep
 checkTableFunctionInModel(Smalls.TestStaticTableFunction.class);
   }
 
-  private CalciteAssert.AssertThat assertWithMacro(Class clazz) {
+  private CalciteAssert.AssertThat assertOverloadedWithMacro(Class... 
clazz) {
+String prefix = ""
 
 Review comment:
   Ok, thanks for pointing this, reverted changing method name.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

neoremind commented on issue #1875: [CALCITE-3873] Use global caching for 
ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#issuecomment-604520763
 
 
   @vlsi I will try to add the benchmark. `MethodHandle` would be a good 
alternative, since it requires both returned and parameter types, it might take 
some time to update the code to make it work. I will look into this solution.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

vlsi commented on issue #1875: [CALCITE-3873] Use global caching for 
ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#issuecomment-604506267
 
 
   @neoremind , do you think you could add the jmh test to Calcite as well (see 
https://github.com/apache/calcite/tree/master/ubenchmark )?
   
   PS Have you considered MethodHandle-based approach?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vlsi closed pull request #1877: RAV-1502

2020-03-26 Thread GitBox

vlsi closed pull request #1877: RAV-1502
URL: https://github.com/apache/calcite/pull/1877
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vlsi commented on issue #1877: RAV-1502

2020-03-26 Thread GitBox

vlsi commented on issue #1877: RAV-1502
URL: https://github.com/apache/calcite/pull/1877#issuecomment-604504754
 
 
   @sarbjeet805, it looks like the PR is for the wrong repository :-/


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] sarbjeet805 opened a new pull request #1877: RAV-1502

2020-03-26 Thread GitBox

sarbjeet805 opened a new pull request #1877: RAV-1502
URL: https://github.com/apache/calcite/pull/1877
 
 
   Support of log function for snowflake dialect added


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

neoremind commented on issue #1875: [CALCITE-3873] Use global caching for 
ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#issuecomment-604479873
 
 
   @danny0405 jmh test result is as below. It shows by using reflection 
invocation to look up methods in every call, the overhead is not small compared 
with global caching. In real case, reflection invocation can happen between 
invocations and different `ReflectVisitorDispatcher` instance. The solution 
might be trivial, but I think it goes toward the right way.
   
   ```
   Benchmark  Mode  CntScore
 Error  Units
   ReflectVisitorDispatcherTest.testGlobalLevelCachingavgt   15  108.398 ±  
15.280  ns/op
   ReflectVisitorDispatcherTest.testInstanceLevelCaching  avgt   15  730.208 ± 
117.693  ns/op
   ```
   
   Test source code is attached in 
[JIRA](https://issues.apache.org/jira/secure/attachment/12997903/ReflectVisitorDispatcherTest.java),
 the full test report can be found 
[here](https://issues.apache.org/jira/secure/attachment/12997904/jmh_result.txt)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on a change in pull request #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

neoremind commented on a change in pull request #1875: [CALCITE-3873] Use 
global caching for ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#discussion_r398636010
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/util/ReflectVisitDispatcherImpl.java
 ##
 @@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Looking up methods relating to reflective visitation. Caching reflect
+ * method globally, every instance of the class shares the global cache to
+ * mitigating java reflection invocation overhead.
+ *
 
 Review comment:
   Thanks for pointing out typo :-)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] danny0405 commented on a change in pull request #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

danny0405 commented on a change in pull request #1875: [CALCITE-3873] Use 
global caching for ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#discussion_r398586118
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/util/ReflectVisitDispatcherImpl.java
 ##
 @@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.collect.ImmutableList;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Looking up methods relating to reflective visitation. Caching reflect
+ * method globally, every instance of the class shares the global cache to
+ * mitigating java reflection invocation overhead.
+ *
 
 Review comment:
   Use mitigate as a verb 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] danny0405 commented on a change in pull request #1837: [CALCITE-3835] Overloaded table functions fail with an assertion error if param types differ

2020-03-26 Thread GitBox

danny0405 commented on a change in pull request #1837: [CALCITE-3835] 
Overloaded table functions fail with an assertion error if param types differ
URL: https://github.com/apache/calcite/pull/1837#discussion_r398548086
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/JdbcTest.java
 ##
 @@ -568,36 +572,44 @@ private void addTableMacro(Connection connection, Method 
method) throws SQLExcep
 checkTableFunctionInModel(Smalls.TestStaticTableFunction.class);
   }
 
-  private CalciteAssert.AssertThat assertWithMacro(Class clazz) {
+  private CalciteAssert.AssertThat assertOverloadedWithMacro(Class... 
clazz) {
+String prefix = ""
 
 Review comment:
   I think we should keep the old name. The passed in param is not necessary to 
be an array, so it maybe not overloaded.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] wenhuitang commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision ex

2020-03-26 Thread GitBox

wenhuitang commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r398505227
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
 ##
 @@ -1015,17 +1015,23 @@ private static boolean flattenFields(
* @param type type descriptor
* @param charSetName  charSet name
* @param maxPrecision The max allowed precision.
+   * @param allowsPrecisionUnspecified whether allows precision not specified
+   * @param defaultPrecision The default precision.
* @return corresponding parse representation
*/
   public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
-  String charSetName, int maxPrecision) {
+  String charSetName, int maxPrecision,
+  boolean allowsPrecisionUnspecified, int defaultPrecision) {
 SqlTypeName typeName = type.getSqlTypeName();
 
 Review comment:
   This pull request has been updated, thanks for your reviewing, it's really 
helpful.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] wenhuitang commented on a change in pull request #1861: [CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision ex

2020-03-26 Thread GitBox

wenhuitang commented on a change in pull request #1861: [CALCITE-3468] JDBC 
adapter may generate casts on Oracle for varchar without the precision and for 
char with the precision exceeding max length of Oracle
URL: https://github.com/apache/calcite/pull/1861#discussion_r398505227
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
 ##
 @@ -1015,17 +1015,23 @@ private static boolean flattenFields(
* @param type type descriptor
* @param charSetName  charSet name
* @param maxPrecision The max allowed precision.
+   * @param allowsPrecisionUnspecified whether allows precision not specified
+   * @param defaultPrecision The default precision.
* @return corresponding parse representation
*/
   public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
-  String charSetName, int maxPrecision) {
+  String charSetName, int maxPrecision,
+  boolean allowsPrecisionUnspecified, int defaultPrecision) {
 SqlTypeName typeName = type.getSqlTypeName();
 
 Review comment:
   This pull request has been updated, thanks for you reviewing, it's really 
helpful.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vvysotskyi commented on a change in pull request #1837: [CALCITE-3835] Overloaded table functions fail with an assertion error if param types differ

2020-03-26 Thread GitBox

vvysotskyi commented on a change in pull request #1837: [CALCITE-3835] 
Overloaded table functions fail with an assertion error if param types differ
URL: https://github.com/apache/calcite/pull/1837#discussion_r398469196
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/sql/SqlUtil.java
 ##
 @@ -676,14 +680,16 @@ public static boolean matchRoutinesByParameterCount(
   }
 
   private static RelDataType bestMatch(List sqlFunctions, int i,
-  RelDataTypePrecedenceList precList) {
+  List argNames, RelDataTypePrecedenceList precList) {
 RelDataType bestMatch = null;
 for (SqlFunction function : sqlFunctions) {
   List paramTypes = function.getParamTypes();
   if (paramTypes == null) {
 continue;
   }
-  final RelDataType paramType = paramTypes.get(i);
+  final RelDataType paramType = argNames != null
+  ? paramTypes.get(function.getParamNames().indexOf(argNames.get(i)))
 
 Review comment:
   I have renamed `filterRoutinesByParameterType()` method and excracted logic 
connected with types and names permutation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] vlsi commented on issue #1876: [CALCITE-3878] Make ArrayList creation with initial capacity when size is fixed

2020-03-26 Thread GitBox

vlsi commented on issue #1876: [CALCITE-3878] Make ArrayList creation with 
initial capacity when size is fixed
URL: https://github.com/apache/calcite/pull/1876#issuecomment-604344052
 
 
   LGTM


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind opened a new pull request #1876: [CALCITE-3878] Make ArrayList creation with initial capacity when size is fixed

2020-03-26 Thread GitBox

neoremind opened a new pull request #1876: [CALCITE-3878] Make ArrayList 
creation with initial capacity when size is fixed
URL: https://github.com/apache/calcite/pull/1876
 
 
   I find many places in Calcite where `new ArrayList<>()` is used, if the list 
is expected to be immutable or not resizing, it is always a good manner to 
create with initial capacity, better for memory usage and performance.
   
   I search all occurrences, to make it safe, I only update local variables 
with fixed size and not working in recursive method. If the local variable 
reference goes out of scope, if resizing is needed, things will work normally 
as well, so no side effect, but for the "escaping" case, I am very conservative 
and do not change them.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] DonnyZone closed pull request #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

DonnyZone closed pull request #1862: [CALCITE-3864] Implement Concat function
URL: https://github.com/apache/calcite/pull/1862
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] wenhuitang opened a new pull request #1862: [CALCITE-3864] Implement Concat function

2020-03-26 Thread GitBox

wenhuitang opened a new pull request #1862: [CALCITE-3864] Implement Concat 
function
URL: https://github.com/apache/calcite/pull/1862
 
 
   Pull request for https://issues.apache.org/jira/browse/CALCITE-3864


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] zabetak commented on a change in pull request #1869: [CALCITE-3868] Remove redundant ruleSet and ruleNames in VolcanoPlanner

2020-03-26 Thread GitBox

zabetak commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r398410186
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
 ##
 @@ -44,18 +44,13 @@
  * Abstract base for implementations of the {@link RelOptPlanner} interface.
  */
 public abstract class AbstractRelOptPlanner implements RelOptPlanner {
-  //~ Static fields/initializers -
-
-  /** Regular expression for integer. */
-  private static final Pattern INTEGER_PATTERN = Pattern.compile("[0-9]+");
-
   //~ Instance fields 
 
   /**
* Maps rule description to rule, just to ensure that rules' descriptions
* are unique.
*/
-  private final Map mapDescToRule = new HashMap<>();
+  protected final Map mapDescToRule = new HashMap<>();
 
 Review comment:
   I think that in most cases the number of rules is not very big so I was 
thinking that copying vs. `mapDescToRule.values()` is not going to have 
significant performance overhead in the planning phase, thus, I tend to prefer 
better encapsulation. Having that said, I do not have any concrete measures to 
support my claims (just instinct that could be wrong) :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on a change in pull request #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation

2020-03-26 Thread GitBox

neoremind commented on a change in pull request #1875: [CALCITE-3873] Use 
global caching for ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#discussion_r398391089
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/util/ReflectUtil.java
 ##
 @@ -403,71 +403,40 @@ private static Method lookupVisitMethod(
   }
 
   /**
-   * Creates a dispatcher for calls to {@link #lookupVisitMethod}. The
-   * dispatcher caches methods between invocations.
+   * Creates a dispatcher for calls to {@link #lookupVisitMethod}. By default 
the
+   * dispatcher caches methods globally. If caching methods between invocations
+   * is preferred, use the overridden method {@link #createDispatcher(Class, 
Class, boolean)}.
*
-   * @param visitorBaseClazz Visitor base class
-   * @param visiteeBaseClazz Visitee base class
+   * @param visitorBaseClazzVisitor base class
+   * @param visiteeBaseClazzVisitee base class
* @return cache of methods
*/
   public static  ReflectiveVisitDispatcher createDispatcher(
   final Class visitorBaseClazz,
   final Class visiteeBaseClazz) {
+return createDispatcher(visitorBaseClazz, visiteeBaseClazz, true);
+  }
+
+  /**
+   * Creates a dispatcher for calls to {@link #lookupVisitMethod}.
+   *
+   * @param visitorBaseClazzVisitor base class
+   * @param visiteeBaseClazzVisitee base class
+   * @param useGlobalMethodCacheIf set to true, the created dispatchers 
will
+   *share a globally cache to store methods to
+   *mitigating reflection invocation overhead
+   *when looking up method. If set to false, 
every
+   *dispatcher instance will only cache methods
+   *between invocations individually.
+   * @return cache of methods
+   */
+  public static  ReflectiveVisitDispatcher createDispatcher(
+  final Class visitorBaseClazz,
+  final Class visiteeBaseClazz,
+  final boolean useGlobalMethodCache) {
 
 Review comment:
   Comment addressed. Make `ReflectiveVisitDispatcher` to always use global 
caching.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL UNCOMPRESS function support

2020-03-26 Thread GitBox

ritesh-kapoor commented on a change in pull request #1865: [CALCITE-3648] MySQL 
UNCOMPRESS function support
URL: https://github.com/apache/calcite/pull/1865#discussion_r398385503
 
 

 ##
 File path: site/_docs/reference.md
 ##
 @@ -2362,6 +2362,7 @@ semantics.
 | o p | TO_DATE(string, format)  | Converts *string* to a 
date using the format *format*
 | o p | TO_TIMESTAMP(string, format) | Converts *string* to a 
timestamp using the format *format*
 | o p | TRANSLATE(expr, fromString, toString)| Returns *expr* with all 
occurrences of each character in *fromString* replaced by its corresponding 
character in *toString*. Characters in *expr* that are not in *fromString* are 
not replaced
+| m | UNCOMPRESS(binary) | UnCompresses binary 
using zlib decompression and returns the result as a string.
 | o | XMLTRANSFORM(xml, xslt)| Returns a string after 
applying xslt to supplied xml.
 
 Review comment:
   Function name to be in alphabetical order makes more sense to me, I can 
raise separate PR if we want to change the order.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] hsyuan commented on a change in pull request #1869: [CALCITE-3868] Remove redundant ruleSet and ruleNames in VolcanoPlanner

2020-03-26 Thread GitBox

hsyuan commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r398362363
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
 ##
 @@ -275,7 +253,7 @@ void executeInstruction(
 LOGGER.trace("Applying rule class {}", instruction.ruleClass);
 if (instruction.ruleSet == null) {
   instruction.ruleSet = new LinkedHashSet<>();
-  for (RelOptRule rule : allRules) {
+  for (RelOptRule rule : mapDescToRule.values()) {
 
 Review comment:
   Yes, will do.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] neoremind commented on a change in pull request #1869: [CALCITE-3868] Remove redundant ruleSet and ruleNames in VolcanoPlanner

2020-03-26 Thread GitBox

neoremind commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r398354625
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
 ##
 @@ -275,7 +253,7 @@ void executeInstruction(
 LOGGER.trace("Applying rule class {}", instruction.ruleClass);
 if (instruction.ruleSet == null) {
   instruction.ruleSet = new LinkedHashSet<>();
-  for (RelOptRule rule : allRules) {
+  for (RelOptRule rule : mapDescToRule.values()) {
 
 Review comment:
   If we already know the size of `mapDescToRule`, we can create a set with 
exact size, which will eliminate capacity expansion overhead and space waste 
when creating. Even though this is trivial update, I think it is always a good 
manner to create collection in such way if possible.
   ```
   instruction.ruleSet = Sets.newHashSetWithExpectedSize(mapDescToRule.size());
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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] [calcite] praveenbingo commented on issue #1859: [CALCITE-3863] Make Truncate/Round return type inference overridable through rel data …

2020-03-26 Thread GitBox

praveenbingo commented on issue #1859: [CALCITE-3863] Make Truncate/Round 
return type inference overridable through rel data …
URL: https://github.com/apache/calcite/pull/1859#issuecomment-604246510
 
 
   > > @danny0405 This is similar to #1311 which made some of the other decimal 
related rules configurable using a custom type system.
   > > Our understanding back then was this was the recommended way. Could you 
please elaborate on your approach if you think this is not the right way to 
achieve the same.
   > 
   > In my opinion, #1311 is very different because `+` and `-` are builtin 
operators and we have no way for downstream projects to replace these 
operators, but `Round` and `Truncate` are different, they are sql functions, 
and if your project has maintained the sql operators by yourself, there is a 
chance you just override the type inference of these two instead of the whole 
type system, which seems too heavy and special to customize.
   
   Hi Danny,
   
   Thanks for the comments. I think we discussed something similar in this 
comment in 
1311(https://issues.apache.org/jira/browse/CALCITE-3187?focusedCommentId=16882084=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16882084).
   
   I think the same holds true even now, I think we want to consistently 
override return type inference for all decimal functions so that even Calcite 
internals use the same rules as us. Having two sets of rules applied 
inconsistently between Calcite internals and Application internals will be hard 
to debug.
   
   Please let us know if you have an alternative that keeps the rules 
consistent across the board.
   
   Thx.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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

56 matches

Mail list logo