hacklu-tu commented on code in PR #68131: URL: https://github.com/apache/doris/pull/68131#discussion_r4214187993
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CharacterSetLiterals.java: ########## @@ -0,0 +1,75 @@ +// 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.doris.nereids.trees.expressions.functions.scalar; + +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral; + +import com.google.common.collect.ImmutableList; + +import java.util.List; + +/** Character sets accepted by encode and decode. */ +final class CharacterSetLiterals { + private static final List<String> SUPPORTED = ImmutableList.of( + "US-ASCII", "ISO-8859-1", "UTF-8", "UTF-16BE", "UTF-16LE", "UTF-16"); + + private CharacterSetLiterals() { + } + + static void checkSecondArgument(ScalarFunction function) { + Expression characterSet = function.getArgument(1); + if (!characterSet.isLiteral()) { + throw new AnalysisException("the second argument of function " + + function.getName() + " must be a literal: " + function.toSql()); + } + if (characterSet.isNullLiteral()) { + return; Review Comment: The intended behavior is to return SQL NULL when the charset argument is a NULL literal, for both encode and decode. The result types remain VARBINARY for encode and STRING for decode. The charset contract intentionally accepts a supported string literal or NULL, but not a charset column. The existing test_encode_decode SQL regression already covers non-NULL source columns with a NULL charset for both functions. I will add planner-level tests that exercise analyze().rewrite() and assert a typed NULL result, plus legality checks for both functions. This preserves the separate validation rule: a non-NULL unsupported charset literal is rejected even when the first argument is NULL. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CharacterSetLiterals.java: ########## @@ -0,0 +1,75 @@ +// 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.doris.nereids.trees.expressions.functions.scalar; + +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral; + +import com.google.common.collect.ImmutableList; + +import java.util.List; + +/** Character sets accepted by encode and decode. */ +final class CharacterSetLiterals { + private static final List<String> SUPPORTED = ImmutableList.of( + "US-ASCII", "ISO-8859-1", "UTF-8", "UTF-16BE", "UTF-16LE", "UTF-16"); + + private CharacterSetLiterals() { + } + + static void checkSecondArgument(ScalarFunction function) { + Expression characterSet = function.getArgument(1); + if (!characterSet.isLiteral()) { Review Comment: I will remove the preceding isLiteral() check and keep the NULL-literal special case followed by the StringLikeLiteral check. That remaining check already rejects charset columns, other expressions, and non-string literals, so the accepted inputs will stay unchanged. I will also update the tests to assert the remaining string-literal diagnostic and verify that these invalid inputs are still rejected. -- 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]
