Re: FailedAssertion("!OidIsValid(def->collOid)", File: "view.c", Line: 89)

Thomas Munro <[email protected]>
Newsgroups gmane.comp.db.postgresql.bugs
Message-ID <CA+hUKGLYApN1sZ1193iyPP3SfCs_D7jviP3BzzgbHqm5BATsBw@mail.gmail.com>
On Mon, Dec 2, 2019 at 12:32 PM Manuel Rigger <[email protected]> wrote:
> CREATE TABLE t0(c0 TEXT COLLATE "POSIX");
> CREATE VIEW v0(c0) AS (SELECT (t0.c0 COLLATE "C")::INT FROM t0); --
> FailedAssertion("!OidIsValid(def->collOid)", File: "view.c", Line: 89)
>
> When building without assertions, the CREATE VIEW statement results in
> an error instead:
>
> ERROR: collations are not supported by type integer

I think the options are to add a check and ereport() instead of an
assertion, or just remove the assertion and let BuildDescForRelation()
report the error as it does in non-assert builds.  I think the latter
is better, since it seems well established that BuildDescForRelation()
will complain about that via its call to GetColumnDefCollation().
Here's a patch to do that.

> TRAP: FailedAssertion("!OidIsValid(def->collOid)", File: "view.c", Line: 89)
> postgres: postgres testdb [local] CREATE
> VIEW(ExceptionalCondition+0x76)[0x563c55747066]
> postgres: postgres testdb [local] CREATE VIEW(DefineView+0x4cb)[0x563c55487cdb]
> ...

I'm loving these new back traces.
0001-Remove-assertion-about-collations-from-DefineView.patch (application/octet-stream, 1.2 KB)
From 3014956d94a241d509e98312bad0731ef7730007 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Mon, 2 Dec 2019 13:57:17 +1300
Subject: [PATCH] Remove assertion about collations from DefineView().

If a user has specified a collation for an expression of a
type that isn't collatable, BuildDescForRelation() will
raise an error further down, but until then we can't make
assertions about that.

Reported-by: Manuel Rigger
Discussion: https://postgr.es/m/CA%2Bu7OA5TwqWTrvRoFOF1Vipf5WpfcbQYwtLGzNVXGwa6Ptor9w%40mail.gmail.com
---
 src/backend/commands/view.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 9b5148093b..816b0b6299 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -85,8 +85,11 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
 									def->colname),
 							 errhint("Use the COLLATE clause to set the collation explicitly.")));
 			}
-			else
-				Assert(!OidIsValid(def->collOid));
+
+			/*
+			 * If it's not collatable and a collation has been provided, then
+			 * BuildDescForRelation() will raise an error further down.
+			 */
 
 			attrList = lappend(attrList, def);
 		}
-- 
2.23.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.