On Tue, Apr 22, 2025 at 4:55 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > Sorry I missed this email while sending the patches - our emails crossed in > the air. > > On Tue, Apr 22, 2025 at 2:15 PM jian he <jian.universal...@gmail.com> wrote: >> >> On Tue, Apr 22, 2025 at 3:02 PM jian he <jian.universal...@gmail.com> wrote: >> > Other than that, it looks good to me for fixing this bug. >> >> The error message seems not that intuitive. >> >> +CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint >> GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE >> ((gtest_part_key)); >> +ERROR: cannot use generated column in partition key >> +LINE 1: ...D ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((gtest_par... >> + ^ >> +DETAIL: Column "f3" is a generated column. > > > Yes. It's not apparent where does f3 appear in the partition key, to a lay > users. Users who explicitly use whole row expression in a partition key, > would know that a whole row expression contains all columns. And the error > location points to the whole-row reference. So the current state isn't that > bad. > >> >> >> with the attached patch. now, >> +CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint >> GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE >> ((gtest_part_key)); >> ERROR: cannot use generated column in partition key >> LINE 1: ...D ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((gtest_par... >> ^ >> DETAIL: Generated column "f3" is part of the partition key of >> relation "gtest_part_key" >> > > The DETAIL here is just mentioning what's already known from the command, so > the change in the DETAIL may not be useful. > > If I understand your intention correctly, we have to mention something to the > effect "partition key expression contains a whole-row reference which in turn > contains a generated column." But that might be too verbose. >
you understand my intention correctly, I aggrees current error message in that location is not that bad. overall your latest patch looks good to me.
