Merge pull request #47488 from ClickHouse/fix-query-parameters

Fix query parameters

src/Interpreters/ReplaceQueryParameterVisitor.cpp

@@ -50,7 +50,16 @@ void ReplaceQueryParameterVisitor::visit(ASTPtr & ast)
 void ReplaceQueryParameterVisitor::visitChildren(ASTPtr & ast)
 {
     for (auto & child : ast->children)
+    {
+        void * old_ptr = child.get();
         visit(child);
+        void * new_ptr = child.get();
+
+        /// Some AST classes have naked pointers to children elements as members.
+        /// We have to replace them if the child was replaced.
+        if (new_ptr != old_ptr)
+            ast->updatePointerToChild(old_ptr, new_ptr);
+    }
 }
 
 const String & ReplaceQueryParameterVisitor::getParamValue(const String & name)
@@ -89,6 +98,7 @@ void ReplaceQueryParameterVisitor::visitQueryParameter(ASTPtr & ast)
         literal = value;
     else
         literal = temp_column[0];
+
     ast = addTypeConversionToAST(std::make_shared<ASTLiteral>(literal), type_name);
 
     /// Keep the original alias.

src/Parsers/ASTAlterQuery.h

@@ -256,6 +256,11 @@ protected:
     void formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override;
 
     bool isOneCommandTypeOnly(const ASTAlterCommand::Type & type) const;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&command_list));
+    }
 };
 
 }

src/Parsers/ASTBackupQuery.h

@@ -94,5 +94,12 @@ public:
     void formatImpl(const FormatSettings & format, FormatState &, FormatStateStacked) const override;
     ASTPtr getRewrittenASTWithoutOnCluster(const WithoutOnClusterASTRewriteParams &) const override;
     QueryKind getQueryKind() const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&backup_name));
+        f(reinterpret_cast<void **>(&base_backup_name));
+    }
 };
+
 }

src/Parsers/ASTConstraintDeclaration.h

@@ -25,5 +25,11 @@ public:
     ASTPtr clone() const override;
 
     void formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&expr));
+    }
 };
+
 }

src/Parsers/ASTCreateQuery.cpp

@@ -91,6 +91,11 @@ public:
     ASTPtr clone() const override;
 
     void formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&elem));
+    }
 };
 
 ASTPtr ASTColumnsElement::clone() const

src/Parsers/ASTCreateQuery.h

@@ -32,6 +32,17 @@ public:
     void formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const override;
 
     bool isExtendedStorageDefinition() const;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&engine));
+        f(reinterpret_cast<void **>(&partition_by));
+        f(reinterpret_cast<void **>(&primary_key));
+        f(reinterpret_cast<void **>(&order_by));
+        f(reinterpret_cast<void **>(&sample_by));
+        f(reinterpret_cast<void **>(&ttl_table));
+        f(reinterpret_cast<void **>(&settings));
+    }
 };
 
 
@@ -57,6 +68,16 @@ public:
         return (!columns || columns->children.empty()) && (!indices || indices->children.empty()) && (!constraints || constraints->children.empty())
             && (!projections || projections->children.empty());
     }
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&columns));
+        f(reinterpret_cast<void **>(&indices));
+        f(reinterpret_cast<void **>(&primary_key));
+        f(reinterpret_cast<void **>(&constraints));
+        f(reinterpret_cast<void **>(&projections));
+        f(reinterpret_cast<void **>(&primary_key));
+    }
 };
 
 
@@ -126,6 +147,19 @@ public:
 
 protected:
     void formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&columns_list));
+        f(reinterpret_cast<void **>(&inner_storage));
+        f(reinterpret_cast<void **>(&storage));
+        f(reinterpret_cast<void **>(&as_table_function));
+        f(reinterpret_cast<void **>(&select));
+        f(reinterpret_cast<void **>(&comment));
+        f(reinterpret_cast<void **>(&table_overrides));
+        f(reinterpret_cast<void **>(&dictionary_attributes_list));
+        f(reinterpret_cast<void **>(&dictionary));
+    }
 };
 
 }

