From 998f84a2cbbdde137070911754d1589c1ba5e414 Mon Sep 17 00:00:00 2001 From: Claude Warren Date: Thu, 10 Aug 2023 10:24:05 +0200 Subject: [PATCH] Fix NPE when using udfContext in UDF after a restart of a node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Claude Warren; reviewed by Stefan Miklosovic and Andres de la Peña for CASSANDRA-18739 --- CHANGES.txt | 1 + .../cql3/functions/UDFContextImpl.java | 24 +++++-- .../cassandra/cql3/functions/UDFunction.java | 4 +- .../cassandra/distributed/test/UDFTest.java | 65 +++++++++++++++++++ 4 files changed, 84 insertions(+), 10 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/test/UDFTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 2b9ee1f73d..7b8fde184b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.11.17 + * Fix NPE when using udfContext in UDF after a restart of a node (CASSANDRA-18739) Merged from 3.0: * Backport of CASSANDRA-16905 Further restrict schema column drop/recreate conversions (CASSANDRA-18760) * CQLSH emits a warning when the server version doesn't match (CASSANDRA-18745) diff --git a/src/java/org/apache/cassandra/cql3/functions/UDFContextImpl.java b/src/java/org/apache/cassandra/cql3/functions/UDFContextImpl.java index 00625cdc08..3e915a5706 100644 --- a/src/java/org/apache/cassandra/cql3/functions/UDFContextImpl.java +++ b/src/java/org/apache/cassandra/cql3/functions/UDFContextImpl.java @@ -23,12 +23,16 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; + import com.datastax.driver.core.DataType; import com.datastax.driver.core.TupleType; import com.datastax.driver.core.TupleValue; import com.datastax.driver.core.TypeCodec; import com.datastax.driver.core.UDTValue; import com.datastax.driver.core.UserType; +import org.apache.cassandra.config.Schema; import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.schema.CQLTypeParser; @@ -40,19 +44,25 @@ import org.apache.cassandra.utils.ByteBufferUtil; */ public final class UDFContextImpl implements UDFContext { - private final KeyspaceMetadata keyspaceMetadata; private final Map> byName = new HashMap<>(); private final TypeCodec[] argCodecs; private final TypeCodec returnCodec; + private final String keyspace; - UDFContextImpl(List argNames, TypeCodec[] argCodecs, TypeCodec returnCodec, - KeyspaceMetadata keyspaceMetadata) + /* + metadata can not be retrieved within the constructor as the keyspace itself may be being + constructed and an NPE will result. + */ + private final Supplier metadata; + + UDFContextImpl(List argNames, TypeCodec[] argCodecs, TypeCodec returnCodec, String keyspace) { for (int i = 0; i < argNames.size(); i++) byName.put(argNames.get(i).toString(), argCodecs[i]); this.argCodecs = argCodecs; this.returnCodec = returnCodec; - this.keyspaceMetadata = keyspaceMetadata; + this.keyspace = keyspace; + this.metadata = Suppliers.memoize(() -> Schema.instance.getKSMetaData(keyspace)); } public UDTValue newArgUDTValue(String argName) @@ -72,9 +82,9 @@ public final class UDFContextImpl implements UDFContext public UDTValue newUDTValue(String udtName) { - Optional udtType = keyspaceMetadata.types.get(ByteBufferUtil.bytes(udtName)); + Optional udtType = metadata.get().types.get(ByteBufferUtil.bytes(udtName)); DataType dataType = UDHelper.driverType(udtType.orElseThrow( - () -> new IllegalArgumentException("No UDT named " + udtName + " in keyspace " + keyspaceMetadata.name) + () -> new IllegalArgumentException("No UDT named " + udtName + " in keyspace " + keyspace) )); return newUDTValue(dataType); } @@ -96,7 +106,7 @@ public final class UDFContextImpl implements UDFContext public TupleValue newTupleValue(String cqlDefinition) { - AbstractType abstractType = CQLTypeParser.parse(keyspaceMetadata.name, cqlDefinition, keyspaceMetadata.types); + AbstractType abstractType = CQLTypeParser.parse(keyspace, cqlDefinition, metadata.get().types); DataType dataType = UDHelper.driverType(abstractType); return newTupleValue(dataType); } diff --git a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java index 6dab536284..11283bff3d 100644 --- a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java +++ b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java @@ -229,9 +229,7 @@ public abstract class UDFunction extends AbstractFunction implements ScalarFunct this.argCodecs = UDHelper.codecsFor(argDataTypes); this.returnCodec = UDHelper.codecFor(returnDataType); this.calledOnNullInput = calledOnNullInput; - KeyspaceMetadata keyspaceMetadata = Schema.instance.getKSMetaData(name.keyspace); - this.udfContext = new UDFContextImpl(argNames, argCodecs, returnCodec, - keyspaceMetadata); + this.udfContext = new UDFContextImpl(argNames, argCodecs, returnCodec, name.keyspace); } public static UDFunction create(FunctionName name, diff --git a/test/distributed/org/apache/cassandra/distributed/test/UDFTest.java b/test/distributed/org/apache/cassandra/distributed/test/UDFTest.java new file mode 100644 index 0000000000..37ec95d5f5 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/UDFTest.java @@ -0,0 +1,65 @@ +/* + * 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.cassandra.distributed.test; + +import org.junit.Test; + +import org.apache.cassandra.distributed.Cluster; + +import static java.lang.Boolean.TRUE; +import static org.apache.cassandra.distributed.api.ConsistencyLevel.ALL; +import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows; +import static org.apache.cassandra.distributed.shared.AssertUtils.row; + +public class UDFTest extends TestBaseImpl +{ + @Test + public void testUDFContextKeyspaceNotNull() throws Throwable + { + /* + To test the keyspace is not null in the context we call UDFContext.newUDTValue() because it + relies on the keyspace not being null. + */ + String[] createStmts = { + "CREATE TABLE " + KEYSPACE + ".current (city text, PRIMARY KEY (city))", + "CREATE TYPE IF NOT EXISTS " + KEYSPACE + ".aggst (lt int, ge int)", + "CREATE FUNCTION " + KEYSPACE + ".udf_not_null ()\n" + + "CALLED ON NULL INPUT\n" + + "RETURNS boolean LANGUAGE java AS $$\n" + + "udfContext.newUDTValue(\"aggst\");\n" + + "return Boolean.TRUE;\n" + + "$$;", + }; + + try (Cluster cluster = init(Cluster.create(1, config -> config.set("enable_user_defined_functions", "true")))) + { + for (String stmt : createStmts) + cluster.schemaChange(stmt); + + cluster.coordinator(1).execute("INSERT INTO " + KEYSPACE + ".current (city) VALUES ('Helsinki')", ALL); + + assertRows(cluster.coordinator(1).execute("SELECT " + KEYSPACE + ".udf_not_null() AS m FROM " + KEYSPACE + ".current", ALL), row(TRUE)); + + cluster.get(1).shutdown().get(); + cluster.get(1).startup(); + + assertRows(cluster.coordinator(1).execute("SELECT " + KEYSPACE + ".udf_not_null() AS m FROM " + KEYSPACE + ".current", ALL), row(TRUE)); + } + } +}