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

2019-02-28 Thread 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 a change in pull request #1031: CALCITE-2742: Update RexImpTable to use DataContext to retrieve USER and SYSTEM_USER

2019-02-27 Thread 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_r260953414
 
 

 ##
 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:
   coding style is to indent arguments 4 spaces, not to line up with '('.


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

2019-02-27 Thread 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_r260952927
 
 

 ##
 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:
   why is this non-deterministic?


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

2019-02-27 Thread 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_r260952365
 
 

 ##
 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:
   "Ensures" not "Ensure".


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

2019-02-27 Thread 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_r260952125
 
 

 ##
 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:
   I think this test will fail intermittently. Rounding to seconds will make it 
less frequent but doesn't eliminate the problem.


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

2019-02-27 Thread 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_r260949061
 
 

 ##
 File path: core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java
 ##
 @@ -175,6 +176,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.
+   * @param operator The Operator to check
+   * @param variable The DataContext variable this operator should be bound to.
+   * @param value The expected value to retrieve.
+   */
+  private void testContextLiteral(
+  final SqlOperator operator,
+  final DataContext.Variable variable,
+  final Object value) {
+Frameworks.withPrepare(new Frameworks.PrepareAction() {
+  public Void apply(final RelOptCluster cluster,
+final RelOptSchema relOptSchema,
+final SchemaPlus rootSchema,
+final CalciteServerStatement statement) {
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final RexExecutorImpl executor = new RexExecutorImpl(
+new SingleValueDataContext(variable.camelName, value));
+try {
+  checkConstant(value, new Function() {
+public RexNode apply(RexBuilder builder) {
+  List output = new ArrayList<>();
+  executor.reduce(rexBuilder,
+  ImmutableList.of(rexBuilder.makeCall(operator)), 
output);
+  return output.get(0);
+}
+  });
+} catch (Exception e) {
+  throw new RuntimeException(e);
 
 Review comment:
   @vlsi Is it possible to add a forbidden-apis check to flag these violations?


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