Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix datatype handling for plpgsql function parameters #256

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

svenklemm
Copy link
Contributor

@svenklemm svenklemm commented Aug 17, 2024

Currently all plpgql function parameters are returned as UNKNOWN. This patch changes function parameters to return actual type names used in the function definition.

Fixes #126

@svenklemm svenklemm force-pushed the plpgsql_function_arg_types branch from 20b5f60 to f5e6830 Compare August 17, 2024 08:55
@lfittl
Copy link
Member

lfittl commented Aug 20, 2024

@svenklemm Thanks for the contribution - @msepga will take a look at this.

Currently all plpgql function parameters are returned as UNKNOWN.
This patch changes function parameters to return actual type names
used in the function definition. This patch also fixes a problem
with cursor variables parsing and adds a test for it.
@svenklemm svenklemm force-pushed the plpgsql_function_arg_types branch from f5e6830 to 4592951 Compare August 30, 2024 18:43
@svenklemm
Copy link
Contributor Author

@msepga any update?

@svenklemm
Copy link
Contributor Author

@lfittl looks like @msepga is MIA

@lfittl
Copy link
Member

lfittl commented Sep 14, 2024

@lfittl looks like @msepga is MIA

Just got a lot on our plate, but I'll make sure we can get this reviewed in the next days. Thanks for your patience!

Copy link
Contributor

@msepga msepga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution and patience, @svenklemm! The mock LGTM 👍

One note, looks like running make extract_source seemed to drop quote_qualified_identifier in the source here. I've added a call to resolve the symbol:

 # PL/pgSQL Parsing
 runner.deep_resolve('plpgsql_compile_inline')
 runner.deep_resolve('plpgsql_free_function_memory')
+runner.deep_resolve('quote_qualified_identifier')

I'll approve this PR once the tests pass with the new commit.

For reference, here's the original diff I get after running make extract_source, without the new commit:

diff --git a/src/postgres/src_backend_utils_adt_ruleutils.c b/src/postgres/src_backend_utils_adt_ruleutils.c
index 30df7c3..9257991 100644
--- a/src/postgres/src_backend_utils_adt_ruleutils.c
+++ b/src/postgres/src_backend_utils_adt_ruleutils.c
@@ -1718,18 +1718,7 @@ quote_identifier(const char *ident)
  * Return a name of the form qualifier.ident, or just ident if qualifier
  * is NULL, quoting each component if necessary.  The result is palloc'd.
  */
-char *
-quote_qualified_identifier(const char *qualifier,
-						   const char *ident)
-{
-	StringInfoData buf;
 
-	initStringInfo(&buf);
-	if (qualifier)
-		appendStringInfo(&buf, "%s.", quote_identifier(qualifier));
-	appendStringInfoString(&buf, quote_identifier(ident));
-	return buf.data;
-}
 
 /*
  * get_relation_name
diff --git a/src/postgres/src_pl_plpgsql_src_pl_comp.c b/src/postgres/src_pl_plpgsql_src_pl_comp.c
index 197de46..f99fab0 100644
--- a/src/postgres/src_pl_plpgsql_src_pl_comp.c
+++ b/src/postgres/src_pl_plpgsql_src_pl_comp.c
@@ -879,6 +879,7 @@ plpgsql_build_recfield(PLpgSQL_rec *rec, const char *fldname)
  * It can be NULL if the type could not be a composite type, or if it was
  * identified by OID to begin with (e.g., it's a function argument type).
  */
+
 PLpgSQL_type * plpgsql_build_datatype(Oid typeOid, int32 typmod, Oid collation, TypeName *origtypname)
 {
 	PLpgSQL_type *typ;
@@ -923,6 +924,8 @@ PLpgSQL_type * plpgsql_build_datatype(Oid typeOid, int32 typmod, Oid collation,
 	return typ;
 }
 
+
+
 /*
  * Utility subroutine to make a PLpgSQL_type struct given a pg_type entry
  * and additional details (see comments for plpgsql_build_datatype).

@msepga msepga merged commit 680f5ee into pganalyze:16-latest Sep 24, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Providing types for plpgsql function signature
3 participants