[jira] [Commented] (CALCITE-5423) Implement TIMESTAMP_DIFF function (compatible with BigQuery)

2022-12-09 Thread Tanner Clary (Jira)


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

Tanner Clary commented on CALCITE-5423:
---

I opened a PR today for TIMESTAMP_DIFF but would love feedback on where more 
tests could be added and whether the function call added to the parser could be 
avoided. I made this decision because there was an issue parsing the third 
argument (the time unit) as it was expecting an interval qualifier instead. Any 
comments or suggestions would be appreciated. Thanks!

> Implement TIMESTAMP_DIFF function (compatible with BigQuery)
> 
>
> Key: CALCITE-5423
> URL: https://issues.apache.org/jira/browse/CALCITE-5423
> Project: Calcite
>  Issue Type: Sub-task
>Reporter: Julian Hyde
>Assignee: Tanner Clary
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Implement TIMESTAMP_DIFF function (compatible with BigQuery).



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


[jira] [Updated] (CALCITE-5423) Implement TIMESTAMP_DIFF function (compatible with BigQuery)

2022-12-09 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot updated CALCITE-5423:

Labels: pull-request-available  (was: )

> Implement TIMESTAMP_DIFF function (compatible with BigQuery)
> 
>
> Key: CALCITE-5423
> URL: https://issues.apache.org/jira/browse/CALCITE-5423
> Project: Calcite
>  Issue Type: Sub-task
>Reporter: Julian Hyde
>Assignee: Tanner Clary
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> Implement TIMESTAMP_DIFF function (compatible with BigQuery).



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


[jira] [Updated] (CALCITE-5428) Reduce minimum Guava version to 16.0.1

2022-12-09 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot updated CALCITE-5428:

Labels: pull-request-available  (was: )

> Reduce minimum Guava version to 16.0.1
> --
>
> Key: CALCITE-5428
> URL: https://issues.apache.org/jira/browse/CALCITE-5428
> Project: Calcite
>  Issue Type: Task
>  Components: core
>Reporter: Gian Merlino
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> If Calcite were to support Guava 16.0.1, this would help Druid upgrade to the 
> latest version while maintaining compatibility with Hadoop 2.x.
> See discussion in: 
> https://lists.apache.org/thread/8g5fpd4kpt7zhvf4l9397p9k95fny8tq.



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


[jira] [Created] (CALCITE-5428) Reduce minimum Guava version to 16.0.1.

2022-12-09 Thread Gian Merlino (Jira)
Gian Merlino created CALCITE-5428:
-

 Summary: Reduce minimum Guava version to 16.0.1.
 Key: CALCITE-5428
 URL: https://issues.apache.org/jira/browse/CALCITE-5428
 Project: Calcite
  Issue Type: Task
  Components: core
Reporter: Gian Merlino


If Calcite were to support Guava 16.0.1, this would help Druid upgrade to the 
latest version while maintaining compatibility with Hadoop 2.x.

See discussion in: 
https://lists.apache.org/thread/8g5fpd4kpt7zhvf4l9397p9k95fny8tq.



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


[jira] [Updated] (CALCITE-5428) Reduce minimum Guava version to 16.0.1

2022-12-09 Thread Gian Merlino (Jira)


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

Gian Merlino updated CALCITE-5428:
--
Summary: Reduce minimum Guava version to 16.0.1  (was: Reduce minimum Guava 
version to 16.0.1.)

> Reduce minimum Guava version to 16.0.1
> --
>
> Key: CALCITE-5428
> URL: https://issues.apache.org/jira/browse/CALCITE-5428
> Project: Calcite
>  Issue Type: Task
>  Components: core
>Reporter: Gian Merlino
>Priority: Major
>
> If Calcite were to support Guava 16.0.1, this would help Druid upgrade to the 
> latest version while maintaining compatibility with Hadoop 2.x.
> See discussion in: 
> https://lists.apache.org/thread/8g5fpd4kpt7zhvf4l9397p9k95fny8tq.



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


[jira] [Commented] (CALCITE-5426) BlockBuilder should not optimize expressions related to mutable objects to variable

2022-12-09 Thread Julian Hyde (Jira)


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

Julian Hyde commented on CALCITE-5426:
--

Thanks for logging this, [~dmsysolyatin]. It's an important correctness issue 
and we should fix it.

{quote}
Are there any performance tests that prove that this optimization significantly 
improves performance?
{quote}

