[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5618: [TE] Upgrade Pinot version to 0.4.0

[GitBox <gi...@apache.org>]

akshayrai commented on a change in pull request #5618:
URL: https://github.com/apache/incubator-pinot/pull/5618#discussion_r445898403



##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/auto/onboard/AutoOnboardPinotMetadataSource.java
##########
@@ -200,23 +201,26 @@ private void addNewDataset(String dataset, Schema schema, Map<String, String> cu
   /**
    * Refreshes an existing dataset in the thirdeye database
    * with any dimension/metric changes from pinot schema
-   * @param dataset
-   * @param schema
-   * @param datasetConfig
    */
-  private void refreshOldDataset(String dataset, Schema schema, Map<String, String> customConfigs,
-      DatasetConfigDTO datasetConfig) throws Exception {
-    checkDimensionChanges(dataset, datasetConfig, schema);
+  private void refreshOldDataset(String dataset, Schema schema, String timeColumnName,
+      Map<String, String> customConfigs, DatasetConfigDTO datasetConfig) {
+    checkDimensionChanges(dataset, datasetConfig, schema, timeColumnName);
     checkMetricChanges(dataset, datasetConfig, schema);
-    checkTimeFieldChanges(datasetConfig, schema);
+    checkTimeFieldChanges(datasetConfig, schema, timeColumnName);
     appendNewCustomConfigs(datasetConfig, customConfigs);
     checkNonAdditive(datasetConfig);
     datasetConfig.setActive(true);
   }
 
-  private void checkDimensionChanges(String dataset, DatasetConfigDTO datasetConfig, Schema schema) {
+  private void checkDimensionChanges(String dataset, DatasetConfigDTO datasetConfig, Schema schema,
+      String timeColumnName) {
     LOG.info("Checking for dimensions changes in {}", dataset);
-    List<String> schemaDimensions = schema.getDimensionNames();
+    List<String> schemaDimensions = new ArrayList<>(schema.getDimensionNames());
+     for (String dateTimeColumn : schema.getDateTimeNames()) { // treat all dateTimeFields specs as dimensions, except the primary time column

Review comment:
       Can you shed some light into why we want to treat dateTimeFields as a dimension? what's the difference?




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

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



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