You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/12 16:32:33 UTC
[GitHub] [pinot] MeihanLi opened a new pull request, #9382: [Bugfix] schema update bug fix
MeihanLi opened a new pull request, #9382:
URL: https://github.com/apache/pinot/pull/9382
### Description
This Pr looses schema backward compatibility checking, allowing max length and default null values to be changed. And it added a new API param `force` for POST /addSchema API with default value `false`. Once it is set to `true`, we will force overriding the existing schema.
This Pr will changed two API, POST /addSchema and POST /updateSchema since both need to pass the backward compatibility checking.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968921814
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
Review Comment:
Thanks done.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on PR #9382:
URL: https://github.com/apache/pinot/pull/9382#issuecomment-1248649119
> Please take a look at the failed tests
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] Jackie-Jiang merged pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #9382:
URL: https://github.com/apache/pinot/pull/9382
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on PR #9382:
URL: https://github.com/apache/pinot/pull/9382#issuecomment-1244002169
cc: @Jackie-Jiang @deemoliu @chenboat
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968921521
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
Review Comment:
Thanks, done
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r969992785
##########
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:
done, please check the second commit
##########
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:
Thanks, can you check the second commit?
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r969992785
##########
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:
done, please check the third commit
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968866490
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1303,14 +1303,20 @@ public void updateSchema(Schema schema, boolean reload)
* Helper method to update the schema, or throw SchemaBackwardIncompatibleException when the new schema is not
* backward-compatible with the existing schema.
*/
- private void updateSchema(Schema schema, Schema oldSchema)
+ private void updateSchema(Schema schema, Schema oldSchema, boolean force)
throws SchemaBackwardIncompatibleException {
String schemaName = schema.getSchemaName();
schema.updateBooleanFieldsIfNeeded(oldSchema);
if (schema.equals(oldSchema)) {
LOGGER.info("New schema: {} is the same as the existing schema, not updating it", schemaName);
return;
}
+ if (force) {
Review Comment:
Thanks Jackie for the comment, I reorganized the codes accordingly.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] Jackie-Jiang commented on pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #9382:
URL: https://github.com/apache/pinot/pull/9382#issuecomment-1247336806
Please take a look at the failed tests
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968867393
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
+ return false;
+ }
+
+ boolean isBackward = EqualityUtils.isEqual(fieldSpec.getName(), oldSchemaFieldSpec.getName())
Review Comment:
Thanks @Jackie-Jiang , this is a good point, I addressed it in the second commit.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968867393
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
+ return false;
+ }
+
+ boolean isBackward = EqualityUtils.isEqual(fieldSpec.getName(), oldSchemaFieldSpec.getName())
Review Comment:
Thanks, this is a good point, I addressed it in the second comment.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
+ return false;
+ }
+
+ boolean isBackward = EqualityUtils.isEqual(fieldSpec.getName(), oldSchemaFieldSpec.getName())
Review Comment:
Thanks, this is a good point, I addressed it in the second commit.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r968736701
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
Review Comment:
This check is redundant. We already checked in the previous part and both of them are not `null`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1303,14 +1303,20 @@ public void updateSchema(Schema schema, boolean reload)
* Helper method to update the schema, or throw SchemaBackwardIncompatibleException when the new schema is not
* backward-compatible with the existing schema.
*/
- private void updateSchema(Schema schema, Schema oldSchema)
+ private void updateSchema(Schema schema, Schema oldSchema, boolean force)
throws SchemaBackwardIncompatibleException {
String schemaName = schema.getSchemaName();
schema.updateBooleanFieldsIfNeeded(oldSchema);
if (schema.equals(oldSchema)) {
LOGGER.info("New schema: {} is the same as the existing schema, not updating it", schemaName);
return;
}
+ if (force) {
Review Comment:
Move this check into the backward compatible check and log a warning when the schema is not backward compatible and force flag is on.
Don't log warning as info because it can be confusing. We may log a warning such as `Force updated schema: {} which is backward incompatible with the existing schema`
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
+ continue;
+ }
+
+ if ((fieldSpec == null && oldSchemaFieldSpec != null) || (fieldSpec != null && oldSchemaFieldSpec == null)) {
+ return false;
+ }
+
+ boolean isBackward = EqualityUtils.isEqual(fieldSpec.getName(), oldSchemaFieldSpec.getName())
Review Comment:
Let's add a method in `FieldSpec`: `boolean isBackwardCompatibleWith(FieldSpec oldFieldSpec)` so that other sub-classes can override it if necessary
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/Schema.java:
##########
@@ -703,7 +705,23 @@ public boolean isBackwardCompatibleWith(Schema oldSchema) {
}
FieldSpec oldSchemaFieldSpec = entry.getValue();
FieldSpec fieldSpec = getFieldSpecFor(oldSchemaColumnName);
- if (!fieldSpec.equals(oldSchemaFieldSpec)) {
+
+ if (EqualityUtils.isSameReference(fieldSpec, oldSchemaFieldSpec)) {
Review Comment:
This check is redundant. They will never share the same reference
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] codecov-commenter commented on pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9382:
URL: https://github.com/apache/pinot/pull/9382#issuecomment-1244831067
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9382?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9382](https://codecov.io/gh/apache/pinot/pull/9382?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8e15d40) into [master](https://codecov.io/gh/apache/pinot/commit/c8d108506d098b6de8017b4226b4a8911ea0d775?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c8d1085) will **decrease** coverage by `43.62%`.
> The diff coverage is `14.28%`.
```diff
@@ Coverage Diff @@
## master #9382 +/- ##
=============================================
- Coverage 69.74% 26.11% -43.63%
+ Complexity 4710 44 -4666
=============================================
Files 1885 1873 -12
Lines 100377 100063 -314
Branches 15275 15245 -30
=============================================
- Hits 70008 26136 -43872
- Misses 25413 71297 +45884
+ Partials 4956 2630 -2326
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `26.11% <14.28%> (+0.12%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9382?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ler/api/resources/TableConfigsRestletResource.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlQ29uZmlnc1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (-77.42%)` | :arrow_down: |
| [...main/java/org/apache/pinot/spi/data/FieldSpec.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-82.04%)` | :arrow_down: |
| [...rc/main/java/org/apache/pinot/spi/data/Schema.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9TY2hlbWEuamF2YQ==) | `0.00% <0.00%> (-75.08%)` | :arrow_down: |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `40.35% <33.33%> (-31.35%)` | :arrow_down: |
| [...ller/api/resources/PinotSchemaRestletResource.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2NoZW1hUmVzdGxldFJlc291cmNlLmphdmE=) | `17.72% <66.66%> (-51.90%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1373 more](https://codecov.io/gh/apache/pinot/pull/9382/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on a diff in pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on code in PR #9382:
URL: https://github.com/apache/pinot/pull/9382#discussion_r969993167
##########
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:
Thanks, can you check the third commit?
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi commented on pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi commented on PR #9382:
URL: https://github.com/apache/pinot/pull/9382#issuecomment-1248650236
Thanks @Jackie-Jiang for the approval. I fixed the unit tests and please check again.
--
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-unsubscribe@pinot.apache.org
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
[GitHub] [pinot] MeihanLi closed pull request #9382: [Bugfix] schema update bug fix
Posted by GitBox <gi...@apache.org>.
MeihanLi closed pull request #9382: [Bugfix] schema update bug fix
URL: https://github.com/apache/pinot/pull/9382
--
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-unsubscribe@pinot.apache.org
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