src/Parsers/ASTDictionary.h

@@ -47,6 +47,11 @@ public:
     ASTPtr clone() const override;
 
     void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&parameters));
+    }
 };
 
 

src/Parsers/ASTExternalDDLQuery.h

@@ -41,6 +41,11 @@ public:
     }
 
     QueryKind getQueryKind() const override { return QueryKind::ExternalDDL; }
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&from));
+    }
 };
 
 }

src/Parsers/ASTFunctionWithKeyValueArguments.h

@@ -33,6 +33,11 @@ public:
     bool hasSecretParts() const override;
 
     void updateTreeHashImpl(SipHash & hash_state) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&second));
+    }
 };
 
 

src/Parsers/ASTIndexDeclaration.h

@@ -23,6 +23,12 @@ public:
 
     ASTPtr clone() const override;
     void formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&expr));
+        f(reinterpret_cast<void **>(&type));
+    }
 };
 
 }

src/Parsers/ASTProjectionDeclaration.h

@@ -18,6 +18,11 @@ public:
 
     ASTPtr clone() const override;
     void formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&query));
+    }
 };
 
 }

src/Parsers/ASTTableOverrides.h

@@ -27,6 +27,12 @@ public:
     String getID(char) const override { return "TableOverride " + table_name; }
     ASTPtr clone() const override;
     void formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override;
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&columns));
+        f(reinterpret_cast<void **>(&storage));
+    }
 };
 
 /// List of table overrides, for example:

src/Parsers/IAST.h

@@ -175,6 +175,16 @@ public:
         field = nullptr;
     }
 
+    /// After changing one of `children` elements, update the corresponding member pointer if needed.
+    void updatePointerToChild(void * old_ptr, void * new_ptr)
+    {
+        forEachPointerToChild([old_ptr, new_ptr](void ** ptr) mutable
+        {
+            if (*ptr == old_ptr)
+                *ptr = new_ptr;
+        });
+    }
+
     /// Convert to a string.
 
     /// Format settings.
@@ -295,6 +305,10 @@ public:
 protected:
     bool childrenHaveSecretParts() const;
 
+    /// Some AST classes have naked pointers to children elements as members.
+    /// This method allows to iterate over them.
+    virtual void forEachPointerToChild(std::function<void(void**)>) {}
+
 private:
     size_t checkDepthImpl(size_t max_depth) const;
 

src/Parsers/MySQL/ASTAlterCommand.h

@@ -80,6 +80,15 @@ protected:
     {
         throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method formatImpl is not supported by MySQLParser::ASTAlterCommand.");
     }
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&index_decl));
+        f(reinterpret_cast<void **>(&default_expression));
+        f(reinterpret_cast<void **>(&additional_columns));
+        f(reinterpret_cast<void **>(&order_by_columns));
+        f(reinterpret_cast<void **>(&properties));
+    }
 };
 
 class ParserAlterCommand : public IParserBase

src/Parsers/MySQL/ASTCreateDefines.h

@@ -31,6 +31,13 @@ protected:
     {
         throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method formatImpl is not supported by MySQLParser::ASTCreateDefines.");
     }
+
+    void forEachPointerToChild(std::function<void(void**)> f) override
+    {
+        f(reinterpret_cast<void **>(&columns));
+        f(reinterpret_cast<void **>(&indices));
+        f(reinterpret_cast<void **>(&constraints));
+    }
 };
 
 class ParserCreateDefines : public IParserBase
@@ -44,4 +51,3 @@ protected:
 }
 
 }
-

tests/queries/0_stateless/02679_query_parameters_dangling_pointer.sql

@@ -0,0 +1,4 @@
+-- There is no use-after-free in the following query:
+
+SET param_o = 'a';
+CREATE TABLE test.xxx (a Int64) ENGINE=MergeTree ORDER BY ({o:String}); -- { serverError 44 }