On Wed, Oct 11, 2023 at 12:47 PM Paul Jungwirth
<pj@illuminatedcomputing.com> wrote:
>
> On 9/25/23 14:00, Peter Eisentraut wrote:
> > Looking through the tests in v16-0001:
> >
> > +-- PK with no columns just WITHOUT OVERLAPS:
> > +CREATE TABLE temporal_rng (
> > + valid_at tsrange,
> > + CONSTRAINT temporal_rng_pk PRIMARY KEY (valid_at WITHOUT OVERLAPS)
> > +);
> > +ERROR: syntax error at or near "WITHOUT"
> > +LINE 3: CONSTRAINT temporal_rng_pk PRIMARY KEY (valid_at WITHOUT OV...
> > + ^
> >
> > I think this error is confusing. The SQL standard requires at least one
> > non-period column in a PK. I don't know why that is or why we should
> > implement it. But if we want to implement it, maybe we should enforce
> > that in parse analysis rather than directly in the parser, to be able to
> > produce a more friendly error message.
>
> Okay.
>
> (I think the reason the standard requires one non-period column is to
> identify the "entity". If philosophically the row is an Aristotelian
> proposition about that thing, the period qualifies it as true just
> during some time span. So the scalar part is doing the work that a PK
> conventionally does, and the period part does something else. Perhaps a
> PK/UNIQUE constraint with no scalar part would still be useful, but not
> very often I think, and I'm not sure it makes sense to call it PK/UNIQUE.)
>
> > +-- PK with a range column/PERIOD that isn't there:
> > +CREATE TABLE temporal_rng (
> > + id INTEGER,
> > + CONSTRAINT temporal_rng_pk PRIMARY KEY (id, valid_at WITHOUT
> > OVERLAPS)
> > +);
> > +ERROR: range or PERIOD "valid_at" in WITHOUT OVERLAPS does not exist
> >
> > I think here we should just produce a "column doesn't exist" error
> > message, the same as if the "id" column was invalid. We don't need to
> > get into the details of what kind of column it should be. That is done
> > in the next test
>
> I'll change it. The reason for the different wording is that it might
> not be a column at all. It might be a PERIOD. So what about just "column
> or PERIOD doesn't exist"? (Your suggestion is fine too though.)
>
> > +ERROR: column "valid_at" in WITHOUT OVERLAPS is not a range type
> >
> > Also, in any case it would be nice to have a location pointer here (for
> > both cases).
>
> Agreed.
>
I refactored findNeworOldColumn to better handle error reports.
please check the attached.