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

fhueske pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/flink.git


The following commit(s) were added to refs/heads/master by this push:
     new aef4bb322d0 [FLINK-40219][table] Fix outputType computation of 
LateralSnapshotJoin (#28803)
aef4bb322d0 is described below

commit aef4bb322d01863432eeb986db0e188ad32ab0fd
Author: Fabian Hueske <[email protected]>
AuthorDate: Wed Jul 22 20:13:38 2026 +0200

    [FLINK-40219][table] Fix outputType computation of LateralSnapshotJoin 
(#28803)
    
    TimeIndicatorRelDataType.getOriginalType() does not reliably provide the 
correct nullability.
    The planner might have changed the nullability on the time indicator type 
without updating the nested original type.
    This change fixes the outputType computation to use the same rowtime type 
conversion as RelTimeIndicatorConverter.
    
    Generated-By: Claude Opus 4.8 (1M context)
---
 .../plan/utils/LateralSnapshotJoinUtil.java        |  21 ++-
 .../plan/utils/LateralSnapshotJoinUtilTest.java    | 188 +++++++++++++++++++++
 2 files changed, 205 insertions(+), 4 deletions(-)

diff --git 
a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtil.java
 
b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtil.java
index 95bca1a1384..aa2f8a22cc9 100644
--- 
a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtil.java
+++ 
b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtil.java
@@ -22,8 +22,12 @@ import org.apache.flink.annotation.Internal;
 import org.apache.flink.table.functions.BuiltInFunctionDefinition;
 import org.apache.flink.table.functions.BuiltInFunctionDefinitions;
 import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.planner.calcite.FlinkTypeFactory;
 import org.apache.flink.table.planner.functions.bridging.BridgingSqlFunction;
 import org.apache.flink.table.planner.plan.schema.TimeIndicatorRelDataType;
+import org.apache.flink.table.types.logical.LocalZonedTimestampType;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.TimestampType;
 
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.type.RelDataType;
@@ -81,12 +85,21 @@ public final class LateralSnapshotJoinUtil {
             RelDataType rightType,
             JoinRelType joinType,
             List<RelDataTypeField> systemFieldList) {
+        final FlinkTypeFactory flinkTypeFactory = (FlinkTypeFactory) 
typeFactory;
         final RelDataTypeFactory.Builder materializedRight = 
typeFactory.builder();
         for (RelDataTypeField field : rightType.getFieldList()) {
-            final RelDataType fieldType =
-                    field.getType() instanceof TimeIndicatorRelDataType
-                            ? ((TimeIndicatorRelDataType) 
field.getType()).getOriginalType()
-                            : field.getType();
+            final RelDataType fieldType;
+            if (field.getType() instanceof TimeIndicatorRelDataType) {
+                // Materialize the build-side time attribute to a regular 
timestamp, following the
+                // same convention as RelTimeIndicatorConverter
+                final LogicalType materialized =
+                        
FlinkTypeFactory.isTimestampLtzIndicatorType(field.getType())
+                                ? new 
LocalZonedTimestampType(field.getType().isNullable(), 3)
+                                : new 
TimestampType(field.getType().isNullable(), 3);
+                fieldType = 
flinkTypeFactory.createFieldTypeFromLogicalType(materialized);
+            } else {
+                fieldType = field.getType();
+            }
             materializedRight.add(field.getName(), fieldType);
         }
         return SqlValidatorUtil.deriveJoinRowType(
diff --git 
a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtilTest.java
 
b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtilTest.java
new file mode 100644
index 00000000000..fb1dd964918
--- /dev/null
+++ 
b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/LateralSnapshotJoinUtilTest.java
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.utils;
+
+import org.apache.flink.table.planner.calcite.FlinkTypeFactory;
+import org.apache.flink.table.planner.calcite.FlinkTypeSystem;
+import org.apache.flink.table.planner.plan.schema.TimeIndicatorRelDataType;
+
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.sql.type.BasicSqlType;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link LateralSnapshotJoinUtil}. */
+class LateralSnapshotJoinUtilTest {
+
+    private FlinkTypeFactory typeFactory;
+
+    @BeforeEach
+    void setup() {
+        typeFactory =
+                new FlinkTypeFactory(
+                        Thread.currentThread().getContextClassLoader(), 
FlinkTypeSystem.INSTANCE);
+    }
+
+    @ParameterizedTest(name = "nullableCols={0}, leftOuter={1}")
+    @CsvSource({"true, true", "true, false", "false, true", "false, false"})
+    void testDeriveRowType(boolean nullableCols, boolean leftOuter) {
+        // Probe (left) side: a scalar column and a rowtime attribute (both 
forwarded unchanged --
+        // left time attributes are not materialized).
+        final TimeIndicatorRelDataType leftRowtime = rowtime(nullableCols);
+        final RelDataType leftType =
+                typeFactory
+                        .builder()
+                        .add("pk", varchar(nullableCols))
+                        .add("pts", leftRowtime)
+                        .build();
+
+        // Build (right) side: a scalar column and a rowtime attribute whose 
originalType has the
+        // same nullability as the indicator.
+        final TimeIndicatorRelDataType buildRowtime = rowtime(nullableCols);
+        final RelDataType rightType =
+                typeFactory
+                        .builder()
+                        .add("bk", varchar(nullableCols))
+                        .add("bts", buildRowtime)
+                        .build();
+
+        final RelDataType rowType =
+                LateralSnapshotJoinUtil.deriveRowType(
+                        typeFactory,
+                        leftType,
+                        rightType,
+                        leftOuter ? JoinRelType.LEFT : JoinRelType.INNER,
+                        Collections.emptyList());
+
+        final List<RelDataTypeField> fields = rowType.getFieldList();
+        assertThat(fields)
+                .extracting(RelDataTypeField::getName)
+                .containsExactly("pk", "pts", "bk", "bts");
+
+        // Left fields are forwarded unchanged (left is never the null-padded 
side of INNER/LEFT).
+        assertThat(fields.get(0).getType())
+                .as("left scalar forwarded unchanged")
+                .isEqualTo(leftType.getFieldList().get(0).getType());
+        assertThat(fields.get(1).getType())
+                .as("left time attribute forwarded unchanged (still a time 
indicator)")
+                .isInstanceOf(TimeIndicatorRelDataType.class)
+                .isEqualTo(leftRowtime);
+
+        // Build-side scalar: base type preserved; nullable iff the column is 
nullable or it is
+        // null-padded by a LEFT join.
+        
assertThat(fields.get(2).getType().getSqlTypeName()).isEqualTo(SqlTypeName.VARCHAR);
+        assertThat(fields.get(2).getType().isNullable())
+                .as("build-side scalar nullability")
+                .isEqualTo(nullableCols || leftOuter);
+
+        // Build-side time attribute: materialized to a regular timestamp (no 
longer an indicator);
+        // nullable iff the column is nullable or it is null-padded by a LEFT 
join.
+        assertThat(fields.get(3).getType())
+                .as("build-side time attribute is materialized")
+                .isNotInstanceOf(TimeIndicatorRelDataType.class);
+        
assertThat(fields.get(3).getType().getSqlTypeName()).isEqualTo(SqlTypeName.TIMESTAMP);
+        assertThat(fields.get(3).getType().isNullable())
+                .as("materialized build-side rowtime nullability")
+                .isEqualTo(nullableCols || leftOuter);
+    }
+
+    /**
+     * Regression guard for the case where a build-side time attribute's 
{@code originalType}
+     * nullability does NOT match the indicator's. {@code
+     * FlinkTypeFactory#createTypeWithNullability} can widen a NOT NULL 
rowtime to nullable by
+     * flipping only the indicator, leaving {@code originalType} inconsistent. 
{@link
+     * LateralSnapshotJoinUtil#deriveRowType} must materialize using the 
indicator's nullability,
+     * not {@code originalType}'s.
+     */
+    @ParameterizedTest(name = "nullableTimeCol={0}")
+    @ValueSource(booleans = {true, false})
+    void testDeriveRowTypeIgnoresStaleOriginalTypeNullability(boolean 
nullableTimeCol) {
+        // originalType is deliberately the opposite nullability of the 
indicator.
+        final TimeIndicatorRelDataType buildRowtime = rowtime(nullableTimeCol, 
!nullableTimeCol);
+        assertThat(buildRowtime.isNullable()).isEqualTo(nullableTimeCol);
+        
assertThat(buildRowtime.getOriginalType().isNullable()).isEqualTo(!nullableTimeCol);
+
+        final RelDataType leftType =
+                typeFactory
+                        .builder()
+                        .add("pk", 
typeFactory.createSqlType(SqlTypeName.VARCHAR))
+                        .build();
+        final RelDataType rightType =
+                typeFactory
+                        .builder()
+                        .add("bk", 
typeFactory.createSqlType(SqlTypeName.VARCHAR))
+                        .add("bts", buildRowtime)
+                        .build();
+
+        final RelDataType rowType =
+                LateralSnapshotJoinUtil.deriveRowType(
+                        typeFactory,
+                        leftType,
+                        rightType,
+                        JoinRelType.INNER,
+                        Collections.emptyList());
+
+        final RelDataTypeField bts = rowType.getField("bts", true, false);
+        assertThat(bts).isNotNull();
+        assertThat(bts.getType())
+                .as("build-side time attribute is materialized")
+                .isNotInstanceOf(TimeIndicatorRelDataType.class);
+        // Nullability must follow the indicator, not the (opposite) 
originalType.
+        assertThat(bts.getType().isNullable())
+                .as("materialized rowtime uses indicator nullability, not 
originalType")
+                .isEqualTo(nullableTimeCol);
+    }
+
+    private RelDataType varchar(boolean nullable) {
+        return typeFactory.createTypeWithNullability(
+                typeFactory.createSqlType(SqlTypeName.VARCHAR), nullable);
+    }
+
+    private TimeIndicatorRelDataType rowtime(boolean nullable) {
+        return rowtime(nullable, nullable);
+    }
+
+    /**
+     * Builds an event-time {@link TimeIndicatorRelDataType} whose indicator 
and underlying {@code
+     * originalType} nullabilities can be set independently.
+     */
+    private TimeIndicatorRelDataType rowtime(
+            boolean indicatorNullable, boolean originalTypeNullable) {
+        final BasicSqlType originalType =
+                (BasicSqlType)
+                        typeFactory.createTypeWithNullability(
+                                
typeFactory.createSqlType(SqlTypeName.TIMESTAMP, 3),
+                                originalTypeNullable);
+        return new TimeIndicatorRelDataType(
+                typeFactory.getTypeSystem(),
+                originalType,
+                indicatorNullable,
+                /* isEventTime= */ true);
+    }
+}

Reply via email to