[jira] [Closed] (CALCITE-6272) Improved LogicalPlan representation of distinct keyword

2024-02-19 Thread Caican Cai (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-6272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Caican Cai closed CALCITE-6272.
---
Resolution: Not A Problem

> Improved LogicalPlan representation of distinct keyword
> ---
>
> Key: CALCITE-6272
> URL: https://issues.apache.org/jira/browse/CALCITE-6272
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Minor
> Fix For: 1.37.0
>
>
> In the ToLogicalConvert test, the LogicalPlan parsed with or without the 
> distinct keyword is the same. I think this is an improvement.
>  
> {code:java}
> @Test void testdistnct() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .distinct()
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> }
> @Test void test() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> } {code}
> I think it can be improved to something like
> {code:java}
> Logical Project (DISTINCT)
> +- Logical Scan (Table: employees){code}
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Resolved] (CALCITE-6273) Add sqrt negative test in SqlOperatorTest

2024-02-19 Thread Jiajun Xie (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-6273?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jiajun Xie resolved CALCITE-6273.
-
Resolution: Fixed

> Add sqrt negative test in SqlOperatorTest
> -
>
> Key: CALCITE-6273
> URL: https://issues.apache.org/jira/browse/CALCITE-6273
> Project: Calcite
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Trivial
>  Labels: pull-request-available
> Fix For: 1.37.0
>
>
> sqrt currently lacks some negatvie tests, such as sqrt(-1), sqrt(0), 
> sqrt(0.5), etc. The idea is mainly because of the log function



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (CALCITE-6273) Add sqrt negative test in SqlOperatorTest

2024-02-19 Thread Jiajun Xie (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818608#comment-17818608
 ] 

Jiajun Xie commented on CALCITE-6273:
-

Fixed in 
[31d66e7.|https://github.com/apache/calcite/commit/31d66e797d3ca66d154262b15ee35876804df963]

Thanks [~caicancai] 

> Add sqrt negative test in SqlOperatorTest
> -
>
> Key: CALCITE-6273
> URL: https://issues.apache.org/jira/browse/CALCITE-6273
> Project: Calcite
>  Issue Type: Test
>  Components: tests
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Trivial
>  Labels: pull-request-available
> Fix For: 1.37.0
>
>
> sqrt currently lacks some negatvie tests, such as sqrt(-1), sqrt(0), 
> sqrt(0.5), etc. The idea is mainly because of the log function



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (CALCITE-6272) Improved LogicalPlan representation of distinct keyword

2024-02-19 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818595#comment-17818595
 ] 

Julian Hyde commented on CALCITE-6272:
--

I think you’re saying the current behavior is fine, and you want to add tests. 
I believe the current behavior is fine and we probably don’t need any more 
tests. 

> Improved LogicalPlan representation of distinct keyword
> ---
>
> Key: CALCITE-6272
> URL: https://issues.apache.org/jira/browse/CALCITE-6272
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Minor
> Fix For: 1.37.0
>
>
> In the ToLogicalConvert test, the LogicalPlan parsed with or without the 
> distinct keyword is the same. I think this is an improvement.
>  
> {code:java}
> @Test void testdistnct() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .distinct()
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> }
> @Test void test() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> } {code}
> I think it can be improved to something like
> {code:java}
> Logical Project (DISTINCT)
> +- Logical Scan (Table: employees){code}
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (CALCITE-6259) The implementation of the Log library operator does not match the actual dialect behavior.

2024-02-19 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818583#comment-17818583
 ] 

Julian Hyde commented on CALCITE-6259:
--

Go with [~mbudiu]'s method (separate SQL functions). But please do not 
slavishly add one method to {{class SqlFunctions}} for every SQL function; as I 
said above, it may make sense to add arguments to existing methods. Use your 
discretion.

> The implementation of the Log library operator does not match the actual 
> dialect behavior.
> --
>
> Key: CALCITE-6259
> URL: https://issues.apache.org/jira/browse/CALCITE-6259
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Major
> Fix For: 1.37.0
>
> Attachments: 302662660-27b21670-5364-463c-b6dc-d750c46d7cd1.png, 
> 302663876-91173a60-695d-409e-b325-3f91655c6d0d.png
>
>
> The implementation of the Log library operator does not match the actual 
> dialect behavior.
> For example, when log10(0) returns null in mysql and spark, but log10(0) 
> returns error in postgres, neither is calcite's -Intity
> {code:java}
> postgres=# select log10(0);
> ERROR:  cannot take logarithm of zero
> postgres=# select log(2,0);
> ERROR:  cannot take logarithm of zero
>  {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (CALCITE-6260) Add already implemented functions in Spark library

2024-02-19 Thread Caican Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818509#comment-17818509
 ] 

Caican Cai commented on CALCITE-6260:
-

