(pinot) branch master updated: Fix chained-transform schema validation error to list conflicting columns (#18512)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new b00b97a2680 Fix chained-transform schema validation error to list conflicting columns (#18512)
b00b97a2680 is described below

commit b00b97a26809ada5db7544275c369192f8a13906
Author: Deepak kumar <de...@meta.com>
AuthorDate: Tue Aug 11 17:09:28 2026 -0700

    Fix chained-transform schema validation error to list conflicting columns (#18512)
---
 .../apache/pinot/core/util/SchemaUtilsTest.java    | 23 ++++++++++++++++++++++
 .../pinot/segment/local/utils/SchemaUtils.java     | 10 +++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java
index 4b6db6ee639..336f984ae5d 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java
@@ -289,6 +289,29 @@ public class SchemaUtilsTest {
     checkValidationFails(pinotSchema);
   }
 
+  /// Regression test: when a transformed column is reused as an argument to another transform, the validation error
+  /// must list the actual conflicting columns. Historically this message reported a boolean ("true"/"false") because
+  /// {@link java.util.Set#retainAll} was passed as the format argument.
+  @Test
+  public void testChainedTransformErrorMessageListsConflictingColumns() {
+    Schema pinotSchema =
+        new Schema.SchemaBuilder().addSingleValueDimension("x", DataType.INT).addSingleValueDimension("z", DataType.INT)
+            .build();
+    pinotSchema.getFieldSpecFor("x").setTransformFunction("Groovy({y + 10}, y)");
+    pinotSchema.getFieldSpecFor("z").setTransformFunction("Groovy({x*w*20}, x, w)");
+
+    try {
+      SchemaUtils.validate(pinotSchema);
+      Assert.fail("Schema validation should have failed for chained transforms.");
+    } catch (IllegalStateException e) {
+      String message = e.getMessage();
+      Assert.assertNotNull(message);
+      // The chained column set has a single deterministic element here, so the formatted set form is "[x]".
+      Assert.assertEquals(message,
+          "Columns: [x] are a result of transformations, and cannot be used as arguments to other transform functions");
+    }
+  }
+
   @Test
   public void testValidateTimeFieldSpec() {
     Schema pinotSchema;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java
index 1481a5b26d8..dafd3dad4c8 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java
@@ -19,7 +19,6 @@
 package org.apache.pinot.segment.local.utils;
 
 import com.google.common.base.Preconditions;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -201,9 +200,14 @@ public class SchemaUtils {
         validateMultiValueCompatibility(fieldSpec);
       }
     }
-    Preconditions.checkState(Collections.disjoint(transformedColumns, argumentColumns),
+    // Compute the intersection in a fresh set so the error message lists the actual conflicting columns and we do not
+    // mutate transformedColumns. Previously, Set#retainAll's boolean return value was passed as the format argument,
+    // producing a useless "Columns: true ..." message.
+    Set<String> chainedTransformColumns = new HashSet<>(transformedColumns);
+    chainedTransformColumns.retainAll(argumentColumns);
+    Preconditions.checkState(chainedTransformColumns.isEmpty(),
         "Columns: %s are a result of transformations, and cannot be used as arguments to other transform functions",
-        transformedColumns.retainAll(argumentColumns));
+        chainedTransformColumns);
     if (schema.getPrimaryKeyColumns() != null) {
       for (String primaryKeyColumn : schema.getPrimaryKeyColumns()) {
         Preconditions.checkState(primaryKeyColumnCandidates.contains(primaryKeyColumn),


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