Jackie-Jiang commented on code in PR #8503:
URL: https://github.com/apache/pinot/pull/8503#discussion_r853507748
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -242,6 +243,7 @@ public enum ColumnDataType {
LONG,
FLOAT,
DOUBLE,
+ BIG_DECIMAL /* Stored as BYTES */,
Review Comment:
Since we added dictionary for `BIG_DECIMAL`, it is no longer just a logical
type, and we should have it as a stored type
##########
pinot-core/src/main/java/org/apache/pinot/core/common/RowBasedBlockValueFetcher.java:
##########
@@ -63,12 +65,16 @@ private ValueFetcher createFetcher(BlockValSet blockValSet)
{
case STRING:
return new StringSingleValueFetcher(blockValSet.getStringValuesSV());
case BYTES:
+ if (dataType == DataType.BIG_DECIMAL) {
+ return new BigDecimalValueFetcher(blockValSet.getBytesValuesSV());
Review Comment:
Since we added dictionary for big-decimal, we should add the big-decimal to
the data access layer (add `BlockValSet.getBigDecimalValuesSV()`)
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -29,6 +29,13 @@
*/
public interface BlockValSet {
+ /**
+ * Returns the number of entries for a single-valued column.
+ *
+ * @return number of single-valued entries
+ */
+ int getNumDocs();
Review Comment:
This method shouldn't be needed for this PR. I find the only usage in
`IdentifierTransformFunction`, which can be replaced with
`ProjectionBlock.getNumDocs()`
##########
pinot-core/src/main/java/org/apache/pinot/core/common/DataBlockCache.java:
##########
@@ -255,9 +255,10 @@ public String[] getStringValuesForSVColumn(String column) {
* @param column Column name
* @param evaluator transform evaluator
* @param buffer values to fill
+ * @param parseExactBigDecimal parse exact BigDecimal values
*/
- public void fillValues(String column, TransformEvaluator evaluator, String[]
buffer) {
- _dataFetcher.fetchStringValues(column, evaluator, _docIds, _length,
buffer);
+ public void fillValues(String column, TransformEvaluator evaluator, String[]
buffer, boolean parseExactBigDecimal) {
Review Comment:
I think we should add a separate method `public void fillValues(String
column, TransformEvaluator evaluator, BigDecimal[] buffer)` instead of reusing
this one
--
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
