Calcite-Master - Build # 1048 - Failure

[Apache Jenkins Server]

The Apache Jenkins build system has built Calcite-Master (build #1048)

Status: Failure

Check console output at https://builds.apache.org/job/Calcite-Master/1048/ to 
view the results.

[GitHub] zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code 
gen error for UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#discussion_r261213393
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
 ##
 @@ -2168,7 +2167,14 @@ public Expression implement(
 RexCall call,
 List translatedOperands) {
   final Expression operand = translatedOperands.get(0);
-  final UnaryExpression e = Expressions.makeUnary(expressionType, operand);
+
+  final Expression e;
+  if (expressionType == ExpressionType.Negate && operand.type == 
BigDecimal.class) {
+e = Expressions.call(BuiltInMethod.UNARY_MINUS.method, operand);
 
 Review comment:
   
   Have you taken a look at the "backup method" convention of the 
`BinaryImplementor`? Is that also suitable here?
   
   
https://github.com/apache/calcite/blob/280642a02a4bcfd1fb9cbe8c5ab672d3619860e7/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java#L2090


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zabetak merged pull request #1059: [CALCITE-2859] Centralize Calcite system properties

[GitBox]

zabetak merged pull request #1059: [CALCITE-2859] Centralize Calcite system 
properties
URL: https://github.com/apache/calcite/pull/1059
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[YuZhao Chan]

Good job Kevin! Hope to see a cleaner PR lists.

new life!
在 2019年2月28日 +0800 AM3:36，dev@calcite.apache.org，写道：
>
> Good idea Kevin!

[GitHub] zhztheplayer commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH function

[GitBox]

zhztheplayer commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH 
function
URL: https://github.com/apache/calcite/pull/1070#issuecomment-468251438
 
 
   @XuQianJin-Stars Sorry for the delaying, will take a look soon.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code 
gen error for UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#discussion_r261217611
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/ReflectiveSchemaTest.java
 ##
 @@ -625,6 +637,12 @@ private void check(ResultSetMetaData metaData, String 
columnName,
   private void checkOp(CalciteAssert.AssertThat with, String fn) {
 for (Field field : EveryType.numericFields()) {
   for (Field field2 : EveryType.numericFields()) {
+//Decimal +-*/ others types has bug see CALCITE-2861
+//when CALCITE-2861 was solved, remove this code logic
 
 Review comment:
   There is a util class[1] for recording unresolved bug programmatically. You 
may want to use that rather than doing something like this.
   
   [1] 
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/util/Bug.java


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Michael Mior]

Thanks Kevin! That's an important task and one that few people are
willing to take on :)

And thanks Hongze for the pointer to Stale. Especially since other
Apache project are already using it, I'd be inclined to have a
discussion on the appropriate configuration and give it a go.
Personally, I think the list of open PRs should things that are
actively being worked on. Closed PRs can always be reopened anyway, so
I don't think we're losing anything.

--
Michael Mior
mm...@apache.org

Le mer. 27 févr. 2019 à 14:36, Kevin Risden  a écrit :
>
> There are 105 open pull requests against apache/calcite repo [1]. There are
> only 48 Calcite JIRAs labeled with pull-request-available [2].
>
> I'm planning to go through in the next few days and make sure that we have
> PRs that match open JIRAs and are labeled pull-request-available. If there
> are PRs that are open for JIRAs that are closed, planning to close those
> PRs with a comment.
>
> [1] https://github.com/apache/calcite/pulls
> [2]
> https://issues.apache.org/jira/issues/?jql=project%20%3D%20CALCITE%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20pull-request-available%20ORDER%20BY%20priority%20DESC
>
> Kevin Risden

Re: [DISCUSS] Towards Calcite 1.19.0

[YuZhao Chan]

I would help to review some PRs this weekend，especially [1]. Hope to help and 
release some of the burden.
[1] 
https://issues.apache.org/jira/browse/CALCITE-2018?jql=project%20%3D%20CALCITE%20AND%20resolution%20%3D%20Unresolved%20AND%20fixVersion%20in%20(EMPTY%2C%20%22next%22)%20AND%20labels%20%3D%20pull-request-available%20ORDER%20BY%20priority%20DESC

Best,
YuZhao Chan
在 2019年2月26日 +0800 AM8:14，Julian Hyde ，写道：
> Hey everyone.
>
> There are 108 open pull requests. What are we going to do about it?
>
> Last release I reviewed and committed dozens of pull requests, and I burned 
> out.
>
> Julian
>
>
> > On Feb 22, 2019, at 1:20 PM, Kevin Risden  wrote:
> >
> > Yea I don't mind pushing out the RC towards the end of next week (beginning
> > of March). I'm not in a rush to build the RC. I just picked Monday to try
> > to hit the target of February that was arbitrarily picked in the sand. At
> > this point, the release will most likely happen in early March.
> >
> > Kevin Risden
> >
> >
> > On Fri, Feb 22, 2019 at 4:15 PM Julian Hyde  wrote:
> >
> > > Can you do the RC towards the end of next week?
> > >
> > > I only just saw this message - 4 days after you sent it - because I’ve
> > > been fighting down a backlog of 500 Apache messages. There are some 
> > > changes
> > > I would like to get into the release but I will need a few days.
> > >
> > > Julian
> > >
> > >
> > > > On Feb 18, 2019, at 9:56 AM,
> > > Kevin Risden
> > >  wrote:
> > > >
> > > > It looks like there are some PRs to be reviewed and some changes to get
> > > > merged in this week.
> > > >
> > > > How does creating the first RC on Monday Feb 25th sound? Does that give
> > > > enough time this week to get changes into Calcite 1.19.0?
> > > >
> > > > Kevin Risden
> > > >
> > > >
> > > > On Wed, Feb 13, 2019 at 11:08 AM Zoltan Haindrich  wrote:
> > > >
> > > > >
> > > > > On 2/13/19 4:24 PM, Julian Hyde wrote:
> > > > > > Sorry, there’s been a misunderstanding. Let me clarify. I didn’t say
> > > > > that your patches were too small. Or intend to imply it.
> > > > > >
> > > > > > When I said “widespread changes for no good reason” - or something 
> > > > > > like
> > > > > that - I meant changes to the RexNode format due to removing IS TRUE
> > > nodes.
> > > > > >
> > > > > > I like small patches, provided each one fixes a well defined issue 
> > > > > > and
> > > > > has appropriate tests.
> > > > >
> > > > > From now on I will try to provide a testcase when opening the jira.
> > > > > Sorry for the misunderstanding, thank you for making it clear!
> > > > >
> > > > > >
> > > > > > Julian
> > > > > >
> > > > > > > On Feb 13, 2019, at 3:40 AM, Zoltan Haindrich  wrote:
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > In Hive I'm a little bit behind in upgrading to 1.18 and although 
> > > > > > > the
> > > > > upgrade would not cause any correctness issues; but in a sense it's 
> > > > > more
> > > > > conservative in doing some simplifications - which could be 
> > > > > interpreted
> > > as
> > > > > regressions; if we take that into account that even the plan could get
> > > > > worse.
> > > > > > >
> > > > > > > I've a few patches almost ready - they are very small changes
> > > (actually
> > > > > Julian mentioned that they are kinda too small, so next time I will 
> > > > > not
> > > be
> > > > > opening separate jiras for them)
> > > > > > >
> > > > > > > I will finish them and launch a custom Hive test with the latest
> > > master
> > > > > to see if there are any new issues coming from that direction.
> > > > > > > I should get the results for it by tomorrow.
> > > > > > >
> > > > > > > cheers,
> > > > > > > Zoltan
> > > > > > >
> > > > > > >
> > > > > > > > On 2/12/19 11:30 PM, Stamatis Zampetakis wrote:
> > > > > > > > I was not suggesting changing the release process. I wanted 
> > > > > > > > just to
> > > > > > > > highlight the fact that if the aforementioned tickets are not 
> > > > > > > > part of
> > > > > 1.19
> > > > > > > > I will have to create an unofficial bundle which includes them 
> > > > > > > > in
> > > > > order to
> > > > > > > > keep the downstream project working. Sorry for the confusion.
> > > > > > > > Στις Τρί, 12 Φεβ 2019 στις 8:56 μ.μ., ο/η Julian Hyde <
> > > > > jh...@apache.org>
> > > > > > > > έγραψε:
> > > > > > > > > Stamatis,
> > > > > > > > >
> > > > > > > > > We’ve so far managed to avoid making patch releases. It keeps 
> > > > > > > > > life
> > > > > simpler
> > > > > > > > > if all releases are from the main line. And simple is 
> > > > > > > > > important,
> > > > > given that
> > > > > > > > > there are no salaried release or QA engineers working on 
> > > > > > > > > Calcite.
> > > > > > > > >
> > > > > > > > > But as part of that contract, we commit to making releases 
> > > > > > > > > from main
> > > > > line
> > > > > > > > > frequently and regularly. Hopefully 1.19 will arrive soon 
> > > > > > > > > enough for
> > > > > your
> > > > > > > > > purposes.
> > > > > > > > >

[GitHub] XuQianJin-Stars commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH function

[GitBox]

XuQianJin-Stars commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH 
function
URL: https://github.com/apache/calcite/pull/1070#issuecomment-468268420
 
 
   > Hi @XuQianJin-Stars, could you please address Michael's comment? By 
looking into MySQL's JSON_LENGTH spec[1], I also think it's no need to 
implement error behavior and wrapper behavior for this function.
   > 
   > [1] 
https://dev.mysql.com/doc/refman/8.0/en/json-attribute-functions.html#function_json-length
   
   Well, I'll change it


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] vlsi commented on a change in pull request #1059: [CALCITE-2859] Centralize Calcite system properties

[GitBox]

vlsi commented on a change in pull request #1059: [CALCITE-2859] Centralize 
Calcite system properties
URL: https://github.com/apache/calcite/pull/1059#discussion_r261129603
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
 ##
 @@ -0,0 +1,323 @@
+/*
+ * 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.config;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.AccessControlException;
+import java.util.Locale;
+import java.util.Properties;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+/**
+ * A Calcite specific system property that is used to configure various 
aspects of the framework.
+ *
+ * Calcite system properties must always be in the "calcite" root 
namespace.
+ *
+ * @param  the type of the property value
+ */
+public final class CalciteSystemProperty {
+  /**
+   * Holds all system properties related with the Calcite.
+   *
+   * Deprecated "saffron.properties" (in namespaces"saffron" 
and "net.sf.saffron")
+   * are also kept here but under "calcite" namespace.
+   */
+  private static final Properties PROPERTIES = loadProperties();
+  /**
+   * Whether to run Calcite in debug mode.
+   *
+   * When debug mode is activated significantly more information is 
gathered and printed to
+   * STDOUT. It is most commonly used to print and identify problems in 
generated java code. Debug
+   * mode is also used to perform more verifications at runtime, which are not 
performed during
+   * normal execution.
+   */
+  public static final CalciteSystemProperty DEBUG =
+  booleanProperty("calcite.debug", false);
+  /**
+   * Whether to exploit join commutative property.
+   */
+  // TODO review zabetak:
+  // Does the property control join commutativity or rather join 
associativity? The property is
+  // associated with {@link org.apache.calcite.rel.rules.JoinAssociateRule} 
and not with
+  // {@link org.apache.calcite.rel.rules.JoinCommuteRule}.
+  public static final CalciteSystemProperty COMMUTE =
+  booleanProperty("calcite.enable.join.commute", false);
+  /**
+   *  Whether to follow the SQL standard strictly.
+   */
+  public static final CalciteSystemProperty STRICT =
+  booleanProperty("calcite.strict.sql", false);
+  /**
+   * Whether to include a GraphViz representation when dumping the state of 
the Volcano planner.
+   */
+  public static final CalciteSystemProperty DUMP_GRAPHVIZ =
+  booleanProperty("calcite.volcano.dump.graphviz", true);
+  /**
+   * Whether to include RelSet information when dumping the state 
of the Volcano
+   * planner.
+   */
+  public static final CalciteSystemProperty DUMP_SETS =
+  booleanProperty("calcite.volcano.dump.sets", true);
+  /**
+   * Whether to run integration tests.
+   */
+  // TODO review zabetak:
+  // The property is used in only one place and it is associated with mongodb. 
Should we drop this
+  // property and just use TEST_MONGODB?
+  public static final CalciteSystemProperty INTEGRATION_TEST =
+  booleanProperty("calcite.integrationTest", false);
+
+  /**
+   * Which database to use for tests that require a JDBC data source.
+   *
+   * The property can take one of the following values:
+   * 
+   *   HSQLDB(default)
+   *   H2
+   *   MYSQL
+   *   ORACLE
+   *   POSTGRESQL
+   * 
+   * If the specified value is not included in the previous list the default 
is used.
+   *
+   * We recommend that casual users use hsqldb, and frequent Calcite 
developers use MySQL.
+   * The test suite runs faster against the MySQL database (mainly because of 
the 0.1s versus 6s
+   * startup time). You have to populate MySQL manually with the foodmart data 
set, otherwise there
+   * will be test failures.
+   * */
+  public static final CalciteSystemProperty TEST_DB =
+  stringProperty("calcite.test.db", "HSQLDB",
+  ImmutableSet.of(
+  "HSQLDB",
+  "H2",
+  "MYSQL",
+  "ORACLE",
+  "POSTGRESQL"));
+
+  /**
+   * Path to the dataset file that should used for integration tests.
+   *
+   

[GitHub] zabetak commented on a change in pull request #1059: [CALCITE-2859] Centralize Calcite system properties

[GitBox]

zabetak commented on a change in pull request #1059: [CALCITE-2859] Centralize 
Calcite system properties
URL: https://github.com/apache/calcite/pull/1059#discussion_r261139530
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
 ##
 @@ -0,0 +1,323 @@
+/*
+ * 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.config;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.AccessControlException;
+import java.util.Locale;
+import java.util.Properties;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+/**
+ * A Calcite specific system property that is used to configure various 
aspects of the framework.
+ *
+ * Calcite system properties must always be in the "calcite" root 
namespace.
+ *
+ * @param  the type of the property value
+ */
+public final class CalciteSystemProperty {
+  /**
+   * Holds all system properties related with the Calcite.
+   *
+   * Deprecated "saffron.properties" (in namespaces"saffron" 
and "net.sf.saffron")
+   * are also kept here but under "calcite" namespace.
+   */
+  private static final Properties PROPERTIES = loadProperties();
+  /**
+   * Whether to run Calcite in debug mode.
+   *
+   * When debug mode is activated significantly more information is 
gathered and printed to
+   * STDOUT. It is most commonly used to print and identify problems in 
generated java code. Debug
+   * mode is also used to perform more verifications at runtime, which are not 
performed during
+   * normal execution.
+   */
+  public static final CalciteSystemProperty DEBUG =
+  booleanProperty("calcite.debug", false);
+  /**
+   * Whether to exploit join commutative property.
+   */
+  // TODO review zabetak:
+  // Does the property control join commutativity or rather join 
associativity? The property is
+  // associated with {@link org.apache.calcite.rel.rules.JoinAssociateRule} 
and not with
+  // {@link org.apache.calcite.rel.rules.JoinCommuteRule}.
+  public static final CalciteSystemProperty COMMUTE =
+  booleanProperty("calcite.enable.join.commute", false);
+  /**
+   *  Whether to follow the SQL standard strictly.
+   */
+  public static final CalciteSystemProperty STRICT =
+  booleanProperty("calcite.strict.sql", false);
+  /**
+   * Whether to include a GraphViz representation when dumping the state of 
the Volcano planner.
+   */
+  public static final CalciteSystemProperty DUMP_GRAPHVIZ =
+  booleanProperty("calcite.volcano.dump.graphviz", true);
+  /**
+   * Whether to include RelSet information when dumping the state 
of the Volcano
+   * planner.
+   */
+  public static final CalciteSystemProperty DUMP_SETS =
+  booleanProperty("calcite.volcano.dump.sets", true);
+  /**
+   * Whether to run integration tests.
+   */
+  // TODO review zabetak:
+  // The property is used in only one place and it is associated with mongodb. 
Should we drop this
+  // property and just use TEST_MONGODB?
+  public static final CalciteSystemProperty INTEGRATION_TEST =
+  booleanProperty("calcite.integrationTest", false);
+
+  /**
+   * Which database to use for tests that require a JDBC data source.
+   *
+   * The property can take one of the following values:
+   * 
+   *   HSQLDB(default)
+   *   H2
+   *   MYSQL
+   *   ORACLE
+   *   POSTGRESQL
+   * 
+   * If the specified value is not included in the previous list the default 
is used.
+   *
+   * We recommend that casual users use hsqldb, and frequent Calcite 
developers use MySQL.
+   * The test suite runs faster against the MySQL database (mainly because of 
the 0.1s versus 6s
+   * startup time). You have to populate MySQL manually with the foodmart data 
set, otherwise there
+   * will be test failures.
+   * */
+  public static final CalciteSystemProperty TEST_DB =
+  stringProperty("calcite.test.db", "HSQLDB",
+  ImmutableSet.of(
+  "HSQLDB",
+  "H2",
+  "MYSQL",
+  "ORACLE",
+  "POSTGRESQL"));
+
+  /**
+   * Path to the dataset file that should used for integration tests.
+   *
+ 

[GitHub] zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code 
gen error for UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#discussion_r261213916
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/ReflectiveSchemaTest.java
 ##
 @@ -953,13 +974,13 @@ public IntAndString(int id, String value) {
 false, (byte) 0, (char) 0, (short) 0, 0, 0L, 0F, 0D,
 false, (byte) 0, (char) 0, (short) 0, 0, 0L, 0F, 0D,
 new java.sql.Date(0), new Time(0), new Timestamp(0),
-new Date(0), "1"),
+new Date(0), "1", BigDecimal.valueOf(-1)),
 new EveryType(
 true, Byte.MAX_VALUE, Character.MAX_VALUE, Short.MAX_VALUE,
 Integer.MAX_VALUE, Long.MAX_VALUE, Float.MAX_VALUE,
 Double.MAX_VALUE,
 null, null, null, null, null, null, null, null,
-null, null, null, null, null),
+null, null, null, null, null, new BigDecimal("1.2")),
 
 Review comment:
   I suggest to change `new BigDecimal("1.2"))` to `null` here.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH function

[GitBox]

zhztheplayer commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH 
function
URL: https://github.com/apache/calcite/pull/1070#issuecomment-468267253
 
 
   Hi @XuQianJin-Stars, could you please address Michael's comment? By looking 
into MySQL's JSON_LENGTH spec[1], I also think it's no need to implement error 
behavior and wrapper behavior for this function.
   
   [1] 
https://dev.mysql.com/doc/refman/8.0/en/json-attribute-functions.html#function_json-length


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer commented on a change in pull request #1057: [CALCITE-2854] code 
gen error for UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#discussion_r261215687
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/ReflectiveSchemaTest.java
 ##
 @@ -625,6 +637,12 @@ private void check(ResultSetMetaData metaData, String 
columnName,
   private void checkOp(CalciteAssert.AssertThat with, String fn) {
 for (Field field : EveryType.numericFields()) {
   for (Field field2 : EveryType.numericFields()) {
+//Decimal +-*/ others types has bug see CALCITE-2861
 
 Review comment:
   Two comments here:
   1. I suggest to add a space after `//`.
   2. Is using the word `arithmetic` better than `+-*/`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] XuQianJin-Stars commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH function

[GitBox]

XuQianJin-Stars commented on issue #1070: [CALCITE-2808]Add the JSON_LENGTH 
function
URL: https://github.com/apache/calcite/pull/1070#issuecomment-468221805
 
 
   hi @michaelmior  @zhztheplayer Thank you very much. I hope that you can take 
the time to review this PR.
   best
   qianjin


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zabetak merged pull request #950: [CALCITE-2703] Reduce code generation and class loading overhead when executing queries in the EnumerableConvention (Stamatis Zampetakis)

[GitBox]

zabetak merged pull request #950: [CALCITE-2703] Reduce code generation and 
class loading overhead when executing queries in the EnumerableConvention 
(Stamatis Zampetakis)
URL: https://github.com/apache/calcite/pull/950
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kgyrtkirk opened a new pull request #1076: [CALCITE-2421] Generalize simplification of self-comparisions

[GitBox]

kgyrtkirk opened a new pull request #1076: [CALCITE-2421] Generalize 
simplification of self-comparisions
URL: https://github.com/apache/calcite/pull/1076
 
 
   This change enables to rewrite comparisions between the same operands to a 
form which relys on the nullability check of the original expression.
   For example `x=x` is rewritten to `null or x is not null`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer edited a comment on issue #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer edited a comment on issue #1057: [CALCITE-2854] code gen error for 
UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#issuecomment-468292958
 
 
   @yuqi1129 I've added some comments to the PR. Besides, please link your 
commit's email address to you Github account to let the community confirm the 
authorship of the changes.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer edited a comment on issue #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer edited a comment on issue #1057: [CALCITE-2854] code gen error for 
UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#issuecomment-468292958
 
 
   @yuqi1129 I've added some comments to the PR. Besides, please link your 
commit's email address to you Github account to let the community confirm the 
authorship of the changes.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: [calcite] branch master updated: [CALCITE-2827] Allow Convention.NONE planning with VolcanoPlanner

[Michael Mior]

Laurent did answer my objection and indicate he's willing to revert.
I'm ok with the provided explanation. (Although an unrelated note,
things like "Fix checkstyle error" and "Fix grammar errors" should not
be part of the final commit message.)
--
Michael Mior
mm...@apache.org

Le mer. 27 févr. 2019 à 16:16, Julian Hyde  a écrit :
>
> Laurent,
>
> Michael had started reviewing this and had some objections. Please don’t 
> summarily commit.
>
> Julian
>
>
> > On Feb 27, 2019, at 1:07 PM, laur...@apache.org wrote:
> >
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > laurent pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/calcite.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new e3a319e  [CALCITE-2827] Allow Convention.NONE planning with 
> > VolcanoPlanner
> > e3a319e is described below
> >
> > commit e3a319e1f79bc807327bd443433c50c4bbf20866
> > Author: Juhwan Kim 
> > AuthorDate: Thu Feb 21 14:31:51 2019 -0800
> >
> >[CALCITE-2827] Allow Convention.NONE planning with VolcanoPlanner
> >
> >By default, cost for Convention.NONE is infinity.
> >- Add option to allow Convention.None planning with VolcanoPlanner
> >- Add a unit test
> >
> >Change-Id: Idcee2ca923c9b10b58cc9352a390a87f899b64ee
> >
> >[CALCITE-2827] Fix checkstyle error
> >
> >Change-Id: I50702e6d50d9fe916e15b5bd42f8ec7b12bf0c6f
> >
> >[CALCITE-2827] Fix grammar errors
> >
> >Change-Id: Icca105184888ff9cc01b2b71f597f3fbd235b638
> > ---
> > .../calcite/plan/volcano/VolcanoPlanner.java   | 18 +--
> > .../plan/volcano/VolcanoPlannerTraitTest.java  | 36 
> > ++
> > 2 files changed, 52 insertions(+), 2 deletions(-)
> >
> > diff --git 
> > a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java 
> > b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
> > index bf8336d..54f5049 100644
> > --- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
> > +++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
> > @@ -227,6 +227,11 @@ public class VolcanoPlanner extends 
> > AbstractRelOptPlanner {
> >*/
> >   private boolean locked;
> >
> > +  /**
> > +   * Whether rels with Convention.NONE has infinite cost.
> > +   */
> > +  private boolean noneConventionHasInfiniteCost = true;
> > +
> >   private final List materializations =
> >   new ArrayList<>();
> >
> > @@ -931,13 +936,22 @@ public class VolcanoPlanner extends 
> > AbstractRelOptPlanner {
> > }
> >   }
> >
> > +  /**
> > +   * Sets whether this planner should consider rel nodes with 
> > Convention.NONE
> > +   * to have inifinte cost or not.
> > +   * @param infinite Whether to make none convention rel nodes inifite cost
> > +   */
> > +  public void setNoneConventionHasInfiniteCost(boolean infinite) {
> > +this.noneConventionHasInfiniteCost = infinite;
> > +  }
> > +
> >   public RelOptCost getCost(RelNode rel, RelMetadataQuery mq) {
> > assert rel != null : "pre-condition: rel != null";
> > if (rel instanceof RelSubset) {
> >   return ((RelSubset) rel).bestCost;
> > }
> > -if (rel.getTraitSet().getTrait(ConventionTraitDef.INSTANCE)
> > -== Convention.NONE) {
> > +if (noneConventionHasInfiniteCost
> > +&& rel.getTraitSet().getTrait(ConventionTraitDef.INSTANCE) == 
> > Convention.NONE) {
> >   return costFactory.makeInfiniteCost();
> > }
> > RelOptCost cost = mq.getNonCumulativeCost(rel);
> > diff --git 
> > a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
> >  
> > b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
> > index 66704ed..2c42b3c 100644
> > --- 
> > a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
> > +++ 
> > b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
> > @@ -258,6 +258,21 @@ public class VolcanoPlannerTraitTest {
> > assertTrue(child instanceof PhysLeafRel);
> >   }
> >
> > +  @Test public void testPlanWithNoneConvention() {
> > +VolcanoPlanner planner = new VolcanoPlanner();
> > +planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
> > +RelOptCluster cluster = newCluster(planner);
> > +NoneTinyLeafRel leaf = new NoneTinyLeafRel(cluster, "noneLeafRel");
> > +planner.setRoot(leaf);
> > +RelOptCost cost = planner.getCost(leaf, RelMetadataQuery.instance());
> > +
> > +assertTrue(cost.isInfinite());
> > +
> > +planner.setNoneConventionHasInfiniteCost(false);
> > +cost = planner.getCost(leaf, RelMetadataQuery.instance());
> > +assertTrue(!cost.isInfinite());
> > +  }
> > +
> >   //~ Inner Classes 
> > --
> >
> >   /** Implementation of {@link RelTrait} for testing. */
> > @@ -553,6 +568,27 @@ public class VolcanoPlannerTraitTest {

[GitHub] zhztheplayer closed pull request #984: [CALCITE-2593] Sometimes fails to plan when a RelNode transform multi…

[GitBox]

zhztheplayer closed pull request #984: [CALCITE-2593] Sometimes fails to plan 
when a RelNode transform multi…
URL: https://github.com/apache/calcite/pull/984
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on issue #984: [CALCITE-2593] Sometimes fails to plan when a RelNode transform multi…

[GitBox]

zhztheplayer commented on issue #984: [CALCITE-2593] Sometimes fails to plan 
when a RelNode transform multi…
URL: https://github.com/apache/calcite/pull/984#issuecomment-468300249
 
 
   Going to close this PR -- let's wait for coming into consensus in JIRA.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] hsyuan opened a new pull request #1077: [CALCITE-2818] EXTRACT returns wrong results for DATE and TIMESTAMP values before epoch

[GitBox]

hsyuan opened a new pull request #1077: [CALCITE-2818] EXTRACT returns wrong 
results for DATE and TIMESTAMP values before epoch
URL: https://github.com/apache/calcite/pull/1077
 
 
   JIRA: https://issues.apache.org/jira/browse/CALCITE-2818


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on issue #1057: [CALCITE-2854] code gen error for UNARY_MINUS operator call with deci…

[GitBox]

zhztheplayer commented on issue #1057: [CALCITE-2854] code gen error for 
UNARY_MINUS operator call with deci…
URL: https://github.com/apache/calcite/pull/1057#issuecomment-468292958
 
 
   @yuqi1129 I've added some comments to the PR. Besides, please link your 
commit's email address to you Github account to let community confirm the 
authorship of the changes.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer closed pull request #897: [CALCITE-2648] Output collation of EnumerableWindow is not consistent…

[GitBox]

zhztheplayer closed pull request #897: [CALCITE-2648] Output collation of 
EnumerableWindow is not consistent…
URL: https://github.com/apache/calcite/pull/897
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer commented on issue #897: [CALCITE-2648] Output collation of EnumerableWindow is not consistent…

[GitBox]

zhztheplayer commented on issue #897: [CALCITE-2648] Output collation of 
EnumerableWindow is not consistent…
URL: https://github.com/apache/calcite/pull/897#issuecomment-468301076
 
 
   I have no cue to solve the problem perfectly, closing temporarily.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] zhztheplayer edited a comment on issue #897: [CALCITE-2648] Output collation of EnumerableWindow is not consistent…

[GitBox]

zhztheplayer edited a comment on issue #897: [CALCITE-2648] Output collation of 
EnumerableWindow is not consistent…
URL: https://github.com/apache/calcite/pull/897#issuecomment-468301076
 
 
   I have no cue to solve the problem perfectly for now, closing temporarily.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema constructor public to pass client cache object

[GitBox]

asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema 
constructor public to pass client cache object
URL: https://github.com/apache/calcite/pull/1071#issuecomment-468109254
 
 
   Either this or #1036 should be merged depending on result of discussion on 
(https://issues.apache.org/jira/browse/CALCITE-2877).
   Should complex objects (Connection, RestClient, GeodeCache etc.) be allowed 
in public constructors in Schemas. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on a change in pull request #1031: CALCITE-2742: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

siddharthteotia commented on a change in pull request #1031: CALCITE-2742: 
Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261378419
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
 ##
 @@ -1966,6 +1967,20 @@ public static TimeZone timeZone(DataContext root) {
 return (TimeZone) DataContext.Variable.TIME_ZONE.get(root);
   }
 
+  /** SQL {@code USER} function. */
+  @NonDeterministic
 
 Review comment:
   Thanks. Yes this should be deterministic. It was initially non-deterministic 
 to be consistent with other functions in the class 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema constructor public to pass client cache object

[GitBox]

asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema 
constructor public to pass client cache object
URL: https://github.com/apache/calcite/pull/1071#issuecomment-468109254
 
 
   Either this or #1022 should be merged depending on result of discussion on 
(https://issues.apache.org/jira/browse/CALCITE-2877).
   Should complex objects (Connection, RestClient, GeodeCache etc.) be allowed 
in public constructors in Schemas. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Julian Hyde]

-1 Using a robot to close stale PRs is solving the wrong problem.

The main reason that we have a lot of open PRs is that we - as a community of 
committers - are not putting sufficient time into reviewing. I think that we 
are giving PR submitters poor service, and they are being very patient with us.

Every PR is a potential new community member, and not one but many improvements 
to Calcite. Our goal, as committers, should be to ensure that we don’t let any 
of those opportunities slip.

We all know that there are a few poor quality PRs. They take more effort to 
review, and bang into shape, than it would have taken the reviewer to write it 
themselves. Comments are misspelled, and the code just looks sloppy. Those, 
frankly, are not very high priority.

But at the other end of the spectrum, there are some - many - high quality PRs, 
where the contributor has done their homework. They are rarely perfect on the 
first submission, but you can see that the person behind it would make the 
community much stronger. Those PRs deserve our immediate attention. 

And I’ll say this just once. I get frustrated when committers prioritize PRs 
from employees of their own company, and I get furious when they hold those PRs 
to a lower standard than other PRs. As a committer, you need to wear your 
Apache hat, and only your Apache hat, when reviewing code.

Julian




> On Feb 28, 2019, at 6:28 AM, Michael Mior  wrote:
> 
> Thanks Kevin! That's an important task and one that few people are
> willing to take on :)
> 
> And thanks Hongze for the pointer to Stale. Especially since other
> Apache project are already using it, I'd be inclined to have a
> discussion on the appropriate configuration and give it a go.
> Personally, I think the list of open PRs should things that are
> actively being worked on. Closed PRs can always be reopened anyway, so
> I don't think we're losing anything.
> 
> --
> Michael Mior
> mm...@apache.org
> 
> Le mer. 27 févr. 2019 à 14:36, Kevin Risden  a écrit :
>> 
>> There are 105 open pull requests against apache/calcite repo [1]. There are
>> only 48 Calcite JIRAs labeled with pull-request-available [2].
>> 
>> I'm planning to go through in the next few days and make sure that we have
>> PRs that match open JIRAs and are labeled pull-request-available. If there
>> are PRs that are open for JIRAs that are closed, planning to close those
>> PRs with a comment.
>> 
>> [1] https://github.com/apache/calcite/pulls
>> [2]
>> https://issues.apache.org/jira/issues/?jql=project%20%3D%20CALCITE%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20pull-request-available%20ORDER%20BY%20priority%20DESC
>> 
>> Kevin Risden

[GitHub] asereda-gs commented on issue #1036: [CALCITE-2815] GeodeSchemaFactory change to pass in clientCache using jndi

[GitBox]

asereda-gs commented on issue #1036: [CALCITE-2815] GeodeSchemaFactory change 
to pass in clientCache using jndi
URL: https://github.com/apache/calcite/pull/1036#issuecomment-468429549
 
 
   If we go JNDI route (see [dev 
list](https://lists.apache.org/thread.html/8c255842f5c8885d772c27546d5fe12dd4e2f2d124ac6ad221e47519@%3Cdev.calcite.apache.org%3E))
 I would like to review this PR once again


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Francis Chuang]

I am also -1 on the bot per the reasons Julian has stated.

Our problem is mainly due to PRs not being reviewed. I think an 
extension to the problem is that because PRs take a while to be 
reviewed, contributors sometime lose context (and can't find time to 
realign themselves with the current state of the codebase) or move on to 
another company where the PR is no longer relevant to their new role, 
causing PRs to be abandoned.


From what I can see on the jira and Github, most PRs just require a bit 
of polish to become mergable. As a contributor, it would be 
disheartening to spend time investigating a bug or feature, work on a PR 
and have it closed because the project did not have enough resources to 
review it (I've had this happen on other opensource projects using stale 
bots). In addition, in cases where feedback was given, but the PR was 
abandoned by the original contributor, there's often scope for another 
contributor to "carry" the PR and the amount of work needed to make it 
mergable is not too onerous.


I'd prefer to manually close PRs and JIRAs where the change does not 
meet the goals of the project, is of too low quality or have diverged 
from master so much (due to age) that creating a PR from scratch is more 
manageable.


On 1/03/2019 6:19 am, Julian Hyde wrote:

-1 Using a robot to close stale PRs is solving the wrong problem.

The main reason that we have a lot of open PRs is that we - as a community of 
committers - are not putting sufficient time into reviewing. I think that we 
are giving PR submitters poor service, and they are being very patient with us.

Every PR is a potential new community member, and not one but many improvements 
to Calcite. Our goal, as committers, should be to ensure that we don’t let any 
of those opportunities slip.

We all know that there are a few poor quality PRs. They take more effort to 
review, and bang into shape, than it would have taken the reviewer to write it 
themselves. Comments are misspelled, and the code just looks sloppy. Those, 
frankly, are not very high priority.

But at the other end of the spectrum, there are some - many - high quality PRs, 
where the contributor has done their homework. They are rarely perfect on the 
first submission, but you can see that the person behind it would make the 
community much stronger. Those PRs deserve our immediate attention.

And I’ll say this just once. I get frustrated when committers prioritize PRs 
from employees of their own company, and I get furious when they hold those PRs 
to a lower standard than other PRs. As a committer, you need to wear your 
Apache hat, and only your Apache hat, when reviewing code.

Julian





On Feb 28, 2019, at 6:28 AM, Michael Mior  wrote:

Thanks Kevin! That's an important task and one that few people are
willing to take on :)

And thanks Hongze for the pointer to Stale. Especially since other
Apache project are already using it, I'd be inclined to have a
discussion on the appropriate configuration and give it a go.
Personally, I think the list of open PRs should things that are
actively being worked on. Closed PRs can always be reopened anyway, so
I don't think we're losing anything.

--
Michael Mior
mm...@apache.org

Le mer. 27 févr. 2019 à 14:36, Kevin Risden  a écrit :


There are 105 open pull requests against apache/calcite repo [1]. There are
only 48 Calcite JIRAs labeled with pull-request-available [2].

I'm planning to go through in the next few days and make sure that we have
PRs that match open JIRAs and are labeled pull-request-available. If there
are PRs that are open for JIRAs that are closed, planning to close those
PRs with a comment.

[1] https://github.com/apache/calcite/pulls
[2]
https://issues.apache.org/jira/issues/?jql=project%20%3D%20CALCITE%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20pull-request-available%20ORDER%20BY%20priority%20DESC

Kevin Risden

[GitHub] siddharthteotia commented on a change in pull request #1031: CALCITE-2742: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

siddharthteotia commented on a change in pull request #1031: CALCITE-2742: 
Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261378248
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
 ##
 @@ -175,6 +177,58 @@ private void checkConstant(final Object operand,
 });
   }
 
+  @Test public void testUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.USER,
+   DataContext.Variable.USER, "happyCalciteUser");
+  }
+
+  @Test public void testSystemUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.SYSTEM_USER,
+   DataContext.Variable.SYSTEM_USER, "");
+  }
+
+  @Test public void testTimestampFromContext() throws Exception {
+// current timestamp currently rounds to the nearest second (?)
+long val = System.currentTimeMillis() / 1000 * 1000;
+testContextLiteral(SqlStdOperatorTable.CURRENT_TIMESTAMP,
+   DataContext.Variable.CURRENT_TIMESTAMP, val);
+  }
+
+  /**
+   * Ensure that for a given context operator, the correct value is retrieved 
from the DataContext.
 
 Review comment:
   done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on a change in pull request #1031: CALCITE-2742: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

siddharthteotia commented on a change in pull request #1031: CALCITE-2742: 
Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261378419
 
 

 ##
 File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
 ##
 @@ -1966,6 +1967,20 @@ public static TimeZone timeZone(DataContext root) {
 return (TimeZone) DataContext.Variable.TIME_ZONE.get(root);
   }
 
+  /** SQL {@code USER} function. */
+  @NonDeterministic
 
 Review comment:
   Thanks. Yes this should be deterministic. It was done to be consistent with 
