Re: SQL:2011 application time

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Paul Jungwirth <pj(at)illuminatedcomputing(dot)com>
Cc: Isaac Morland <isaac(dot)morland(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: SQL:2011 application time
Date: 2024-08-16 02:12:00
Message-ID: CACJufxHc-TvRVmgVLYLWZO0-=+W8Kj8=n_xnh4=FADWznXhysw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 8, 2024 at 4:54 AM Paul Jungwirth
<pj(at)illuminatedcomputing(dot)com> wrote:
>
> Rebased to e56ccc8e42.

I only applied to 0001-0003.
in create_table.sgml, I saw the WITHOUT OVERLAPS change is mainly in
table_constraint.
but we didn't touch alter_table.sgml.
Do we also need to change alter_table.sgml correspondingly?

+ if (constraint->without_overlaps)
+ {
+ /*
+ * This enforces that there is at least one equality column
+ * besides the WITHOUT OVERLAPS columns. This is per SQL
+ * standard. XXX Do we need this?
+ */
+ if (list_length(constraint->keys) < 2)
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("constraint using WITHOUT OVERLAPS needs at least two columns"));
+
+ /* WITHOUT OVERLAPS requires a GiST index */
+ index->accessMethod = "gist";
+ }
if Constraint->conname is not NULL, we can
+ errmsg("constraint \"%s\" using WITHOUT OVERLAPS needs at least two
columns"));

"XXX Do we need this?"
I think currently we need this, otherwise the following create_table
synopsis will not be correct.
UNIQUE [ NULLS [ NOT ] DISTINCT ] ( column_name [, ... ] [,
column_name WITHOUT OVERLAPS ] )
PRIMARY KEY ( column_name [, ... ] [, column_name WITHOUT OVERLAPS ] )

we add a column in catalog-pg-constraint.
do we need change column conexclop,
"If an exclusion constraint, list of the per-column exclusion operators "
but currently, primary key, unique constraint both have valid conexclop.

+static void
+ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname, Datum
attval, char typtype, Oid atttypid)
+{
+ bool isempty;
+ RangeType *r;
+ MultirangeType *mr;
+
+ switch (typtype)
+ {
+ case TYPTYPE_RANGE:
+ r = DatumGetRangeTypeP(attval);
+ isempty = RangeIsEmpty(r);
+ break;
+ case TYPTYPE_MULTIRANGE:
+ mr = DatumGetMultirangeTypeP(attval);
+ isempty = MultirangeIsEmpty(mr);
+ break;
+ default:
+ elog(ERROR, "WITHOUT OVERLAPS column \"%s\" is not a range or multirange",
+ NameStr(attname));
+ }
+
+ /* Report a CHECK_VIOLATION */
+ if (isempty)
+ ereport(ERROR,
+ (errcode(ERRCODE_CHECK_VIOLATION),
+ errmsg("empty WITHOUT OVERLAPS value found in column \"%s\" in
relation \"%s\"",
+ NameStr(attname), RelationGetRelationName(rel))));
+}
I think in the default branch, you need at least set the isempty
value, otherwise maybe there will be a compiler warning
because later your use isempty, but via default branch is value undefined?

+ /*
+ * If this is a WITHOUT OVERLAPS constraint,
+ * we must also forbid empty ranges/multiranges.
+ * This must happen before we look for NULLs below,
+ * or a UNIQUE constraint could insert an empty
+ * range along with a NULL scalar part.
+ */
+ if (indexInfo->ii_WithoutOverlaps)
+ {
+ ExecWithoutOverlapsNotEmpty(heap, att->attname,
+ }
previously we found out that if this happens later, then it won't work.
but this comment didn't explain why this must have happened earlier.
I didn't dig deep enough to find out why.
but explaining it would be very helpful.

I think some tests are duplicated, so I did the refactoring.

Attachment Content-Type Size
v39-0001-refactor-tests.no-cfbot application/octet-stream 14.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2024-08-16 02:45:58 Re: BUG #18348: Inconsistency with EXTRACT([field] from INTERVAL);
Previous Message Thomas Munro 2024-08-16 01:12:33 Re: Remaining dependency on setlocale()