pg_proc cleanup (cmu-db#1630)

goddessIU · Jul 29, 2021 · 29af663 · 29af663
1 parent c7e6949
commit 29af663
Show file tree

Hide file tree

Showing 11 changed files with 289 additions and 188 deletions.
diff --git a/script/testing/junit/traces/functions.test b/script/testing/junit/traces/functions.test
@@ -678,4 +678,21 @@ SELECT CONCAT(str_i_val, str_a_val) AS result FROM functions1 WHERE is_null = 1
 1 values hashing to 68b329da9893e34099c7d8ad5cb9c940
 
 statement ok
-DROP TABLE functions1
+DROP TABLE functions1
+
+# should fail to signature match in the branch of matching function has no args, but input has args
+statement error
+select replication_get_last_txn_id('foo')
+----
+
+# should fail to signature match in the branch of matching function has variadic args, but input doesn't match (case 2 of variadic matching)
+# note that Postgres supports this, but I don't think we implicitly cast the number to a string
+statement error
+select concat('foo','bar',3)
+----
+
+# should fail to signature match in the branch of matching function has variadic args, but input doesn't match (case 1 of variadic matching)
+# note that Postgres supports this, but I don't think we implicitly cast the number to a string
+statement error
+select concat(3,'foo','bar')
+----
diff --git a/src/catalog/catalog_accessor.cpp b/src/catalog/catalog_accessor.cpp
@@ -187,14 +187,15 @@ language_oid_t CatalogAccessor::GetLanguageOid(const std::string &lanname) {
 
 bool CatalogAccessor::DropLanguage(language_oid_t language_oid) { return dbc_->DropLanguage(txn_, language_oid); }
 
-proc_oid_t CatalogAccessor::CreateProcedure(const std::string &procname, language_oid_t language_oid,
-                                            namespace_oid_t procns, const std::vector<std::string> &args,
+proc_oid_t CatalogAccessor::CreateProcedure(const std::string &procname, const language_oid_t language_oid,
+                                            const namespace_oid_t procns, const type_oid_t variadic_type,
+                                            const std::vector<std::string> &args,
                                             const std::vector<type_oid_t> &arg_types,
                                             const std::vector<type_oid_t> &all_arg_types,
                                             const std::vector<postgres::PgProc::ArgModes> &arg_modes,
-                                            type_oid_t rettype, const std::string &src, bool is_aggregate) {
-  return dbc_->CreateProcedure(txn_, procname, language_oid, procns, args, arg_types, all_arg_types, arg_modes, rettype,
-                               src, is_aggregate);
+                                            const type_oid_t rettype, const std::string &src, const bool is_aggregate) {
+  return dbc_->CreateProcedure(txn_, procname, language_oid, procns, variadic_type, args, arg_types, all_arg_types,
+                               arg_modes, rettype, src, is_aggregate);
 }
 
 bool CatalogAccessor::DropProcedure(proc_oid_t proc_oid) { return dbc_->DropProcedure(txn_, proc_oid); }

diff --git a/src/catalog/database_catalog.cpp b/src/catalog/database_catalog.cpp
@@ -445,23 +445,24 @@ language_oid_t DatabaseCatalog::GetLanguageOid(const common::ManagedPointer<tran
   return pg_language_.GetLanguageOid(txn, lanname);
 }
 
-proc_oid_t DatabaseCatalog::CreateProcedure(common::ManagedPointer<transaction::TransactionContext> txn,
-                                            const std::string &procname, language_oid_t language_oid,
-                                            namespace_oid_t procns, const std::vector<std::string> &args,
+proc_oid_t DatabaseCatalog::CreateProcedure(const common::ManagedPointer<transaction::TransactionContext> txn,
+                                            const std::string &procname, const language_oid_t language_oid,
+                                            const namespace_oid_t procns, const type_oid_t variadic_type,
+                                            const std::vector<std::string> &args,
                                             const std::vector<type_oid_t> &arg_types,
                                             const std::vector<type_oid_t> &all_arg_types,
                                             const std::vector<postgres::PgProc::ArgModes> &arg_modes,
-                                            type_oid_t rettype, const std::string &src, bool is_aggregate) {
+                                            const type_oid_t rettype, const std::string &src, bool is_aggregate) {
   if (!TryLock(txn)) return INVALID_PROC_OID;
-  proc_oid_t oid = proc_oid_t{next_oid_++};
-  return pg_proc_.CreateProcedure(txn, oid, procname, language_oid, procns, args, arg_types, all_arg_types, arg_modes,
-                                  rettype, src, is_aggregate)
-             ? oid
+  const proc_oid_t proc_oid = proc_oid_t{next_oid_++};
+  return pg_proc_.CreateProcedure(txn, proc_oid, procname, language_oid, procns, variadic_type, args, arg_types,
+                                  all_arg_types, arg_modes, rettype, src, is_aggregate)
+             ? proc_oid
              : INVALID_PROC_OID;
 }
 
 bool DatabaseCatalog::DropProcedure(const common::ManagedPointer<transaction::TransactionContext> txn,
-                                    proc_oid_t proc) {
+                                    const proc_oid_t proc) {
   if (!TryLock(txn)) return false;
   return pg_proc_.DropProcedure(txn, proc);
 }

diff --git a/src/catalog/postgres/builder.cpp b/src/catalog/postgres/builder.cpp
@@ -819,24 +819,23 @@ Schema Builder::GetProcTableSchema() {
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Integer));
   columns.back().SetOid(PgProc::PRORETTYPE.oid_);
 
-  columns.emplace_back("proargtypes", execution::sql::SqlTypeId::Varbinary, 4096, false,
+  columns.emplace_back("proargtypes", execution::sql::SqlTypeId::Varbinary, 4096, true,
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Varbinary));
   columns.back().SetOid(PgProc::PROARGTYPES.oid_);
 
-  // TODO(WAN): PROALLARGTYPES does not follow Postgres semantics, see #1359
-  columns.emplace_back("proallargtypes", execution::sql::SqlTypeId::Varbinary, 4096, false,
+  columns.emplace_back("proallargtypes", execution::sql::SqlTypeId::Varbinary, 4096, true,
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Varbinary));
   columns.back().SetOid(PgProc::PROALLARGTYPES.oid_);
 
-  columns.emplace_back("proargmodes", execution::sql::SqlTypeId::Varbinary, 4096, false,
+  columns.emplace_back("proargmodes", execution::sql::SqlTypeId::Varbinary, 4096, true,
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Varbinary));
   columns.back().SetOid(PgProc::PROARGMODES.oid_);
 
   columns.emplace_back("proargdefaults", execution::sql::SqlTypeId::Varbinary, 4096, false,
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Varbinary));
   columns.back().SetOid(PgProc::PROARGDEFAULTS.oid_);
 
-  columns.emplace_back("proargnames", execution::sql::SqlTypeId::Varbinary, 4096, false,
+  columns.emplace_back("proargnames", execution::sql::SqlTypeId::Varbinary, 4096, true,
                        parser::ConstantValueExpression(execution::sql::SqlTypeId::Varbinary));
   columns.back().SetOid(PgProc::PROARGNAMES.oid_);