diff --git a/src/Functions/in.cpp b/src/Functions/in.cpp index 6b0f7960ffe9..dffc41a81061 100644 --- a/src/Functions/in.cpp +++ b/src/Functions/in.cpp @@ -17,6 +17,7 @@ namespace ErrorCodes extern const int ILLEGAL_COLUMN; extern const int LOGICAL_ERROR; extern const int ILLEGAL_TYPE_OF_ARGUMENT; + extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH; } namespace @@ -45,13 +46,24 @@ class FunctionIn final : public IFunction return function_name; } + /// The `IgnoreSet` variants are called with the left operand alone during type analysis, but + /// `inIgnoreSet(x, set)` written explicitly stays valid, hence the variadic arity. + bool isVariadic() const override { return ignore_set; } + size_t getNumberOfArguments() const override { - return 2; + return ignore_set ? 0 : 2; } DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override { + if (ignore_set && (arguments.empty() || arguments.size() > 2)) + throw Exception( + ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, + "Number of arguments for function {} doesn't match: passed {}, should be 1 or 2", + getName(), + arguments.size()); + if (arguments[0]->hasDynamicStructure()) throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, "Illegal type {} of argument of function {}", arguments[0]->getName(), getName()); diff --git a/src/Interpreters/ActionsVisitor.cpp b/src/Interpreters/ActionsVisitor.cpp index ae5a0f1a0b76..595f064b0a01 100644 --- a/src/Interpreters/ActionsVisitor.cpp +++ b/src/Interpreters/ActionsVisitor.cpp @@ -1308,10 +1308,20 @@ void ActionsMatcher::visit(const ASTFunction & node, const ASTPtr & ast, Data & /// We are in the part of the tree that we are not going to compute. You just need to define types. /// Do not evaluate subquery and create sets. We replace "in*" function to "in*IgnoreSet". + /// Pass the left operand alone: the `IgnoreSet` variants never read the set, and the + /// real `in` always gets its set as a constant column, so a constant is what the + /// `LowCardinality` bookkeeping in `IFunctionOverloadResolver::getReturnType` expects + /// to see there. Passing the left operand twice instead would count two full + /// `LowCardinality` columns and type the expression as plain `UInt8` while execution + /// yields `LowCardinality(UInt8)`, so a query reading such a column across a subquery + /// boundary would fail the type check in `ActionsDAG::updateHeader`. A stand-in + /// constant column is not an option either: it would become part of the captured + /// arguments of an enclosing lambda and be looked up in later analysis passes that + /// never created it. auto argument_name = node.arguments->children.at(0)->getColumnName(); data.addFunction( FunctionFactory::instance().get(node.name + "IgnoreSet", data.getContext()), - {argument_name, argument_name}, + {argument_name}, column_name); } return; diff --git a/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.reference b/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.reference new file mode 100644 index 000000000000..e491be679d8b --- /dev/null +++ b/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.reference @@ -0,0 +1,18 @@ +-- old analyzer +LowCardinality(UInt8) LowCardinality(UInt8) +LowCardinality(UInt8) +('a',1) +('b',0) +['a'] +[] +1 +1 +-- the analyzer +LowCardinality(UInt8) LowCardinality(UInt8) +LowCardinality(UInt8) +('a',1) +('b',0) +['a'] +[] +1 +1 diff --git a/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.sql b/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.sql new file mode 100644 index 000000000000..a0453f032059 --- /dev/null +++ b/tests/queries/0_stateless/04894_in_subquery_lowcardinality_type_old_analyzer.sql @@ -0,0 +1,35 @@ +-- The result type of `IN` over a full `LowCardinality` column must not depend on the form of the +-- right-hand side, and must be the same during analysis and during execution. The old analyzer +-- replaces `in` with `inIgnoreSet` while it only needs the types, and used to pass the left operand +-- as the stand-in for the set that is not built yet: two full `LowCardinality` arguments type as +-- plain `UInt8`, while executing the real `in` against a constant set yields `LowCardinality(UInt8)`. +-- Reading such a column across a subquery boundary then failed the type check in +-- `ActionsDAG::updateHeader` with `Unexpected return type from tuple` (`LOGICAL_ERROR`). +-- The `arrayFilter` queries cover the same rewrite inside a lambda, whose captured arguments +-- must not gain columns that later analysis passes never create. + +DROP TABLE IF EXISTS t_in_lc_type; +CREATE TABLE t_in_lc_type (s LowCardinality(String)) ENGINE = MergeTree ORDER BY s; +INSERT INTO t_in_lc_type VALUES ('a'), ('b'); + +SELECT '-- old analyzer'; +SET enable_analyzer = 0; + +SELECT DISTINCT toTypeName(s IN ('a')) AS literal_set, toTypeName(s IN (SELECT 'a')) AS subquery_set FROM t_in_lc_type; +SELECT DISTINCT toTypeName(f) FROM (SELECT s IN (SELECT 'a') AS f FROM t_in_lc_type); +SELECT tuple(*) AS t FROM (SELECT s, s IN (SELECT 'a') AS f FROM t_in_lc_type) ORDER BY t; +SELECT arrayFilter(x -> (x IN (SELECT 'a')), [s, 'b']) FROM t_in_lc_type ORDER BY s; +SELECT count() FROM t_in_lc_type WHERE s IN (SELECT 'a'); +SELECT count() FROM t_in_lc_type WHERE s NOT IN (SELECT 'a'); + +SELECT '-- the analyzer'; +SET enable_analyzer = 1; + +SELECT DISTINCT toTypeName(s IN ('a')) AS literal_set, toTypeName(s IN (SELECT 'a')) AS subquery_set FROM t_in_lc_type; +SELECT DISTINCT toTypeName(f) FROM (SELECT s IN (SELECT 'a') AS f FROM t_in_lc_type); +SELECT tuple(*) AS t FROM (SELECT s, s IN (SELECT 'a') AS f FROM t_in_lc_type) ORDER BY t; +SELECT arrayFilter(x -> (x IN (SELECT 'a')), [s, 'b']) FROM t_in_lc_type ORDER BY s; +SELECT count() FROM t_in_lc_type WHERE s IN (SELECT 'a'); +SELECT count() FROM t_in_lc_type WHERE s NOT IN (SELECT 'a'); + +DROP TABLE t_in_lc_type;