[GitHub] [calcite] julianhyde commented on a diff in pull request #3245: [CALCITE-5747] Conflicting FLOOR return type between Calcite and BigQ…

2023-06-21 Thread via GitHub 


julianhyde commented on code in PR #3245:
URL: https://github.com/apache/calcite/pull/3245#discussion_r1237732224


##
site/_docs/reference.md:
##
@@ -2661,6 +2661,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | s | ARRAY_SIZE(array)  | Synonym for 
`CARDINALITY`
 | * | ASINH(numeric) | Returns the inverse 
hyperbolic sine of *numeric*
 | * | ATANH(numeric) | Returns the inverse 
hyperbolic tangent of *numeric*
+| b | CEIL(value)   | Similar to standard 
`CEIL(value)` except if *value* is an integer type, the return type is a double

Review Comment:
   add space before `|`



##
core/src/main/codegen/templates/Parser.jj:
##
@@ -7335,9 +7336,13 @@ SqlNode StandardFloorCeilOptions(Span s, boolean 
floorFlag) :
 }
 )?
  {
-SqlOperator op = floorFlag
-? SqlStdOperatorTable.FLOOR
-: SqlStdOperatorTable.CEIL;
+SqlOperator op;
+boolean isBigQuery = this.conformance.semantics() == 
SqlLibrary.BIG_QUERY;

Review Comment:
   can you create a function in SqlStdOperatorTable similar to 
`SqlStdOperatorTable.like(boolean,boolean)`? Its args should be boolean and 
conformance.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [calcite] julianhyde commented on a diff in pull request #3245: [CALCITE-5747] Conflicting FLOOR return type between Calcite and BigQ…

2023-06-09 Thread via GitHub 


julianhyde commented on code in PR #3245:
URL: https://github.com/apache/calcite/pull/3245#discussion_r1224881785


##
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java:
##
@@ -1853,12 +1853,14 @@ public class SqlStdOperatorTable extends 
ReflectiveSqlOperatorTable {
   /**
* The FLOOR function.
*/
-  public static final SqlFunction FLOOR = new SqlFloorFunction(SqlKind.FLOOR);
+  public static final SqlFunction FLOOR =
+  new SqlFloorFunction(SqlKind.FLOOR, ReturnTypes.ARG0_OR_EXACT_NO_SCALE);

Review Comment:
   I would keep the same constructor, and add methods `withName`, 
`withReturnTypeInference`. (We have discovered that for functions/operators, 
subclassing doesn't scale, and adding constructor parameters doesn't scale.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [calcite] julianhyde commented on a diff in pull request #3245: [CALCITE-5747] Conflicting FLOOR return type between Calcite and BigQ…

2023-06-09 Thread via GitHub 


julianhyde commented on code in PR #3245:
URL: https://github.com/apache/calcite/pull/3245#discussion_r1224877404


##
core/src/main/java/org/apache/calcite/sql/fun/SqlFloorFunction.java:
##
@@ -41,22 +45,23 @@
 public class SqlFloorFunction extends SqlMonotonicUnaryFunction {
   //~ Constructors ---
 
-  public SqlFloorFunction(SqlKind kind) {
-super(kind.name(), kind, ReturnTypes.ARG0_OR_EXACT_NO_SCALE, null,
+  public SqlFloorFunction(SqlKind kind, SqlReturnTypeInference 
returnTypeInference) {
+super(kind.name(), kind, returnTypeInference, null,
 OperandTypes.NUMERIC_OR_INTERVAL.or(
 OperandTypes.sequence("'" + kind + "( TO )'\n"
 + "'" + kind + "( TO )'\n"
 + "'" + kind + "( TO )'",
 OperandTypes.DATETIME,
 OperandTypes.ANY)),
 SqlFunctionCategory.NUMERIC);
-Preconditions.checkArgument(kind == SqlKind.FLOOR || kind == SqlKind.CEIL);
+Preconditions.checkArgument(kind == SqlKind.FLOOR || kind == SqlKind.CEIL
+|| kind == SqlKind.FLOOR_BIG_QUERY || kind == SqlKind.CEIL_BIG_QUERY);
   }
 
   //~ Methods 
 
   @Override public SqlMonotonicity getMonotonicity(SqlOperatorBinding call) {
-// Monotonic iff its first argument is, but not strict.
+// Monotonic if its first argument is, but not strict.

Review Comment:
   yes it was intentional.
   
   When podcasters say "If you liked this episode, leave a review on Apple 
Music" I'm always jumping up and down, "You mean if and only if!"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org