github-actions[bot] commented on code in PR #63182:
URL: https://github.com/apache/doris/pull/63182#discussion_r3253088584
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -3213,9 +3214,93 @@ public void validateForFlexiblePartialUpdate() throws
UserException {
throw new UserException("Flexible partial update can only support
table with light_schema_change enabled."
+ " But table " + getName() + "'s property
light_schema_change is false");
}
- if (hasVariantColumns()) {
- throw new UserException("Flexible partial update can only support
table without variant columns.");
+ validateVariantColumnsForFlexiblePartialUpdate();
+ }
+
+ public void validateVariantColumnsForFlexiblePartialUpdate() throws
UserException {
+ validateVariantColumnsForFlexiblePartialUpdate(getBaseSchema(),
variantEnableFlattenNested());
+ }
+
+ public static void
validateVariantColumnsForFlexiblePartialUpdate(List<Column> columns) throws
UserException {
+ validateVariantColumnsForFlexiblePartialUpdate(columns, false);
+ }
+
+ public static void validateVariantColumnsForFlexiblePartialUpdate(
+ List<Column> columns, boolean deprecatedVariantFlattenNested)
throws UserException {
+ boolean hasVariantColumn = false;
+ for (Column column : columns) {
+ validateVariantColumnForFlexiblePartialUpdate(column);
+ if (column.getType().isVariantType() &&
deprecatedVariantFlattenNested) {
+ throw new UserException(
+ "VARIANT flexible partial update does not support "
+ + "deprecated_variant_enable_flatten_nested in
this version");
+ }
+ hasVariantColumn |= column.getType().isVariantType();
+ }
+ if (hasVariantColumn) {
+ try {
+ validateBackendsSupportVariantFlexiblePartialUpdate(
+
Env.getCurrentSystemInfo().getAllBackendsByAllCluster().values());
+ } catch (AnalysisException e) {
+ throw new UserException(e.getMessage(), e);
+ }
+ }
+ }
+
+ public static void validateVariantColumnForFlexiblePartialUpdate(Column
column) throws UserException {
+ if (column.getType().isVariantType() &&
column.getVariantEnableDocMode()) {
+ throw new UserException(
+ "VARIANT flexible partial update does not support doc mode
in this version");
+ }
+ }
+
+ @VisibleForTesting
+ static void
validateBackendsSupportVariantFlexiblePartialUpdate(Collection<Backend>
backends)
+ throws UserException {
+ for (Backend backend : backends) {
+ if (!backend.isAlive()) {
+ continue;
+ }
+ if
(backendVersionSupportsVariantFlexiblePartialUpdate(backend.getVersion())) {
+ continue;
+ }
+ throw new UserException("VARIANT flexible partial update requires
all alive backends to "
+ + "support variant patch skip-bitmap markers. Backend " +
backend.getId() + " ("
+ + backend.getHost() + ") is running version " +
backend.getVersion()
+ + ", expected at least " + Version.DORIS_BUILD_VERSION);
+ }
+ }
+
+ @VisibleForTesting
+ static boolean backendVersionSupportsVariantFlexiblePartialUpdate(String
backendVersion) {
+ if (Strings.isNullOrEmpty(backendVersion)) {
+ return false;
+ }
+ if (backendVersion.equals(Version.DORIS_BUILD_VERSION)
+ || backendVersion.startsWith(Version.DORIS_BUILD_VERSION +
"-")) {
Review Comment:
This does not actually prove that every backend understands the new VARIANT
skip-bitmap marker format. It compares only the semantic build version, so an
old BE binary from the same release branch/build version as the FE still passes
this check during rolling upgrade even though its
`FlexibleReadPlan::prepare_to_read()` will treat the high-bit marker values as
ordinary column unique ids. The `!backend.isAlive()` skip above has a similar
failure path: a BE that is down during validation can later restart with old
code and publish/compact/read rowsets containing these markers. This is a
follow-up to the existing marker-compatibility thread, but distinct because
this newly added gate is the concrete implementation that still lets
incompatible BEs through. Please gate on an explicit BE capability/build
hash/protocol feature advertised by every backend that can participate in the
table, or otherwise keep VARIANT flexible partial update disabled until old BEs
cannot process the rowsets.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