I would rather create a few more jira cases, and I don't want calcite 
submissions to be confusing.

> Add already implemented functions in Spark library
> --
>
> Key: CALCITE-6260
> URL: https://issues.apache.org/jira/browse/CALCITE-6260
> Project: Calcite
>  Issue Type: Improvement
>Reporter:  EveyWu
>Priority: Minor
>  Labels: pull-request-available
>
> Add Spark functions that have been implemented but have different 
> OperandTypes/Returns.
> Spark Functions Link:[https://spark.apache.org/docs/latest/api/sql/index.html]
> Add function List:
>  * CHR
>  * CONCAT_WS
>  * REGEXP
>  * REVERSE
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Comment Edited] (CALCITE-6272) Improved LogicalPlan representation of distinct keyword

2024-02-19 Thread Caican Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818438#comment-17818438
 ] 

Caican Cai edited comment on CALCITE-6272 at 2/19/24 10:13 AM:
---

[~julianhyde] Hi,I noticed your jira on [distinct|CALCITE-1634 ] processing, I 
think this jira can be turned off, should I add distinct test to 
ToLogicalConverterTest and modify the jira description.


was (Author: JIRAUSER302115):
[~julianhyde] Hi,I noticed your jira on [distinct|[CALCITE-1634] Make 
RelBuilder.distinct no-op if input is already distinct - ASF JIRA (apache.org)] 
processing, I think this jira can be turned off, should I add distinct test to 
ToLogicalConverterTest and modify the jira description.

> Improved LogicalPlan representation of distinct keyword
> ---
>
> Key: CALCITE-6272
> URL: https://issues.apache.org/jira/browse/CALCITE-6272
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Minor
> Fix For: 1.37.0
>
>
> In the ToLogicalConvert test, the LogicalPlan parsed with or without the 
> distinct keyword is the same. I think this is an improvement.
>  
> {code:java}
> @Test void testdistnct() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .distinct()
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> }
> @Test void test() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> } {code}
> I think it can be improved to something like
> {code:java}
> Logical Project (DISTINCT)
> +- Logical Scan (Table: employees){code}
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (CALCITE-6272) Improved LogicalPlan representation of distinct keyword

2024-02-19 Thread Caican Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-6272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17818438#comment-17818438
 ] 

Caican Cai commented on CALCITE-6272:
-

[~julianhyde] Hi,I noticed your jira on [distinct|[CALCITE-1634] Make 
RelBuilder.distinct no-op if input is already distinct - ASF JIRA (apache.org)] 
processing, I think this jira can be turned off, should I add distinct test to 
ToLogicalConverterTest and modify the jira description.

> Improved LogicalPlan representation of distinct keyword
> ---
>
> Key: CALCITE-6272
> URL: https://issues.apache.org/jira/browse/CALCITE-6272
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.36.0
>Reporter: Caican Cai
>Priority: Minor
> Fix For: 1.37.0
>
>
> In the ToLogicalConvert test, the LogicalPlan parsed with or without the 
> distinct keyword is the same. I think this is an improvement.
>  
> {code:java}
> @Test void testdistnct() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .distinct()
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> }
> @Test void test() {
>   // Equivalent SQL:
>   //   SELECT DISTINCT *
>   //   FROM emp
>   final RelBuilder builder = builder();
>   final RelNode rel =
>   builder.scan("EMP")
>   .build();
>   String expectedPhysical = ""
>   + "EnumerableTableScan(table=[[scott, EMP]])\n";
>   String expectedLogical = ""
>   + "LogicalTableScan(table=[[scott, EMP]])\n";
>   verify(rel, expectedPhysical, expectedLogical);
> } {code}
> I think it can be improved to something like
> {code:java}
> Logical Project (DISTINCT)
> +- Logical Scan (Table: employees){code}
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Resolved] (CALCITE-6249) RelNode::estimatedRowCount should not be used in computeSelfCost

2024-02-19 Thread Ruben Q L (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-6249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ruben Q L resolved CALCITE-6249.

Resolution: Fixed

Done in 
[{{a19a954}}|https://github.com/apache/calcite/commit/a19a954738e3405f621843a9df71e51002e1609b]
 

Thanks [~kramerul] for the patch!

> RelNode::estimatedRowCount should not be used in computeSelfCost
> 
>
> Key: CALCITE-6249
> URL: https://issues.apache.org/jira/browse/CALCITE-6249
> Project: Calcite
>  Issue Type: Bug
>Reporter: Ulrich Kramer
>Priority: Minor
>  Labels: pull-request-available
> Fix For: 1.37.0
>
>
> {{RelNode::estimatedRowCount}} is still used in many place inside 
> {{computeSelfCost}}
> If it should be possible to implement the estimation of the row count 
> completely outside Calcite, these calls should be replaced by 
> {{mq.getRowCount(rel)}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)