yashmayya commented on code in PR #15263:
URL: https://github.com/apache/pinot/pull/15263#discussion_r2016113370


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java:
##########
@@ -62,16 +62,36 @@ public boolean shouldConvertRaggedUnionTypesToVarying() {
     return true;
   }
 
+  @Override
+  public int getMaxScale(SqlTypeName typeName) {
+    return typeName == SqlTypeName.DECIMAL ? MAX_DECIMAL_SCALE : 
super.getMaxScale(typeName);
+  }
+
   @Override
   public int getMaxNumericScale() {
     return MAX_DECIMAL_SCALE;
   }
 
+  @Override
+  public int getDefaultScale(SqlTypeName typeName) {
+    return typeName == SqlTypeName.DECIMAL ? MAX_DECIMAL_SCALE : 
super.getDefaultScale(typeName);
+  }
+
+  @Override
+  public int getMaxPrecision(SqlTypeName typeName) {
+    return typeName == SqlTypeName.DECIMAL ? MAX_DECIMAL_PRECISION : 
super.getMaxPrecision(typeName);
+  }
+
   @Override
   public int getMaxNumericPrecision() {
     return MAX_DECIMAL_PRECISION;
   }
 
+  @Override
+  public int getDefaultPrecision(SqlTypeName typeName) {
+    return typeName == SqlTypeName.DECIMAL ? MAX_DECIMAL_PRECISION : 
super.getDefaultPrecision(typeName);

Review Comment:
   Yeah, that makes sense. I also discussed with @Jackie-Jiang offline and in 
his opinion, the original intention of 1000, 1000 was actually probably to 
actually have 2000 precision and 1000 scale. I've updated this but it does mean 
that Calcite is now forcing some literals (direct cast to `DECIMAL` without 
explicit precision / scale or comparison between `BIG_DECIMAL` column and 
literals) to have 1000 scale which like you've pointed out might not be very 
efficient for Java's BigDecimal that is used internally by both Calcite and 
Pinot to represent SQL `DECIMAL` types.



-- 
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...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to