No, because there are no performance tests. But if we removed common 
subexpression elimination - and say moved an array initializer from a static 
initializer to inside a for loop - I'm sure there would be significant 
degradation.

Let's provide a way for the caller of the API a way to say that the object is 
mutable. If the object is of an immutable type, say {{String}} or {{Double}} or 
{{ImmutableList}} then it would be an error if the mutable flag is true. If the 
object is of a possibly-mutable type, say {{List}} or {{ArrayList}} or an 
array, then we have to take the caller at their word.

Maybe there are different kinds of mutable. Perhaps a list is populated at the 
start of each execution but doesn't change from one row to the next. Keep an 
eye out for those cases; we might need more information than boolean mutable vs 
immutable, or we might be able to safely treat them as immutable.

> BlockBuilder should not optimize expressions related to mutable objects to 
> variable
> ---
>
> Key: CALCITE-5426
> URL: https://issues.apache.org/jira/browse/CALCITE-5426
> Project: Calcite
>  Issue Type: Bug
>  Components: linq4j
>Affects Versions: 1.32.0
>Reporter: Dmitry Sysolyatin
>Priority: Major
>
> Inside BlockBuilder there is an optimization that replaces an expression with 
> a variable if the expressions are equal and have the final modifier.
> But this optimization can cause problems when used with a mutable objects 
> (One of the problems has been found in CALCITE-5388):
> For example [1]:
> {code:java}
>   @Test void testReuseCollectionExpression() throws NoSuchMethodException {
> Method putMethod = HashMap.class.getMethod("put", Object.class, 
> Object.class);
> Method sizeMethod = HashMap.class.getMethod("size");
> Expression multiMapParent = b.append("multiMap", 
> Expressions.new_(Types.of(HashMap.class)));
> b.add(Expressions.statement(
> Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
> Expressions.box(ONE;
> BlockBuilder nested = new BlockBuilder(true, b);
> Expression multiMapNested = nested.append("multiMap",
> Expressions.new_(Types.of(HashMap.class)));
> nested.add(Expressions.statement(
> Expressions.call(multiMapNested, putMethod, Expressions.box(TWO), 
> Expressions.box(TWO;
> nested.add(Expressions.call(multiMapNested, sizeMethod));
> b.add(nested.toBlock());
> b.append(Expressions.call(multiMapParent, sizeMethod));
> // It is wrong output. Map should be reused
> assertEquals(
> "{\n"
> + "  final java.util.HashMap multiMap = new 
> java.util.HashMap();\n"
> + "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
> + "  {\n"
> + "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
> + "return multiMap.size();\n"
> + "  }\n"
> + "  return multiMap.size();\n"
> + "}\n",
> b.toBlock().toString());
>   }
> {code}
> Are there any performance tests that prove that this optimization 
> significantly improves performance?
> [1] 
> https://github.com/dssysolyatin/calcite/commit/626a5f48ef9e69e543aeec277a4f38000a190b10



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


[jira] [Updated] (CALCITE-5426) BlockBuilder should not optimize expressions related to mutable objects to variable

2022-12-09 Thread Dmitry Sysolyatin (Jira)


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

Dmitry Sysolyatin updated CALCITE-5426:
---
Summary: BlockBuilder should not optimize expressions related to mutable 
objects to variable  (was: BlockBuilder should not reuse mutable objects)

> BlockBuilder should not optimize expressions related to mutable objects to 
> variable
> ---
>
> Key: CALCITE-5426
> URL: https://issues.apache.org/jira/browse/CALCITE-5426
> Project: Calcite
>  Issue Type: Bug
>  Components: linq4j
>Affects Versions: 1.32.0
>Reporter: Dmitry Sysolyatin
>Priority: Major
>
> Inside BlockBuilder there is an optimization that replaces an expression with 
> a variable if the expressions are equal and have the final modifier.
> But this optimization can cause problems when used with a mutable objects 
> (One of the problems has been found in CALCITE-5388):
> For example [1]:
> {code:java}
>   @Test void testReuseCollectionExpression() throws NoSuchMethodException {
> Method putMethod = HashMap.class.getMethod("put", Object.class, 
> Object.class);
> Method sizeMethod = HashMap.class.getMethod("size");
> Expression multiMapParent = b.append("multiMap", 
> Expressions.new_(Types.of(HashMap.class)));
> b.add(Expressions.statement(
> Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
> Expressions.box(ONE;
> BlockBuilder nested = new BlockBuilder(true, b);
> Expression multiMapNested = nested.append("multiMap",
> Expressions.new_(Types.of(HashMap.class)));
> nested.add(Expressions.statement(
> Expressions.call(multiMapNested, putMethod, Expressions.box(TWO), 
> Expressions.box(TWO;
> nested.add(Expressions.call(multiMapNested, sizeMethod));
> b.add(nested.toBlock());
> b.append(Expressions.call(multiMapParent, sizeMethod));
> // It is wrong output. Map should be reused
> assertEquals(
> "{\n"
> + "  final java.util.HashMap multiMap = new 
> java.util.HashMap();\n"
> + "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
> + "  {\n"
> + "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
> + "return multiMap.size();\n"
> + "  }\n"
> + "  return multiMap.size();\n"
> + "}\n",
> b.toBlock().toString());
>   }
> {code}
> Are there any performance tests that prove that this optimization 
> significantly improves performance?
> [1] 
> https://github.com/dssysolyatin/calcite/commit/626a5f48ef9e69e543aeec277a4f38000a190b10



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


[jira] [Commented] (CALCITE-5427) Provide code quality/coverage metrics with SonarCloud and JaCoCo

2022-12-09 Thread Stamatis Zampetakis (Jira)


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

Stamatis Zampetakis commented on CALCITE-5427:
--

Example of how things will look when the analysis is done on PR: 
https://github.com/zabetak/calcite/pull/9

> Provide code quality/coverage metrics with SonarCloud and JaCoCo
> 
>
> Key: CALCITE-5427
> URL: https://issues.apache.org/jira/browse/CALCITE-5427
> Project: Calcite
>  Issue Type: Improvement
>  Components: build
>Reporter: Stamatis Zampetakis
>Assignee: Stamatis Zampetakis
>Priority: Major
> Fix For: 1.33.0
>
>
> Provide code quality and coverage metrics for the master branch and every PR 
> to facilitate code reviews, improve security, and protect against regressions 
> by exploiting SonarCloud and JaCoCo.
> [SonarCloud|https://sonarcloud.io/] is popular platform for providing code 
> quality insights and widely used by [ASF 
> projects|https://sonarcloud.io/organizations/apache/projects].
> [JaCoCo|https://www.jacoco.org/jacoco/trunk/index.html] relies on 
> instrumentation via a java agent to gather code coverage information while 
> running tests.
> The integration in the Calcite build can be done via Gradle plugins and the 
> analysis will be triggered automatically via GitHub actions. Appropriate 
> configuration properties will be added ensure that local builds will not be 
> impacted.
> Initially, the code quality metrics will have purely informational character 
> and will be at the discretion of the reviewer to enforce or ignore them.
> Later on, we can decide and agree on code quality gates which can be enforced 
> automatically by SonarCloud and JaCoCo. It is possible to enforce things 
> mostly at CI level but certain things can also be applied on local builds.



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


[jira] [Commented] (CALCITE-5427) Provide code quality/coverage metrics with SonarCloud and JaCoCo

2022-12-09 Thread Stamatis Zampetakis (Jira)


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

Stamatis Zampetakis commented on CALCITE-5427:
--

An example of how things will look like when the integration is done can be 
seen here: https://sonarcloud.io/summary/overall?id=org.apache.calcite%3Acalcite

> Provide code quality/coverage metrics with SonarCloud and JaCoCo
> 
>
> Key: CALCITE-5427
> URL: https://issues.apache.org/jira/browse/CALCITE-5427
> Project: Calcite
>  Issue Type: Improvement
>  Components: build
>Reporter: Stamatis Zampetakis
>Assignee: Stamatis Zampetakis
>Priority: Major
> Fix For: 1.33.0
>
>
> Provide code quality and coverage metrics for the master branch and every PR 
> to facilitate code reviews, improve security, and protect against regressions 
> by exploiting SonarCloud and JaCoCo.
> [SonarCloud|https://sonarcloud.io/] is popular platform for providing code 
> quality insights and widely used by [ASF 
> projects|https://sonarcloud.io/organizations/apache/projects].
> [JaCoCo|https://www.jacoco.org/jacoco/trunk/index.html] relies on 
> instrumentation via a java agent to gather code coverage information while 
> running tests.
> The integration in the Calcite build can be done via Gradle plugins and the 
> analysis will be triggered automatically via GitHub actions. Appropriate 
> configuration properties will be added ensure that local builds will not be 
> impacted.
> Initially, the code quality metrics will have purely informational character 
> and will be at the discretion of the reviewer to enforce or ignore them.
> Later on, we can decide and agree on code quality gates which can be enforced 
> automatically by SonarCloud and JaCoCo. It is possible to enforce things 
> mostly at CI level but certain things can also be applied on local builds.



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


[jira] [Created] (CALCITE-5427) Provide code quality/coverage metrics with SonarCloud and JaCoCo

2022-12-09 Thread Stamatis Zampetakis (Jira)
Stamatis Zampetakis created CALCITE-5427:


 Summary: Provide code quality/coverage metrics with SonarCloud and 
JaCoCo
 Key: CALCITE-5427
 URL: https://issues.apache.org/jira/browse/CALCITE-5427
 Project: Calcite
  Issue Type: Improvement
  Components: build
Reporter: Stamatis Zampetakis
Assignee: Stamatis Zampetakis
 Fix For: 1.33.0


Provide code quality and coverage metrics for the master branch and every PR to 
facilitate code reviews, improve security, and protect against regressions by 
exploiting SonarCloud and JaCoCo.

[SonarCloud|https://sonarcloud.io/] is popular platform for providing code 
quality insights and widely used by [ASF 
projects|https://sonarcloud.io/organizations/apache/projects].

[JaCoCo|https://www.jacoco.org/jacoco/trunk/index.html] relies on 
instrumentation via a java agent to gather code coverage information while 
running tests.

The integration in the Calcite build can be done via Gradle plugins and the 
analysis will be triggered automatically via GitHub actions. Appropriate 
configuration properties will be added ensure that local builds will not be 
impacted.

Initially, the code quality metrics will have purely informational character 
and will be at the discretion of the reviewer to enforce or ignore them.

Later on, we can decide and agree on code quality gates which can be enforced 
automatically by SonarCloud and JaCoCo. It is possible to enforce things mostly 
at CI level but certain things can also be applied on local builds.



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


[jira] [Updated] (CALCITE-5426) BlockBuilder should not reuse mutable objects

2022-12-09 Thread Dmitry Sysolyatin (Jira)


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

Dmitry Sysolyatin updated CALCITE-5426:
---
Description: 
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example [1]:
{code:java}
  @Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapNested, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any performance tests that prove that this optimization significantly 
improves performance?

[1] 
https://github.com/dssysolyatin/calcite/commit/626a5f48ef9e69e543aeec277a4f38000a190b10

  was:
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example [1]:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any performance tests that prove that this optimization significantly 
improves performance?

[1] 
https://github.com/dssysolyatin/calcite/commit/a7bedd6595ebe69bd93e9fcc3f05850da5f673b2


> BlockBuilder should not reuse mutable objects
> -
>
> Key: CALCITE-5426
> URL: https://issues.apache.org/jira/browse/CALCITE-5426
> Project: Calcite
>  Issue Type: Bug
>  Components: linq4j
>Affects Versions: 1.32.0
>Reporter: Dmitry Sysolyatin
>Priority: Major
>
> Inside BlockBuilder there is an optimization that replaces an expression with 
> a variable if the expressions are equal and have the final modifier.
> But this optimization can cause problems when used with a mutable objects 
> (One of the problems has been found in CALCITE-5388):
> For example [1]:
> {code:java}
>   @Test void testReuseCollectionExpression() throws NoSuchMethodException {
> Method putMethod = HashMap.class.getMethod("put", Object.class, 
> Object.class);
> Method sizeMethod = 

[jira] [Updated] (CALCITE-5426) BlockBuilder should not reuse mutable objects

2022-12-09 Thread Dmitry Sysolyatin (Jira)


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

Dmitry Sysolyatin updated CALCITE-5426:
---
Description: 
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example [1]:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any performance tests that prove that this optimization significantly 
improves performance?

[1] 
https://github.com/dssysolyatin/calcite/commit/a7bedd6595ebe69bd93e9fcc3f05850da5f673b2

  was:
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example [1]:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any tests that prove that this optimization significantly improves 
performance?

[1] 
https://github.com/dssysolyatin/calcite/commit/a7bedd6595ebe69bd93e9fcc3f05850da5f673b2


> BlockBuilder should not reuse mutable objects
> -
>
> Key: CALCITE-5426
> URL: https://issues.apache.org/jira/browse/CALCITE-5426
> Project: Calcite
>  Issue Type: Bug
>  Components: linq4j
>Affects Versions: 1.32.0
>Reporter: Dmitry Sysolyatin
>Priority: Major
>
> Inside BlockBuilder there is an optimization that replaces an expression with 
> a variable if the expressions are equal and have the final modifier.
> But this optimization can cause problems when used with a mutable objects 
> (One of the problems has been found in CALCITE-5388):
> For example [1]:
> {code:java}
> @Test void testReuseCollectionExpression() throws NoSuchMethodException {
> Method putMethod = HashMap.class.getMethod("put", Object.class, 
> Object.class);
> Method sizeMethod = 

[jira] [Updated] (CALCITE-5426) BlockBuilder should not reuse mutable objects

2022-12-09 Thread Dmitry Sysolyatin (Jira)


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

Dmitry Sysolyatin updated CALCITE-5426:
---
Description: 
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example [1]:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any tests that prove that this optimization significantly improves 
performance?

[1] 
https://github.com/dssysolyatin/calcite/commit/a7bedd6595ebe69bd93e9fcc3f05850da5f673b2

  was:
Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any tests that prove that this optimization significantly improves 
performance?


> BlockBuilder should not reuse mutable objects
> -
>
> Key: CALCITE-5426
> URL: https://issues.apache.org/jira/browse/CALCITE-5426
> Project: Calcite
>  Issue Type: Bug
>  Components: linq4j
>Affects Versions: 1.32.0
>Reporter: Dmitry Sysolyatin
>Priority: Major
>
> Inside BlockBuilder there is an optimization that replaces an expression with 
> a variable if the expressions are equal and have the final modifier.
> But this optimization can cause problems when used with a mutable objects 
> (One of the problems has been found in CALCITE-5388):
> For example [1]:
> {code:java}
> @Test void testReuseCollectionExpression() throws NoSuchMethodException {
> Method putMethod = HashMap.class.getMethod("put", Object.class, 
> Object.class);
> Method sizeMethod = HashMap.class.getMethod("size");
> 
> Expression multiMapParent = b.append("multiMap", 
> 

[jira] [Created] (CALCITE-5426) BlockBuilder should not reuse mutable objects

2022-12-09 Thread Dmitry Sysolyatin (Jira)
Dmitry Sysolyatin created CALCITE-5426:
--

 Summary: BlockBuilder should not reuse mutable objects
 Key: CALCITE-5426
 URL: https://issues.apache.org/jira/browse/CALCITE-5426
 Project: Calcite
  Issue Type: Bug
  Components: linq4j
Affects Versions: 1.32.0
Reporter: Dmitry Sysolyatin


Inside BlockBuilder there is an optimization that replaces an expression with a 
variable if the expressions are equal and have the final modifier.

But this optimization can cause problems when used with a mutable objects (One 
of the problems has been found in CALCITE-5388):

For example:
{code:java}
@Test void testReuseCollectionExpression() throws NoSuchMethodException {
Method putMethod = HashMap.class.getMethod("put", Object.class, 
Object.class);
Method sizeMethod = HashMap.class.getMethod("size");

Expression multiMapParent = b.append("multiMap", 
Expressions.new_(Types.of(HashMap.class)));
b.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(ONE), 
Expressions.box(ONE;

BlockBuilder nested = new BlockBuilder(true, b);
Expression multiMapNested = nested.append("multiMap",
Expressions.new_(Types.of(HashMap.class)));
nested.add(Expressions.statement(
Expressions.call(multiMapParent, putMethod, Expressions.box(TWO), 
Expressions.box(TWO;
nested.add(Expressions.call(multiMapNested, sizeMethod));

b.add(nested.toBlock());
b.append(Expressions.call(multiMapParent, sizeMethod));

// It is wrong output. Map should be reused
assertEquals(
"{\n"
+ "  final java.util.HashMap multiMap = new java.util.HashMap();\n"
+ "  multiMap.put(Integer.valueOf(1), Integer.valueOf(1));\n"
+ "  {\n"
+ "multiMap.put(Integer.valueOf(2), Integer.valueOf(2));\n"
+ "return multiMap.size();\n"
+ "  }\n"
+ "  return multiMap.size();\n"
+ "}\n",
b.toBlock().toString());
  }
{code}

Are there any tests that prove that this optimization significantly improves 
performance?



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


[jira] [Commented] (CALCITE-5390) RelDecorrelator throws NullPointerException

2022-12-09 Thread Dmitry Sysolyatin (Jira)


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

Dmitry Sysolyatin commented on CALCITE-5390:


[~FrankZou] Hi! Do you have any progress with this ticket ?

> RelDecorrelator throws NullPointerException
> ---
>
> Key: CALCITE-5390
> URL: https://issues.apache.org/jira/browse/CALCITE-5390
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Dmitry Sysolyatin
>Assignee: Zou
>Priority: Major
>
> The current query throws NullPointerException
> {code:java}
> SELECT
>   (SELECT 1 FROM emp d WHERE d.job = a.job LIMIT 1) AS t1,
>   (SELECT a.job = 'PRESIDENT' FROM emp s LIMIT 1) as t2
> FROM emp a;
> {code}
> Test case - 
> [https://github.com/apache/calcite/commit/46fe9bc456f2d34cf7dccd29829c9e85abe69d5f]
> Logical plan before it fails:
> {code:java}
> LogicalProject(T1=[$8], T2=[$9])
>   LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], 
> SAL=[$5], COMM=[$6], DEPTNO=[$7], $f0=[$8], $f09=[$9])
>     LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], 
> SAL=[$5], COMM=[$6], DEPTNO=[$7], $f0=[$8], $f00=[$10])
>       LogicalCorrelate(correlation=[$cor0], joinType=[left], 
> requiredColumns=[{9}])
>         LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], 
> HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], $f0=[$8], $f9=[=($2, 
> 'PRESIDENT')])
>           LogicalCorrelate(correlation=[$cor0], joinType=[left], 
> requiredColumns=[{2}])
>             LogicalTableScan(table=[[scott, EMP]])
>             LogicalAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)])
>               LogicalSort(fetch=[1])
>                 LogicalProject(EXPR$0=[1])
>                   LogicalFilter(condition=[=($2, $cor0.JOB)])
>                     LogicalTableScan(table=[[scott, EMP]])
>         LogicalAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)])
>           LogicalSort(fetch=[1])
>             LogicalProject(EXPR$0=[$cor0.$f9])
>               LogicalTableScan(table=[[scott, EMP]]) {code}
> Stack trace:
> {code:java}
>  Caused by: java.lang.NullPointerException
>   at java.util.Objects.requireNonNull(Objects.java:203)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.createValueGenerator(RelDecorrelator.java:833)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateInputWithValueGenerator(RelDecorrelator.java:1028)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:764)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:738)
>   at sun.reflect.GeneratedMethodAccessor9.invoke(Unknown Source)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.lang.reflect.Method.invoke(Method.java:498)
>   at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:707)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:464)
>   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>   at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.lang.reflect.Method.invoke(Method.java:498)
>   at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:707)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:512)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:495)
>   at sun.reflect.GeneratedMethodAccessor13.invoke(Unknown Source)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.lang.reflect.Method.invoke(Method.java:498)
>   at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.getInvoke(RelDecorrelator.java:707)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1187)
>   at 
> org.apache.calcite.sql2rel.RelDecorrelator.decorrelateRel(RelDecorrelator.java:1169)
>   at sun.reflect.GeneratedMethodAccessor12.invoke(Unknown Source)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.lang.reflect.Method.invoke(Method.java:498)
>   at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:531)
>   at 
> 

[jira] [Assigned] (CALCITE-5425) Should not pushdown filter through aggregate with an empty groupset

2022-12-09 Thread Zou (Jira)


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

Zou reassigned CALCITE-5425:


Assignee: Zou

> Should not pushdown filter through aggregate with an empty groupset
> ---
>
> Key: CALCITE-5425
> URL: https://issues.apache.org/jira/browse/CALCITE-5425
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Reporter: Steve Carlin
>Assignee: Zou
>Priority: Major
>
> If we are given a query 
> SELECT count(x) FROM tbl HAVING false;
> This query should produce an empty set.  
> We should not allow a filter to pass through this aggregate.  When the 
> aggregate has no group by, there is always one implied group (for the whole 
> dataset).  So if we apply the filter before the aggregate, the implied group 
> will still be created.  This is not what we want, since the filter should 
> produce an empty set.



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