snleee commented on PR #8462:
URL: https://github.com/apache/pinot/pull/8462#issuecomment-1087990840

   I have one general comment before reviewing the code.
   
   Currently, our old and new configs look like the following:
   ```
   Current config:
   
   
     "tableIndexConfig": {
       "aggregateMetrics": "true",
       ...
     }
   --------------------------------
   
   New config:
   
   "aggregationConfigs": [
     {
       "columnName": "destColumn",
       "aggregationFunction": "MIN(srcColumn)"
     }
   ]
   
   ```
   
   
   The old config can be translated into the new config with the same behavior 
like the following:
   ```
   e.g.
   
   Schema:
   
   dimCol1 | metricCol1 | metriccol2 
   
   old config:
   
     "tableIndexConfig": {
       "aggregateMetrics": "true",
       ...
     }
   
   
   new config:
   
   "aggregationConfigs": [
     {
       "columnName": "metricCol1",
       "aggregationFunction": "SUM(metricCol1)"
     },
     {
       "columnName": "metricCol2",
       "aggregationFunction": "SUM(metricCol2)"
     }
   ]
   ```
   
   
   So, we can actually do the following:
   1. Put deprecated annotation for the old config in the table config
   2. Keep backward compatibility by having a config conversion layer from the 
old config to the new config format
   3. From the code, we can always use the new config format.
   
   It's fine that thie above can be done in multiple steps but I think that we 
should not keep the 2 different configs for aggregateMetrics feature.


-- 
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