public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: jian he <[email protected]>
To: Paul Jungwirth <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 application time
Date: Wed, 13 Nov 2024 11:53:06 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACJufxHc-TvRVmgVLYLWZO0-=+W8Kj8=n_xnh4=FADWznXhysw@mail.gmail.com>
References: <CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com>
	<CA+Tgmob518sSV4Y8F4LN3X-7+7DmExznG1wCbYrOurfPL_xBOg@mail.gmail.com>
	<CAMsGm5duDGbSp4un6JaRn_MvujfTWCNMPSFDzJNnyW9jn-dWcA@mail.gmail.com>
	<[email protected]>
	<CACJufxHPQaD+q6o_AMg1PG6ETOWC=VqzN_=6QV9C5zbR8j=Tww@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CACJufxESqArbu=R8m3a+ZF-Yia26KTGpEnyqz+8d5BXb1o=c=w@mail.gmail.com>
	<CACJufxECGaLjMcN8TJbX1fGrMFKs6qu+6RiZm98=sLcpiVpu2g@mail.gmail.com>
	<[email protected]>
	<CACJufxHc-TvRVmgVLYLWZO0-=+W8Kj8=n_xnh4=FADWznXhysw@mail.gmail.com>

I committed a few fixes in this area today.  Has everything here been 
addressed?


On 16.08.24 04:12, jian he wrote:
> On Thu, Aug 8, 2024 at 4:54 AM Paul Jungwirth
> <[email protected]> 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.

view thread (35+ 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: SQL:2011 application time
  In-Reply-To: <[email protected]>

* 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