other functions in the class 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on a change in pull request #1031: CALCITE-2742: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

siddharthteotia commented on a change in pull request #1031: CALCITE-2742: 
Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261378619
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
 ##
 @@ -175,6 +177,58 @@ private void checkConstant(final Object operand,
 });
   }
 
+  @Test public void testUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.USER,
+   DataContext.Variable.USER, "happyCalciteUser");
+  }
 
 Review comment:
   done.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on a change in pull request #1059: [CALCITE-2859] Centralize Calcite system properties

[GitBox]

julianhyde commented on a change in pull request #1059: [CALCITE-2859] 
Centralize Calcite system properties
URL: https://github.com/apache/calcite/pull/1059#discussion_r261350210
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
 ##
 @@ -0,0 +1,323 @@
+/*
+ * 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.config;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.AccessControlException;
+import java.util.Locale;
+import java.util.Properties;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+/**
+ * A Calcite specific system property that is used to configure various 
aspects of the framework.
+ *
+ * Calcite system properties must always be in the "calcite" root 
namespace.
+ *
+ * @param  the type of the property value
+ */
+public final class CalciteSystemProperty {
+  /**
+   * Holds all system properties related with the Calcite.
+   *
+   * Deprecated "saffron.properties" (in namespaces"saffron" 
and "net.sf.saffron")
+   * are also kept here but under "calcite" namespace.
+   */
+  private static final Properties PROPERTIES = loadProperties();
+  /**
+   * Whether to run Calcite in debug mode.
+   *
+   * When debug mode is activated significantly more information is 
gathered and printed to
+   * STDOUT. It is most commonly used to print and identify problems in 
generated java code. Debug
+   * mode is also used to perform more verifications at runtime, which are not 
performed during
+   * normal execution.
+   */
+  public static final CalciteSystemProperty DEBUG =
+  booleanProperty("calcite.debug", false);
+  /**
+   * Whether to exploit join commutative property.
+   */
+  // TODO review zabetak:
+  // Does the property control join commutativity or rather join 
associativity? The property is
+  // associated with {@link org.apache.calcite.rel.rules.JoinAssociateRule} 
and not with
+  // {@link org.apache.calcite.rel.rules.JoinCommuteRule}.
+  public static final CalciteSystemProperty COMMUTE =
+  booleanProperty("calcite.enable.join.commute", false);
+  /**
+   *  Whether to follow the SQL standard strictly.
+   */
+  public static final CalciteSystemProperty STRICT =
+  booleanProperty("calcite.strict.sql", false);
+  /**
+   * Whether to include a GraphViz representation when dumping the state of 
the Volcano planner.
+   */
+  public static final CalciteSystemProperty DUMP_GRAPHVIZ =
+  booleanProperty("calcite.volcano.dump.graphviz", true);
+  /**
+   * Whether to include RelSet information when dumping the state 
of the Volcano
+   * planner.
+   */
+  public static final CalciteSystemProperty DUMP_SETS =
+  booleanProperty("calcite.volcano.dump.sets", true);
+  /**
+   * Whether to run integration tests.
+   */
+  // TODO review zabetak:
+  // The property is used in only one place and it is associated with mongodb. 
Should we drop this
+  // property and just use TEST_MONGODB?
+  public static final CalciteSystemProperty INTEGRATION_TEST =
+  booleanProperty("calcite.integrationTest", false);
+
+  /**
+   * Which database to use for tests that require a JDBC data source.
+   *
+   * The property can take one of the following values:
+   * 
+   *   HSQLDB(default)
+   *   H2
+   *   MYSQL
+   *   ORACLE
+   *   POSTGRESQL
+   * 
+   * If the specified value is not included in the previous list the default 
is used.
+   *
+   * We recommend that casual users use hsqldb, and frequent Calcite 
developers use MySQL.
+   * The test suite runs faster against the MySQL database (mainly because of 
the 0.1s versus 6s
+   * startup time). You have to populate MySQL manually with the foodmart data 
set, otherwise there
+   * will be test failures.
+   * */
+  public static final CalciteSystemProperty TEST_DB =
+  stringProperty("calcite.test.db", "HSQLDB",
+  ImmutableSet.of(
+  "HSQLDB",
+  "H2",
+  "MYSQL",
+  "ORACLE",
+  "POSTGRESQL"));
+
+  /**
+   * Path to the dataset file that should used for integration tests.
+   

[GitHub] asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema constructor public to pass client cache object

[GitBox]

asereda-gs edited a comment on issue #1071: [CALCITE-2877] Make GeodeSchema 
constructor public to pass client cache object
URL: https://github.com/apache/calcite/pull/1071#issuecomment-468109254
 
 
   Either this or #1036 should be merged depending on result of discussion on 
([CALCITE-2877](https://issues.apache.org/jira/browse/CALCITE-2877)).
   
   Should complex objects (Connection, RestClient, GeodeCache etc.) be allowed 
in public constructors in Schemas ? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on issue #1029: [CALCITE-2824] Fix invalid usage of RexExecutorImpl

[GitBox]

siddharthteotia commented on issue #1029: [CALCITE-2824] Fix invalid usage of 
RexExecutorImpl
URL: https://github.com/apache/calcite/pull/1029#issuecomment-468441305
 
 
   @vlsi , @laurentgo , I am not sure how exactly we can test test this. The 
code change here avoids the direct casing of RexExecutor into a RexExecutorImpl 
instance and enhances the interface with an implies() method. Right now there 
is only one implementation of RexExecutor interface and this method has been 
implemented there 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on issue #811: Add LGTM code quality badges

[GitBox]

risdenk commented on issue #811: Add LGTM code quality badges
URL: https://github.com/apache/calcite/pull/811#issuecomment-468460341
 
 
   Not sure how I feel about this one. Based on 
https://issues.apache.org/jira/browse/INFRA-17226 LGTM can't be integrated with 
the build. Since INFRA is unable to add LGTM to builds due to repository access 
requirements, that would make this like an advertisement that we aren't 
actually using.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on issue #811: Add LGTM code quality badges

[GitBox]

risdenk commented on issue #811: Add LGTM code quality badges
URL: https://github.com/apache/calcite/pull/811#issuecomment-468472598
 
 
   Closing this PR due to no immediate need to move forward. If LGTM and Apache 
legal due to Github permissions end up working together we can look at this 
again.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk closed pull request #811: Add LGTM code quality badges

[GitBox]

risdenk closed pull request #811: Add LGTM code quality badges
URL: https://github.com/apache/calcite/pull/811
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on issue #1034: [CALCITE-2819] Add version of LoptOptimizeJoinRule that uses the first ordering it finds

[GitBox]

siddharthteotia commented on issue #1034: [CALCITE-2819] Add version of 
LoptOptimizeJoinRule that uses the first ordering it finds
URL: https://github.com/apache/calcite/pull/1034#issuecomment-468449888
 
 
   @vlsi ... we use this initially only to get some random ordering (not 
possibly the best ordering) to convert multi-join to join nodes.. There is 
probably no other way in Calcite to do that so added this unoptimized instance 
where we just return after getting the first ordering. So the test case also 
aims to verify if the result is the ordering by considering first join factor 
as the first factor.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] vlsi commented on issue #784: [CALCITE-2456] fix VolcanoRuleCall#match for unordered child operand

[GitBox]

vlsi commented on issue #784: [CALCITE-2456] fix VolcanoRuleCall#match for 
unordered child operand
URL: https://github.com/apache/calcite/pull/784#issuecomment-468457409
 
 
   Yes it is good and committable.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Michael Mior]

Julian,

Overall, I agree with you Julian and Francis. Ideally, we would find
the resources among committers to review all PRs. My +1 for the bot
mainly came from a desire to keep the list of open PRs as ones which
are actively being worked on. Perhaps many of the same goals would be
accomplished by a one-time cleanup of existing abandoned PRs.

I think having the PRs on GitHub better managed would go a long way to
encouraging people to review them. For example, if you're reviewing a
PR with the intent of eventually merging, assign it to yourself. If
any committer could easily find a list of recent, unassigned PRs, it
would make reviewing much easier.

--
Michael Mior
mm...@apache.org

Le jeu. 28 févr. 2019 à 14:19, Julian Hyde  a écrit :
>
> -1 Using a robot to close stale PRs is solving the wrong problem.
>
> The main reason that we have a lot of open PRs is that we - as a community of 
> committers - are not putting sufficient time into reviewing. I think that we 
> are giving PR submitters poor service, and they are being very patient with 
> us.
>
> Every PR is a potential new community member, and not one but many 
> improvements to Calcite. Our goal, as committers, should be to ensure that we 
> don’t let any of those opportunities slip.
>
> We all know that there are a few poor quality PRs. They take more effort to 
> review, and bang into shape, than it would have taken the reviewer to write 
> it themselves. Comments are misspelled, and the code just looks sloppy. 
> Those, frankly, are not very high priority.
>
> But at the other end of the spectrum, there are some - many - high quality 
> PRs, where the contributor has done their homework. They are rarely perfect 
> on the first submission, but you can see that the person behind it would make 
> the community much stronger. Those PRs deserve our immediate attention.
>
> And I’ll say this just once. I get frustrated when committers prioritize PRs 
> from employees of their own company, and I get furious when they hold those 
> PRs to a lower standard than other PRs. As a committer, you need to wear your 
> Apache hat, and only your Apache hat, when reviewing code.
>
> Julian
>
>
>
>
> > On Feb 28, 2019, at 6:28 AM, Michael Mior  wrote:
> >
> > Thanks Kevin! That's an important task and one that few people are
> > willing to take on :)
> >
> > And thanks Hongze for the pointer to Stale. Especially since other
> > Apache project are already using it, I'd be inclined to have a
> > discussion on the appropriate configuration and give it a go.
> > Personally, I think the list of open PRs should things that are
> > actively being worked on. Closed PRs can always be reopened anyway, so
> > I don't think we're losing anything.
> >
> > --
> > Michael Mior
> > mm...@apache.org
> >
> > Le mer. 27 févr. 2019 à 14:36, Kevin Risden  a écrit :
> >>
> >> There are 105 open pull requests against apache/calcite repo [1]. There are
> >> only 48 Calcite JIRAs labeled with pull-request-available [2].
> >>
> >> I'm planning to go through in the next few days and make sure that we have
> >> PRs that match open JIRAs and are labeled pull-request-available. If there
> >> are PRs that are open for JIRAs that are closed, planning to close those
> >> PRs with a comment.
> >>
> >> [1] https://github.com/apache/calcite/pulls
> >> [2]
> >> https://issues.apache.org/jira/issues/?jql=project%20%3D%20CALCITE%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20pull-request-available%20ORDER%20BY%20priority%20DESC
> >>
> >> Kevin Risden
>

Generating javadoc using Docker

[Siddharth Teotia]

Hi,

I am following the instructions here
https://github.com/apache/calcite/tree/master/site to generate java doc for
one of my patches and running into some dependency issues. Can I get some
help in identifying the problem?

I am able to build Calcite locally. However, after running the step
docker-compose run generate-javadoc, I see the following errors:

Failed to execute goal
org.apache.maven.plugins:maven-site-plugin:3.7.1:site (default-site) on
project calcite: failed to get report for
org.apache.maven.plugins:maven-javadoc-plugin: Failed to execute goal on
project calcite-babel: Could not resolve dependencies for project
org.apache.calcite:calcite-babel:jar:1.19.0-SNAPSHOT: Failure to find
org.apache.calcite:calcite-core:jar:tests:1.19.0-SNAPSHOT

I would appreciate the help.

Thanks,
Siddharth

Re: Generating javadoc using Docker

[Francis Chuang]

I wonder if this is because is because it uses the maven:alpine image, 
which uses the openjdk-8 alpine image.


Can you try changing this line 
(https://github.com/apache/calcite/blob/master/site/docker-compose.yml#L31) 
to just "maven" and see if it builds?


Franics

On 1/03/2019 9:23 am, Siddharth Teotia wrote:

Hi,

I am following the instructions here
https://github.com/apache/calcite/tree/master/site to generate java doc for
one of my patches and running into some dependency issues. Can I get some
help in identifying the problem?

I am able to build Calcite locally. However, after running the step
docker-compose run generate-javadoc, I see the following errors:

Failed to execute goal
org.apache.maven.plugins:maven-site-plugin:3.7.1:site (default-site) on
project calcite: failed to get report for
org.apache.maven.plugins:maven-javadoc-plugin: Failed to execute goal on
project calcite-babel: Could not resolve dependencies for project
org.apache.calcite:calcite-babel:jar:1.19.0-SNAPSHOT: Failure to find
org.apache.calcite:calcite-core:jar:tests:1.19.0-SNAPSHOT

I would appreciate the help.

Thanks,
Siddharth

[GitHub] julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove 
Aggregate if grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#discussion_r261429146
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java
 ##
 @@ -58,37 +68,63 @@ public AggregateRemoveRule(Class 
aggregateClass,
 // distinct to make it correct up-front, we can get rid of the reference
 // to the child here.
 super(
-operand(aggregateClass,
+operandJ(aggregateClass, null, agg -> isAggregateSupported(agg),
 operand(RelNode.class, any())),
 relBuilderFactory, null);
   }
 
+  private static boolean isAggregateSupported(Aggregate aggregate) {
+if (aggregate.indicator
+|| aggregate.getGroupType() != Aggregate.Group.SIMPLE) {
+  return false;
+}
+// If any aggregate functions do not support splitting, bail out
+for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+  if (aggregateCall.filterArg >= 0
+  || aggregateCall.getAggregation()
+  .unwrap(SqlSplittableAggFunction.class) == null) {
+return false;
+  }
+}
+return true;
+  }
+
   //~ Methods 
 
   public void onMatch(RelOptRuleCall call) {
 final Aggregate aggregate = call.rel(0);
 final RelNode input = call.rel(1);
-if (!aggregate.getAggCallList().isEmpty() || aggregate.indicator) {
-  return;
-}
 final RelMetadataQuery mq = call.getMetadataQuery();
 if (!SqlFunctions.isTrue(mq.areColumnsUnique(input, 
aggregate.getGroupSet( {
   return;
 }
-// Distinct is "GROUP BY c1, c2" (where c1, c2 are a set of columns on
-// which the input is unique, i.e. contain a key) and has no aggregate
-// functions. It can be removed.
-final RelNode newInput = convert(input, 
aggregate.getTraitSet().simplify());
 
-// If aggregate was projecting a subset of columns, add a project for the
-// same effect.
+final RexBuilder rexBuilder = aggregate.getCluster().getRexBuilder();
 
 Review comment:
   an easier way to get rexBuilder is from relBuilder; just initialize 
relBuilder a few lines earlier


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove 
Aggregate if grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#discussion_r261428663
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java
 ##
 @@ -58,37 +68,63 @@ public AggregateRemoveRule(Class 
aggregateClass,
 // distinct to make it correct up-front, we can get rid of the reference
 // to the child here.
 super(
-operand(aggregateClass,
+operandJ(aggregateClass, null, agg -> isAggregateSupported(agg),
 operand(RelNode.class, any())),
 relBuilderFactory, null);
   }
 
+  private static boolean isAggregateSupported(Aggregate aggregate) {
+if (aggregate.indicator ||
+aggregate.getGroupType() != Aggregate.Group.SIMPLE) {
+  return false;
+}
+// If any aggregate functions do not support splitting, bail out
+for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+  if (aggregateCall.filterArg >= 0 ||
+  aggregateCall.getAggregation()
+  .unwrap(SqlSplittableAggFunction.class) == null) {
+return false;
+  }
+}
+return true;
+  }
+
   //~ Methods 
 
   public void onMatch(RelOptRuleCall call) {
 final Aggregate aggregate = call.rel(0);
 final RelNode input = call.rel(1);
-if (!aggregate.getAggCallList().isEmpty() || aggregate.indicator) {
-  return;
-}
 final RelMetadataQuery mq = call.getMetadataQuery();
 if (!SqlFunctions.isTrue(mq.areColumnsUnique(input, 
aggregate.getGroupSet( {
   return;
 }
-// Distinct is "GROUP BY c1, c2" (where c1, c2 are a set of columns on
-// which the input is unique, i.e. contain a key) and has no aggregate
-// functions. It can be removed.
-final RelNode newInput = convert(input, 
aggregate.getTraitSet().simplify());
 
-// If aggregate was projecting a subset of columns, add a project for the
-// same effect.
+final RexBuilder rexBuilder = aggregate.getCluster().getRexBuilder();
+final List projects = new LinkedList<>();
+for (AggregateCall aggCall : aggregate.getAggCallList()) {
+  final SqlAggFunction aggregation = aggCall.getAggregation();
+  final SqlSplittableAggFunction splitter =
+  Objects.requireNonNull(
+  aggregation.unwrap(SqlSplittableAggFunction.class));
+  final RexNode singleton = splitter.singleton(
+  rexBuilder, input.getRowType(), aggCall);
+  projects.add(singleton);
+}
+
+final RelNode newInput = convert(input, 
aggregate.getTraitSet().simplify());
 final RelBuilder relBuilder = call.builder();
 relBuilder.push(newInput);
-if (newInput.getRowType().getFieldCount()
+if (!projects.isEmpty()) {
+  projects.addAll(0, relBuilder.fields(aggregate.getGroupSet().asList()));
+  relBuilder.project(projects);
+} else if (newInput.getRowType().getFieldCount()
 > aggregate.getRowType().getFieldCount()) {
+  // If aggregate was projecting a subset of columns, and there were no
+  // aggregate functions, add a project for the same effect.
   relBuilder.project(relBuilder.fields(aggregate.getGroupSet().asList()));
 }
-call.transformTo(relBuilder.build());
+final RelNode newRel = relBuilder.build();
+call.transformTo(newRel);
 
 Review comment:
   the variable doesn't seem necessary


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Vladimir Sitnikov]

Francis> Our problem is mainly due to PRs not being reviewed.

Once upon a time I did try to pass over the PRs, and I added some randoms
here and there.
I'm not sure what others think of that, however we have:

17 "returned-with-feedback":
https://github.com/apache/calcite/pulls?q=is%3Apr+is%3Aopen+label%3Areturned-with-feedback
6 with pending discussion in JIRA:
https://github.com/apache/calcite/pulls?utf8=%E2%9C%93=is%3Apr+is%3Aopen+label%3Adiscussion-in-jira

I like "LGTM-will-merge-soon" as a soft warning to other committers that
"hey, either you chime in or I just commit this stuff".
We have 2 by the way:
https://github.com/apache/calcite/labels/LGTM-will-merge-soon

Vladimir

[GitHub] vlsi commented on issue #1034: [CALCITE-2819] Add version of LoptOptimizeJoinRule that uses the first ordering it finds

[GitBox]

vlsi commented on issue #1034: [CALCITE-2819] Add version of 
LoptOptimizeJoinRule that uses the first ordering it finds
URL: https://github.com/apache/calcite/pull/1034#issuecomment-468458376
 
 
   Where do you get MultiJoins in the first place?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Kevin Risden]

One thing that I think would help is to add a Github PR template [1]. We
did this for Apache Knox to help make sure that PRs follow some simple
guidelines. After the gitbox migration, PRs are automatically linked to
JIRA (which is good). I think we just had some PRs prior to gitbox
migration that didn't have the autolink part.

[1]
https://help.github.com/en/articles/creating-a-pull-request-template-for-your-repository
[2] https://issues.apache.org/jira/browse/KNOX-1760

Kevin Risden


On Thu, Feb 28, 2019 at 4:07 PM Vladimir Sitnikov <
sitnikov.vladi...@gmail.com> wrote:

> Francis> Our problem is mainly due to PRs not being reviewed.
>
> Once upon a time I did try to pass over the PRs, and I added some randoms
> here and there.
> I'm not sure what others think of that, however we have:
>
> 17 "returned-with-feedback":
>
> https://github.com/apache/calcite/pulls?q=is%3Apr+is%3Aopen+label%3Areturned-with-feedback
> 6 with pending discussion in JIRA:
>
> https://github.com/apache/calcite/pulls?utf8=%E2%9C%93=is%3Apr+is%3Aopen+label%3Adiscussion-in-jira
>
> I like "LGTM-will-merge-soon" as a soft warning to other committers that
> "hey, either you chime in or I just commit this stuff".
> We have 2 by the way:
> https://github.com/apache/calcite/labels/LGTM-will-merge-soon
>
> Vladimir
>

Re: JIRAs and Pull Requests Cleanup

[Julian Hyde]

Two things that we are doing that are working:
 * Use the “pull-request-available” tag for JIRA cases;
 * Beat down the number of open cases with the “pull-request-available” tag in 
the lead up to the release.

Let’s continue doing these.

Shall we agree a target of number of open cases with “pull-request-available” 
that we want to aim for before RC 1 of 1.19? And perhaps a date that we want to 
achieve it by?

Right now there are 92[1]. I propose that we get to 40 by Wednesday.

I’m not against the PR template, or the Stale bot, but let’s have that 
discussion after the release. Right now, there’s s**t to be shoveled, and I 
thank those of you who have picked up a shovel. (Especially Kevin.)

Julian

[1] 
https://issues.apache.org/jira/issues/?jql=project%20%3D%20CALCITE%20%26%20labels%20%3D%20pull-request-available%20%26%20statusCategory%20%3D%20%22To%20Do%22%20
 

 


> On Feb 28, 2019, at 2:25 PM, Kevin Risden  wrote:
> 
> As of now, the open PRs and Calcite JIRAs are close enough to matching (96
> vs 98). (There are a few for Avatica in the JIRA query). Thanks all those
> that helped clean up.
> 
> Kevin Risden
> 
> 
> On Thu, Feb 28, 2019 at 4:21 PM Kevin Risden  wrote:
> 
>> One thing that I think would help is to add a Github PR template [1]. We
>> did this for Apache Knox to help make sure that PRs follow some simple
>> guidelines. After the gitbox migration, PRs are automatically linked to
>> JIRA (which is good). I think we just had some PRs prior to gitbox
>> migration that didn't have the autolink part.
>> 
>> [1]
>> https://help.github.com/en/articles/creating-a-pull-request-template-for-your-repository
>> [2] https://issues.apache.org/jira/browse/KNOX-1760
>> 
>> Kevin Risden
>> 
>> 
>> On Thu, Feb 28, 2019 at 4:07 PM Vladimir Sitnikov <
>> sitnikov.vladi...@gmail.com> wrote:
>> 
>>> Francis> Our problem is mainly due to PRs not being reviewed.
>>> 
>>> Once upon a time I did try to pass over the PRs, and I added some randoms
>>> here and there.
>>> I'm not sure what others think of that, however we have:
>>> 
>>> 17 "returned-with-feedback":
>>> 
>>> https://github.com/apache/calcite/pulls?q=is%3Apr+is%3Aopen+label%3Areturned-with-feedback
>>> 6 with pending discussion in JIRA:
>>> 
>>> https://github.com/apache/calcite/pulls?utf8=%E2%9C%93=is%3Apr+is%3Aopen+label%3Adiscussion-in-jira
>>> 
>>> I like "LGTM-will-merge-soon" as a soft warning to other committers that
>>> "hey, either you chime in or I just commit this stuff".
>>> We have 2 by the way:
>>> https://github.com/apache/calcite/labels/LGTM-will-merge-soon
>>> 
>>> Vladimir
>>> 
>> 

[GitHub] julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove 
Aggregate if grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#discussion_r261430620
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
 ##
 @@ -3619,6 +3620,69 @@ private void transitiveInference(RelOptRule... 
extraRules) throws Exception {
 checkPlanUnchanged(new HepPlanner(program), sql);
   }
 
+  @Test public void testAggregateRemove1() {
 
 Review comment:
   Describe what the test is trying to achieve, and why.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove 
Aggregate if grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#discussion_r261429313
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java
 ##
 @@ -58,37 +68,63 @@ public AggregateRemoveRule(Class 
aggregateClass,
 // distinct to make it correct up-front, we can get rid of the reference
 // to the child here.
 super(
-operand(aggregateClass,
+operandJ(aggregateClass, null, agg -> isAggregateSupported(agg),
 operand(RelNode.class, any())),
 relBuilderFactory, null);
   }
 
+  private static boolean isAggregateSupported(Aggregate aggregate) {
+if (aggregate.indicator
+|| aggregate.getGroupType() != Aggregate.Group.SIMPLE) {
+  return false;
+}
+// If any aggregate functions do not support splitting, bail out
+for (AggregateCall aggregateCall : aggregate.getAggCallList()) {
+  if (aggregateCall.filterArg >= 0
+  || aggregateCall.getAggregation()
+  .unwrap(SqlSplittableAggFunction.class) == null) {
+return false;
+  }
+}
+return true;
+  }
+
   //~ Methods 
 
   public void onMatch(RelOptRuleCall call) {
 final Aggregate aggregate = call.rel(0);
 final RelNode input = call.rel(1);
-if (!aggregate.getAggCallList().isEmpty() || aggregate.indicator) {
-  return;
-}
 final RelMetadataQuery mq = call.getMetadataQuery();
 if (!SqlFunctions.isTrue(mq.areColumnsUnique(input, 
aggregate.getGroupSet( {
   return;
 }
-// Distinct is "GROUP BY c1, c2" (where c1, c2 are a set of columns on
-// which the input is unique, i.e. contain a key) and has no aggregate
-// functions. It can be removed.
-final RelNode newInput = convert(input, 
aggregate.getTraitSet().simplify());
 
-// If aggregate was projecting a subset of columns, add a project for the
-// same effect.
+final RexBuilder rexBuilder = aggregate.getCluster().getRexBuilder();
+final List projects = new LinkedList<>();
+for (AggregateCall aggCall : aggregate.getAggCallList()) {
 
 Review comment:
   it's unusual to use LinkedList; since you're only appending, ArrayList would 
be the least surprising choice


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

julianhyde commented on a change in pull request #1078: [CALCITE-896] Remove 
Aggregate if grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#discussion_r261430446
 
 

 ##
 File path: 
core/src/main/java/org/apache/calcite/sql/SqlSplittableAggFunction.java
 ##
 @@ -173,11 +173,11 @@ public RexNode singleton(RexBuilder rexBuilder, 
RelDataType inputRowType,
   final RexNode predicate =
   RexUtil.composeConjunction(rexBuilder, predicates, true);
   if (predicate == null) {
-return rexBuilder.makeExactLiteral(BigDecimal.ONE);
+return rexBuilder.makeBigintLiteral(BigDecimal.ONE);
 
 Review comment:
   Let's not assume that COUNT is BIGINT. (In some type systems it might be 
INT.) Use AggregateCall.type?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on issue #1033: [CALCITE-2820] Add a version of AggregateReduceFunctionsRule that does not reduce SUM to SUM0

[GitBox]

siddharthteotia commented on issue #1033: [CALCITE-2820] Add a version of 
AggregateReduceFunctionsRule that does not reduce SUM to SUM0
URL: https://github.com/apache/calcite/pull/1033#issuecomment-468497873
 
 
   ping @vlsi , @julianhyde, @laurentgo 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] siddharthteotia commented on a change in pull request #1031: [CALCITE-2742]: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

siddharthteotia commented on a change in pull request #1031: [CALCITE-2742]: 
Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261386852
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
 ##
 @@ -175,6 +177,58 @@ private void checkConstant(final Object operand,
 });
   }
 
+  @Test public void testUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.USER,
+   DataContext.Variable.USER, "happyCalciteUser");
+  }
+
+  @Test public void testSystemUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.SYSTEM_USER,
+   DataContext.Variable.SYSTEM_USER, "");
+  }
+
+  @Test public void testTimestampFromContext() throws Exception {
+// current timestamp currently rounds to the nearest second (?)
+long val = System.currentTimeMillis() / 1000 * 1000;
+testContextLiteral(SqlStdOperatorTable.CURRENT_TIMESTAMP,
+   DataContext.Variable.CURRENT_TIMESTAMP, val);
+  }
 
 Review comment:
   How can we address this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on a change in pull request #1031: [CALCITE-2742]: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

[GitBox]

julianhyde commented on a change in pull request #1031: [CALCITE-2742]: Update 
RexImpTable to use DataContext to retrieve USER and SYSTEM_USER
URL: https://github.com/apache/calcite/pull/1031#discussion_r261393964
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
 ##
 @@ -175,6 +177,58 @@ private void checkConstant(final Object operand,
 });
   }
 
+  @Test public void testUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.USER,
+   DataContext.Variable.USER, "happyCalciteUser");
+  }
+
+  @Test public void testSystemUserFromContext() throws Exception {
+testContextLiteral(SqlStdOperatorTable.SYSTEM_USER,
+   DataContext.Variable.SYSTEM_USER, "");
+  }
+
+  @Test public void testTimestampFromContext() throws Exception {
+// current timestamp currently rounds to the nearest second (?)
+long val = System.currentTimeMillis() / 1000 * 1000;
+testContextLiteral(SqlStdOperatorTable.CURRENT_TIMESTAMP,
+   DataContext.Variable.CURRENT_TIMESTAMP, val);
+  }
 
 Review comment:
   Well, if the test starts at 10,000 and finishes at 12,000 then any timestamp 
value between 10,000 and 12,000 should be considered valid.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] julianhyde commented on issue #811: Add LGTM code quality badges

[GitBox]

julianhyde commented on issue #811: Add LGTM code quality badges
URL: https://github.com/apache/calcite/pull/811#issuecomment-468467926
 
 
   I'm -0 on this. Doesn't seem to be worth the effort.
   
   The PR submitter works for the company, so we would be free advertising for 
them. But our labor is not free.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] hsyuan opened a new pull request #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

hsyuan opened a new pull request #1078: [CALCITE-896] Remove Aggregate if 
grouping columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078
 
 
   JIRA: https://issues.apache.org/jira/browse/CALCITE-896


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[jira] [Created] (CALCITE-2883) HepPlanner subprogram may loop till getting out of memory

[Jesus Camacho Rodriguez (JIRA)]

Jesus Camacho Rodriguez created CALCITE-2883:


 Summary: HepPlanner subprogram may loop till getting out of memory
 Key: CALCITE-2883
 URL: https://issues.apache.org/jira/browse/CALCITE-2883
 Project: Calcite
  Issue Type: Bug
Reporter: Jesus Camacho Rodriguez
Assignee: Jesus Camacho Rodriguez


Consider the following two hep programs.

Program 1:
{code}
final HepProgramBuilder programBuilder = new HepProgramBuilder();
programBuilder.addMatchOrder(HepMatchOrder.BOTTOM_UP);
programBuilder.addRuleInstance(JoinToMultiJoinRule.INSTANCE);
programBuilder.addRuleInstance(LoptOptimizeJoinRule.INSTANCE);
final HepProgram program = programBuilder.build();
{code}

Program 2:
{code}
final HepProgramBuilder programBuilder = new HepProgramBuilder();
final HepProgramBuilder subprogramBuilder = new HepProgramBuilder();
subprogramBuilder.addMatchOrder(HepMatchOrder.BOTTOM_UP);
subprogramBuilder.addRuleInstance(JoinToMultiJoinRule.INSTANCE);
subprogramBuilder.addRuleInstance(LoptOptimizeJoinRule.INSTANCE);
programBuilder.addSubprogram(subprogramBuilder.build());
final HepProgram program = programBuilder.build();
{code}

I would expect both programs to behave similarly. However, program 2 will loop 
indefinitely. The reason is that {{HepPlanner}} subprogram execution loops if 
subprogram generates any new expression.
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java#L339
This does not seem right since planner can control exiting the program (and 
thus, subprogram) depending on its own internal state and configuration 
properties, e.g., match limit.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[GitHub] jcamachor opened a new pull request #1079: [CALCITE-2883] HepPlanner subprogram may loop till getting out of memory

[GitBox]

jcamachor opened a new pull request #1079: [CALCITE-2883] HepPlanner subprogram 
may loop till getting out of memory
URL: https://github.com/apache/calcite/pull/1079
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: [DISCUSS] Move gitbox notification emails to another list?

[Francis Chuang]

Infra has updated the Gitbox settings for all Calcite repositories. The 
notifications will now be sent to the commits@calcite.a.o list.


On 27/02/2019 9:13 pm, Francis Chuang wrote:

Hey all,

I wanted to gauge your opinions regarding the gitbox emails being sent 
to the dev@ list. I am finding these emails to be quite noisy (there is 
an email every time there is activity in our Github repos). I end up 
deleting most of these emails without reading them and I feel that there 
is a lot of noise compared to signal.


How do you guys feel about moving these to another list (for example 
git...@calcite.apache.org)? As Vladimir mentioned in another thread, 
these emails are quite useful and important as they serve as a 
searchable archive of activity in our Github repos.


If we do move the emails to a different list, what do we do with the 
emails that have been sent to dev@? Do we nominate someone to forward a 
copy to the new list?


Francis

[GitHub] risdenk commented on issue #998: Suggestion: Improvement of Autoboxing and Unboxing

[GitBox]

risdenk commented on issue #998: Suggestion: Improvement of Autoboxing and 
Unboxing
URL: https://github.com/apache/calcite/pull/998#issuecomment-468459175
 
 
   Closing since there is no JIRA attached to this PR and only addresses a 
single line. As @vlsi pointed out, there isn't a big reason to do this and 
would have to be across the board.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk closed pull request #998: Suggestion: Improvement of Autoboxing and Unboxing

[GitBox]

risdenk closed pull request #998: Suggestion: Improvement of Autoboxing and 
Unboxing
URL: https://github.com/apache/calcite/pull/998
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: JIRAs and Pull Requests Cleanup

[Kevin Risden]

As of now, the open PRs and Calcite JIRAs are close enough to matching (96
vs 98). (There are a few for Avatica in the JIRA query). Thanks all those
that helped clean up.

Kevin Risden


On Thu, Feb 28, 2019 at 4:21 PM Kevin Risden  wrote:

> One thing that I think would help is to add a Github PR template [1]. We
> did this for Apache Knox to help make sure that PRs follow some simple
> guidelines. After the gitbox migration, PRs are automatically linked to
> JIRA (which is good). I think we just had some PRs prior to gitbox
> migration that didn't have the autolink part.
>
> [1]
> https://help.github.com/en/articles/creating-a-pull-request-template-for-your-repository
> [2] https://issues.apache.org/jira/browse/KNOX-1760
>
> Kevin Risden
>
>
> On Thu, Feb 28, 2019 at 4:07 PM Vladimir Sitnikov <
> sitnikov.vladi...@gmail.com> wrote:
>
>> Francis> Our problem is mainly due to PRs not being reviewed.
>>
>> Once upon a time I did try to pass over the PRs, and I added some randoms
>> here and there.
>> I'm not sure what others think of that, however we have:
>>
>> 17 "returned-with-feedback":
>>
>> https://github.com/apache/calcite/pulls?q=is%3Apr+is%3Aopen+label%3Areturned-with-feedback
>> 6 with pending discussion in JIRA:
>>
>> https://github.com/apache/calcite/pulls?utf8=%E2%9C%93=is%3Apr+is%3Aopen+label%3Adiscussion-in-jira
>>
>> I like "LGTM-will-merge-soon" as a soft warning to other committers that
>> "hey, either you chime in or I just commit this stuff".
>> We have 2 by the way:
>> https://github.com/apache/calcite/labels/LGTM-will-merge-soon
>>
>> Vladimir
>>
>

[GitHub] julianhyde commented on issue #784: [CALCITE-2456] fix VolcanoRuleCall#match for unordered child operand

[GitBox]

julianhyde commented on issue #784: [CALCITE-2456] fix VolcanoRuleCall#match 
for unordered child operand
URL: https://github.com/apache/calcite/pull/784#issuecomment-468468357
 
 
   +1 but please change the commit message; remove the word 'fix' and describe 
the actual problem being fixed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] hsyuan commented on issue #1078: [CALCITE-896] Remove Aggregate if grouping columns are unique and all functions are splittable

[GitBox]

hsyuan commented on issue #1078: [CALCITE-896] Remove Aggregate if grouping 
columns are unique and all functions are splittable
URL: https://github.com/apache/calcite/pull/1078#issuecomment-468496263
 
 
   @julianhyde I have addressed your comments, thanks for the review. Can you 
take another look?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[jira] [Created] (CALCITE-2882) ConnectionProperties lose effectiveness when connection reopen after expired

[shining (JIRA)]

shining created CALCITE-2882:


 Summary: ConnectionProperties lose effectiveness when connection 
reopen after expired
 Key: CALCITE-2882
 URL: https://issues.apache.org/jira/browse/CALCITE-2882
 Project: Calcite
  Issue Type: Bug
  Components: avatica
Affects Versions: 1.12.0
 Environment: Phoenix 5.1
avatca 1.12
Reporter: shining
 Attachments: image-2019-02-28-17-25-39-478.png, 
image-2019-02-28-17-28-31-926.png

When use avatica connect Phoenix QueryServer, I create an AvaticaConnection:

{code:java}
Connection conntion = DriverManage.getConnection(url);
connection.setAutoCommit(true);
{code}

Avatica keep PhoenixConnection alive in the Cache, which will be expired after 
10min by default.
I still use the older AvaticaConnection , it will reopen an PhoenixConnection, 
but the ConnectionProperties is loss, such as AutoCommit.

I use sqlline-thin.py to reappear the problem:
1) sqlline-thin.py http://localhost:8765
2) upsert one row and select
 !image-2019-02-28-17-25-39-478.png! 
3) after 10 min, upsert again, the connection will be recreate, but select null
 !image-2019-02-28-17-28-31-926.png! 




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[GitHub] ritesh-kapoor opened a new pull request #1075: CALCITE-2881: Adding JSON_PRETTY function support

[GitBox]

ritesh-kapoor opened a new pull request #1075: CALCITE-2881: Adding JSON_PRETTY 
function support
URL: https://github.com/apache/calcite/pull/1075
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services