public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 2/3] Allow composite types in bootstrap
2+ messages / 2 participants
[nested] [flat]

* [PATCH 2/3] Allow composite types in bootstrap
@ 2020-11-17 15:28  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Justin Pryzby @ 2020-11-17 15:28 UTC (permalink / raw)

---
 src/backend/bootstrap/bootstrap.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 18eb62ca47..e4fc75ab84 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -916,6 +916,7 @@ gettype(char *type)
 {
 	if (Typ != NIL)
 	{
+		static bool did_reread PG_USED_FOR_ASSERTS_ONLY = false; /* Already reread pg_types */
 		ListCell *lc;
 
 		foreach (lc, Typ)
@@ -927,6 +928,33 @@ gettype(char *type)
 				return app->am_oid;
 			}
 		}
+
+		/*
+		 * The type wasn't known; check again to handle composite
+		 * types, added since first populating the array.
+		 */
+
+		/*
+		 * Once all the types are populated and we handled composite
+		 * types, shouldn't need to do that again.
+		 */
+		Assert(!did_reread);
+		did_reread = true;
+
+		list_free_deep(Typ);
+		Typ = NULL;
+		populate_typ_array();
+
+		/* Need to avoid infinite recursion... */
+		foreach (lc, Typ)
+		{
+			struct typmap *app = lfirst(lc);
+			if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
+			{
+				Ap = app;
+				return app->am_oid;
+			}
+		}
 	}
 	else
 	{
-- 
2.26.2


--------------EED78D4A67CFF327F6B880D2
Content-Type: text/x-patch; charset=UTF-8;
 name="0003-Extended-statistics-on-expressions-20210122b.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0003-Extended-statistics-on-expressions-20210122b.patch"



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: cataloguing NOT NULL constraints
@ 2023-08-15 10:15  Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Alvaro Herrera @ 2023-08-15 10:15 UTC (permalink / raw)
  To: Dean Rasheed <[email protected]>; +Cc: Robert Haas <[email protected]>; Peter Eisentraut <[email protected]>; [email protected]

On 2023-Aug-15, Dean Rasheed wrote:

> I think perhaps for ALTER TABLE INHERIT, it should check that the
> child has a NOT NULL constraint, and error out if not. That's the
> current behaviour, and also matches other constraints types (e.g.,
> CHECK constraints).

Yeah, I reached the same conclusion yesterday while trying it out, so
that's what I implemented.  I'll post later today.

> More generally though, I'm worried that this is starting to get very
> complicated. I wonder if there might be a different, simpler approach.
> One vague idea is to have a new attribute on the column that counts
> the number of constraints (local and inherited PK and NOT NULL
> constraints) that make the column not null.

Hmm.  I grant that this is different, but I don't see that it is
simpler.

> Something else I noticed when reading the SQL standard is that a
> user-defined CHECK (col IS NOT NULL) constraint should be recognised
> by the system as also making the column not null (setting its
> "nullability characteristic" to "known not nullable").

I agree with this view actually, but I've refrained from implementing
it(*) because our SQL-standards people have advised against it.  Insider
knowledge?  I don't know.  I think this is a comparatively smaller
consideration though, and we can adjust for it afterwards.

(*) Rather: at some point I removed the implementation of that from the
patch.

> I'm also wondering whether creating a pg_constraint entry for *every*
> not-nullable column is actually going too far. If we were to
> distinguish between "defined as NOT NULL" and being not null as a
> result of one or more constraints, in the way that the standard seems
> to suggest, perhaps the former (likely to be much more common) could
> simply be a new attribute stored on the column. I think we actually
> only need to create pg_constraint entries if a constraint name or any
> additional constraint properties such as NOT VALID are specified. That
> would lead to far fewer new constraints, less catalog bloat, and less
> noise in the \d output.

There is a problem if we do this, though, which is that we cannot use
the constraints for the things that we want them for -- for example,
remove_useless_groupby_columns() would like to use unique constraints,
not just primary keys; but it depends on the NOT NULL rows being there
for invalidation reasons (namely: if the NOT NULL constraint is dropped,
we need to be able to replan.  Without catalog rows, we don't have a
mechanism to let that happen).

If we don't add all those redundant catalog rows, then this is all for
naught.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree.              (Don Knuth)






^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2023-08-15 10:15 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 15:28 [PATCH 2/3] Allow composite types in bootstrap Justin Pryzby <[email protected]>
2023-08-15 10:15 Re: cataloguing NOT NULL constraints Alvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox