strange context message in spi.c?

Started by Pavel Stehuleover 1 year ago7 messages
#1Pavel Stehule
pavel.stehule@gmail.com

Hi

I am found strange switch:

<--><-->switch (carg->mode)
<--><-->{
<--><--><-->case RAW_PARSE_PLPGSQL_EXPR:
<--><--><--><-->errcontext("SQL expression \"%s\"", query);
<--><--><--><-->break;
<--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN1:
<--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN2:
<--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN3:
<--><--><--><-->errcontext("PL/pgSQL assignment \"%s\"", query);
<--><--><--><-->break;
<--><--><-->default:
<--><--><--><-->errcontext("SQL statement \"%s\"", query);
<--><--><--><-->break;
<--><-->}

Is the message "SQL expression ..." for RAW_PLPGSQL_EXPR correct?

Should there be a "PL/pgSQL expression" instead?

Regards

Pavel

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Pavel Stehule (#1)
Re: strange context message in spi.c?

On 13 Jun 2024, at 19:21, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Is the message "SQL expression ..." for RAW_PLPGSQL_EXPR correct?

That indeed seems incorrect.

Should there be a "PL/pgSQL expression" instead?

I think that would make more sense.

--
Daniel Gustafsson

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Daniel Gustafsson (#2)
1 attachment(s)
Re: strange context message in spi.c?

Hi

čt 13. 6. 2024 v 20:56 odesílatel Daniel Gustafsson <daniel@yesql.se>
napsal:

On 13 Jun 2024, at 19:21, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Is the message "SQL expression ..." for RAW_PLPGSQL_EXPR correct?

That indeed seems incorrect.

Should there be a "PL/pgSQL expression" instead?

I think that would make more sense.

here is the patch

Regards

Pavel

Show quoted text

--
Daniel Gustafsson

Attachments:

0001-fix-mesleading-info-about-expression-context.patchtext/x-patch; charset=US-ASCII; name=0001-fix-mesleading-info-about-expression-context.patchDownload
From 3ad34d511f3b8f1df451cd86178da771692d4926 Mon Sep 17 00:00:00 2001
From: "okbob@github.com" <pavel.stehule@gmail.com>
Date: Sat, 15 Jun 2024 07:56:47 +0200
Subject: [PATCH] fix mesleading info about expression context

---
 src/backend/executor/spi.c                       | 2 +-
 src/pl/plpgsql/src/expected/plpgsql_record.out   | 8 ++++----
 src/pl/plpgsql/src/expected/plpgsql_varprops.out | 2 +-
 src/test/regress/expected/plpgsql.out            | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index e516c0a67c..a84e444a9a 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2976,7 +2976,7 @@ _SPI_error_callback(void *arg)
 		switch (carg->mode)
 		{
 			case RAW_PARSE_PLPGSQL_EXPR:
-				errcontext("SQL expression \"%s\"", query);
+				errcontext("PL/pgSQL expression \"%s\"", query);
 				break;
 			case RAW_PARSE_PLPGSQL_ASSIGN1:
 			case RAW_PARSE_PLPGSQL_ASSIGN2:
diff --git a/src/pl/plpgsql/src/expected/plpgsql_record.out b/src/pl/plpgsql/src/expected/plpgsql_record.out
index a9b5b778ef..6974c8f4a4 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_record.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_record.out
@@ -272,7 +272,7 @@ NOTICE:  r1.q1 = <NULL>
 NOTICE:  r1.q2 = <NULL>
 NOTICE:  r1 = <NULL>
 ERROR:  record "r1" has no field "nosuchfield"
-CONTEXT:  SQL expression "r1.nosuchfield"
+CONTEXT:  PL/pgSQL expression "r1.nosuchfield"
 PL/pgSQL function inline_code_block line 7 at RAISE
 -- records, not so much
 do $$
@@ -286,7 +286,7 @@ end$$;
 NOTICE:  r1 = <NULL>
 ERROR:  record "r1" is not assigned yet
 DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.
-CONTEXT:  SQL expression "r1.f1"
+CONTEXT:  PL/pgSQL expression "r1.f1"
 PL/pgSQL function inline_code_block line 5 at RAISE
 -- but OK if you assign first
 do $$
@@ -304,7 +304,7 @@ NOTICE:  r1.f1 = 1
 NOTICE:  r1.f2 = 2
 NOTICE:  r1 = (1,2)
 ERROR:  record "r1" has no field "nosuchfield"
-CONTEXT:  SQL expression "r1.nosuchfield"
+CONTEXT:  PL/pgSQL expression "r1.nosuchfield"
 PL/pgSQL function inline_code_block line 9 at RAISE
 -- check %type with block-qualified variable names
 do $$
@@ -598,7 +598,7 @@ create function getf3(x mutable) returns int language plpgsql as
 $$ begin return x.f3; end $$;
 select getf3(null::mutable);  -- doesn't work yet
 ERROR:  record "x" has no field "f3"
-CONTEXT:  SQL expression "x.f3"
+CONTEXT:  PL/pgSQL expression "x.f3"
 PL/pgSQL function getf3(mutable) line 1 at RETURN
 alter table mutable add column f3 int;
 select getf3(null::mutable);  -- now it works
diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
index 25115a02bd..958d7bca9a 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_varprops.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
@@ -76,7 +76,7 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  division by zero
-CONTEXT:  SQL expression "1/0"
+CONTEXT:  PL/pgSQL expression "1/0"
 PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x bigint[] := array[1,3,5];
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 074af8f33a..0a6945581b 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2388,7 +2388,7 @@ begin
 end $$ language plpgsql;
 select namedparmcursor_test7();
 ERROR:  division by zero
-CONTEXT:  SQL expression "42/0 AS p1, 77 AS p2"
+CONTEXT:  PL/pgSQL expression "42/0 AS p1, 77 AS p2"
 PL/pgSQL function namedparmcursor_test7() line 6 at OPEN
 -- check that line comments work correctly within the argument list
 -- (this used to require a special hack in the code; it no longer does,
@@ -4563,11 +4563,11 @@ end
 $$;
 select fail();
 ERROR:  division by zero
-CONTEXT:  SQL expression "1/0"
+CONTEXT:  PL/pgSQL expression "1/0"
 PL/pgSQL function fail() line 3 at RETURN
 select fail();
 ERROR:  division by zero
-CONTEXT:  SQL expression "1/0"
+CONTEXT:  PL/pgSQL expression "1/0"
 PL/pgSQL function fail() line 3 at RETURN
 drop function fail();
 -- Test handling of string literals.
-- 
2.45.2

#4Stepan Neretin
sncfmgg@gmail.com
In reply to: Pavel Stehule (#3)
Re: strange context message in spi.c?

Hi! Looks good to me!
Best regards, Stepan Neretin.

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Stepan Neretin (#4)
Re: strange context message in spi.c?

On 24 Jun 2024, at 11:14, Stepan Neretin <sncfmgg@gmail.com> wrote:

Hi! Looks good to me!

Thanks for review. I have this on my TODO for when the tree branches, it
doesn't seem like anything worth squeezing in before then.

--
Daniel Gustafsson

#6Umar Hayat
postgresql.wizard@gmail.com
In reply to: Daniel Gustafsson (#5)
Re: strange context message in spi.c?

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

As tree is branched out for PG17, I guess now it's time to commit.
- No need to rebase
- make, make-check , install-check verified

The new status of this patch is: Ready for Committer

#7Peter Eisentraut
peter@eisentraut.org
In reply to: Umar Hayat (#6)
Re: strange context message in spi.c?

On 03.08.24 07:43, Umar Hayat wrote:

The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: tested, failed

As tree is branched out for PG17, I guess now it's time to commit.
- No need to rebase
- make, make-check , install-check verified

The new status of this patch is: Ready for Committer

committed