Jackie-Jiang commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r969980710


##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -690,24 +690,37 @@ public void updateBooleanFieldsIfNeeded(Schema oldSchema) 
{
 
   /**
    * Check whether the current schema is backward compatible with oldSchema.
-   * Backward compatibility requires all columns and fieldSpec in oldSchema 
should be retained.
+   * Always return true if force to set to true.
+   *
+   * Backward compatibility requires
+   * (1) all columns in oldSchema should be retained.
+   * (2) all column fieldSpecs should be backward compatible with the old ones.
    *
    * @param oldSchema old schema
+   * @param force whether to force overriding the old schema
    */
-  public boolean isBackwardCompatibleWith(Schema oldSchema) {
+  public boolean isBackwardCompatibleWith(Schema oldSchema, boolean force) {

Review Comment:
   We should not put `force` into this method. The `force` should be handled on 
the caller side based on the return result of this method.



##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java:
##########
@@ -536,4 +536,22 @@ public int compareTo(FieldSpec otherSpec) {
     // Sort fieldspecs based on their name
     return _name.compareTo(otherSpec._name);
   }
+
+  /***
+   * Return true if it is backward compatible with the old FieldSpec.
+   * Backward compatibility requires
+   * all other fields except DefaultNullValue and Max Length should be 
retained.
+   *
+   * @param oldFieldSpec
+   * @return
+   */
+  public boolean isBackwardCompatibleWith(FieldSpec oldFieldSpec) {
+
+    return EqualityUtils.isEqual(_name, oldFieldSpec._name)
+            && EqualityUtils.isEqual(_dataType, oldFieldSpec._dataType)
+            && EqualityUtils.isEqual(_isSingleValueField, 
oldFieldSpec._isSingleValueField)
+            && EqualityUtils.isEqual(_transformFunction, 
oldFieldSpec._transformFunction)
+            && EqualityUtils.
+            isEqual(_virtualColumnProvider, 
oldFieldSpec._virtualColumnProvider);

Review Comment:
   We can also remove these 2 checks



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