public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kirill Reshke <[email protected]>
To: Tom Lane <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: CREATE SCHEMA ... CREATE DOMAIN support
Date: Thu, 28 Nov 2024 10:27:00 +0500
Message-ID: <CALdSSPhckRXW+KEvdsUmkQ-ErbrP_vPNjGwgXNdpXDb8xnLEbQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CALdSSPh4jUSDsWu3K58hjO60wnTRR0DuO4CKRcwa8EVuOSfXxg@mail.gmail.com>
<CACJufxG+mrh2O9RS0gX43gU6sv+CMY847eMjMQpe8t4ou-2ryg@mail.gmail.com>
<CACJufxFUdgqDiK9B+VNtnAwZOj=O3NqdLtXO_OrOwE5XPdCpBA@mail.gmail.com>
<CALdSSPggNNvcad69dhUceZ_gPuEYnKNNd=WJ_WnP=YDmh=iwmw@mail.gmail.com>
<[email protected]>
On Wed, 27 Nov 2024 at 23:39, Tom Lane <[email protected]> wrote:
>
> Kirill Reshke <[email protected]> writes:
> > On Wed, 27 Nov 2024 at 08:42, jian he <[email protected]> wrote:
> >> CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
> >> create domain ss1 as ss
> >> create domain ss as text;
> >> ERROR: type "ss" does not exist
> >>
> >> the error message seems not that OK,
> >> if we can point out the error position, that would be great.
>
> > To implement this, we need to include `ParseLoc location` to the
> > `CreateDomainStmt` struct, which is doubtful, because I don't see any
> > other type of create *something* that does this.
>
> No, that error is thrown from typenameType(), which has a perfectly
> good location in the TypeName. What it's lacking is a ParseState
> containing the source query string.
>
> Breakpoint 1, typenameType (pstate=pstate@entry=0x0, typeName=0x25d6b58,
> typmod_p=typmod_p@entry=0x7ffe7dcd641c) at parse_type.c:268
> 268 tup = LookupTypeName(pstate, typeName, typmod_p, false);
> (gdb) p pstate
> $2 = (ParseState *) 0x0
> (gdb) p typeName->location
> $3 = 21
>
> We've fixed a few utility statements so that they can receive
> a passed-down ParseState, but not DefineDomain.
>
> regards, tom lane
Indeed, my analysis is wrong.
Turns out passing parsestate to DefineDomain is itself enhancement.
Before this patch:
```
db1=# create domain ss1 as ss;
ERROR: type "ss" does not exist
```
after:
```
db1=# create domain ss1 as ss;
ERROR: type "ss" does not exist
LINE 1: create domain ss1 as ss;
^
```
PFA as an independent patch then. Or should we combine these two into one?
--
Best regards,
Kirill Reshke
Attachments:
[application/octet-stream] v1-0001-Pass-ParseState-as-first-param-to-DefineRelation.patch (2.7K, 2-v1-0001-Pass-ParseState-as-first-param-to-DefineRelation.patch)
download | inline diff:
From e9a0908697ff89e73197b2a366b4c1c41a416975 Mon Sep 17 00:00:00 2001
From: reshke kirill <[email protected]>
Date: Thu, 28 Nov 2024 05:19:58 +0000
Subject: [PATCH v1] Pass ParseState as first param to DefineRelation
ParseState was lacking in typenameType call inside DefineDomain.
This patch fixes that. Now error-message for create domain
also show error location.
---
src/backend/commands/typecmds.c | 8 ++------
src/backend/tcop/utility.c | 2 +-
src/include/commands/typecmds.h | 2 +-
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 859e2191f08..42407df89a5 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -694,7 +694,7 @@ RemoveTypeById(Oid typeOid)
* Registers a new domain.
*/
ObjectAddress
-DefineDomain(CreateDomainStmt *stmt)
+DefineDomain(ParseState *pstate, CreateDomainStmt *stmt)
{
char *domainName;
char *domainArrayName;
@@ -761,7 +761,7 @@ DefineDomain(CreateDomainStmt *stmt)
/*
* Look up the base type.
*/
- typeTup = typenameType(NULL, stmt->typeName, &basetypeMod);
+ typeTup = typenameType(pstate, stmt->typeName, &basetypeMod);
baseType = (Form_pg_type) GETSTRUCT(typeTup);
basetypeoid = baseType->oid;
@@ -885,12 +885,8 @@ DefineDomain(CreateDomainStmt *stmt)
if (constr->raw_expr)
{
- ParseState *pstate;
Node *defaultExpr;
- /* Create a dummy ParseState for transformExpr */
- pstate = make_parsestate(NULL);
-
/*
* Cook the constr->raw_expr into an expression. Note:
* name is strictly for error message
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index f28bf371059..33dea5a781c 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1712,7 +1712,7 @@ ProcessUtilitySlow(ParseState *pstate,
break;
case T_CreateDomainStmt:
- address = DefineDomain((CreateDomainStmt *) parsetree);
+ address = DefineDomain(pstate, (CreateDomainStmt *) parsetree);
break;
case T_CreateConversionStmt:
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index e1b02927c4b..cb30d1a2583 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -23,7 +23,7 @@
extern ObjectAddress DefineType(ParseState *pstate, List *names, List *parameters);
extern void RemoveTypeById(Oid typeOid);
-extern ObjectAddress DefineDomain(CreateDomainStmt *stmt);
+extern ObjectAddress DefineDomain(ParseState *pstate, CreateDomainStmt *stmt);
extern ObjectAddress DefineEnum(CreateEnumStmt *stmt);
extern ObjectAddress DefineRange(ParseState *pstate, CreateRangeStmt *stmt);
extern ObjectAddress AlterEnum(AlterEnumStmt *stmt);
--
2.34.1
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: CREATE SCHEMA ... CREATE DOMAIN support
In-Reply-To: <CALdSSPhckRXW+KEvdsUmkQ-ErbrP_vPNjGwgXNdpXDb8xnLEbQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox